Skip to content

Harden Phase 0–3: validate tick sizes, enforce time ordering on PATCH#4

Merged
1816x merged 5 commits into
mainfrom
claude/revision-fase-0-2-x16tjl
Jul 23, 2026
Merged

Harden Phase 0–3: validate tick sizes, enforce time ordering on PATCH#4
1816x merged 5 commits into
mainfrom
claude/revision-fase-0-2-x16tjl

Conversation

@1816x

@1816x 1816x commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Summary

This PR hardens the backtest system by adding validation at critical boundaries and enforcing domain invariants more strictly. The changes close two latent holes: zero-size ticks that could poison metrics with NaN values, and partial timestamp updates that could leave journal entries in an inconsistent state.

Key Changes

  • Reject zero-size ticks at parse time (engine/src/tick.rs):

    • Added positive_size serde field validator that rejects size == 0 when the tape is deserialized
    • Prevents windows with zero total volume, which would cause OFI and VWAP to divide by zero
    • Updated MetricsBucket::ofi documentation to reflect that the denominator is now guaranteed non-zero
    • Added tests for zero and negative size rejection
  • Enforce time ordering on partial PATCH updates (backtest/src/backtest/journal.py):

    • Added InvalidEntryUpdate exception for time-ordering violations
    • Refactored update_entry to fetch the stored row, merge updates with current values, and validate the merged result before writing
    • Ensures exited_at_ns >= entered_at_ns even when only one timestamp is in the request body
    • API endpoint now catches InvalidEntryUpdate and returns 422 with descriptive error message
    • Added comprehensive tests covering partial updates that would violate ordering
  • Consolidate INSERT statement (backtest/src/backtest/journal.py):

    • Extracted _INSERT_SQL constant used by both insert_entry and import_csv
    • Changed bulk import to stamp each row with its own time.time_ns() instead of sharing a batch timestamp, matching single-insert behavior
  • Dependency constraint (backtest/pyproject.toml):

    • Capped anthropic below major version 1.0 (>=0.117,<1) to protect against breaking changes in recent SDK surface (structured output parsing, adaptive thinking)
    • Removed stale uv.lock entry from .gitattributes (project doesn't use uv)

Implementation Details

The time-ordering validation in update_entry uses a new _check_times_ordered helper that:

  • Takes the merged state (updated values + stored values for unmodified fields)
  • Allows open positions (when exited_at_ns is None)
  • Raises InvalidEntryUpdate only when the effective exit would be before the entry
  • Runs before any database write, ensuring atomicity

This approach keeps the pydantic validator as a fast path for requests with both timestamps, while the database layer enforces the rule against the actual stored state.

claude added 5 commits July 22, 2026 22:48
A window whose ticks all had size 0 produced zero total volume, making
OFI (`(buy-sell)/(buy+sell)`) and VWAP (`price_size_sum/total_volume`)
evaluate to NaN, which was then written to the metrics table. The
synthetic generator never emits 0, so this was latent but reachable with
any hand-crafted tape.

Reject `size == 0` at parse time with a serde field validator, so the
error carries row context via the existing csv error path and no metrics
code has to guard the denominator. `MetricsBucket::ofi`'s "denominator is
never zero" doc claim is now actually guaranteed. Negative inputs were
already rejected by `u32` parsing; both cases are covered by new tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012nrxeE8qNcKXkYovFWkxGH
The PATCH body validator only enforced `exited_at_ns >= entered_at_ns`
when both fields arrived together, so a partial update setting just one
timestamp could persist an exit before its entry (the stored value the
Pydantic validator can't see). `journal.update_entry` now merges the
update with the stored row before writing and raises `InvalidEntryUpdate`
when the result would be out of order; the endpoint maps it to 422, the
same status the rule already returns on POST. Nothing is written when the
check fails. Regression tests cover both single-timestamp directions at
the journal and API layers.

Also folds in two form cleanups to the same module: `insert_entry` and
`import_csv` now build the INSERT from one shared `_INSERT_SQL` constant,
and the bulk import stamps `created_at_ns` per row (matching the
single-insert path) instead of sharing one batch timestamp.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012nrxeE8qNcKXkYovFWkxGH
`main()` was the only public function in the module without a docstring.
Describe the CLI (optional --load seeding, then analyze and print).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012nrxeE8qNcKXkYovFWkxGH
…tribute

`coach.py` rides recent Anthropic SDK surface (messages.parse with
output_format, parsed_output, adaptive thinking), so a 1.0 major could
break it silently. Cap it `>=0.117,<1`; leave fastapi/uvicorn
lower-bounded so CI keeps exercising the latest of the stable deps.

Also remove the `uv.lock linguist-vendored` line: the project builds with
pip, not uv, and the file has never existed. A full uv.lock plus CI
migration was considered and deferred.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012nrxeE8qNcKXkYovFWkxGH
Note the PATCH stored-state validation in the journal API contract, and
add a Design decisions entry logging the Phase 0-3 hardening review: the
zero-size parse guard, the partial-update ordering check, the INSERT/
created_at cleanups, and the anthropic version cap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012nrxeE8qNcKXkYovFWkxGH
@1816x
1816x merged commit 44461c1 into main Jul 23, 2026
2 checks passed
@1816x
1816x deleted the claude/revision-fase-0-2-x16tjl branch July 23, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants