Skip to content

[obvious-fleet] Backend: make the capture-journey fixture explicit and fail loudly when it disappears - #260

Merged
SamGu-NRX merged 1 commit into
alphafrom
obv/burst5-20261015-bs-fixture-manifest
Oct 16, 2026
Merged

SamGu-NRX merged 1 commit into
alphafrom
obv/burst5-20261015-bs-fixture-manifest

Conversation

@obvious-autobuild

Copy link
Copy Markdown

fleet-packet: burst5-20261015-bs-fixture-manifest

What changed and why

The capture journey test (backend/tests/test_capture_fixture_flow.py, PR #254) copies the committed data/fixtures/packets/mini-arkit packet and drives the app's upload sequence over it. Two things made that fixture implicit:

  • The fixture manifest named the packet (mini-arkit, kind packet_dir) but hashed only packet.json — none of the eleven keyframes and streams its files array references. Directory discovery (the glob in _packet_manifests, plus shutil.copytree in the test) was the only thing tying those files to the packet.
  • The committed packet check runs server.contracts.cli check-packets --no-files for every manifest, so nothing on the validation path ever read those files from disk. A keyframe could leave the repository and make validate still passed; the only symptom was a raw FileNotFoundError from copytree/_sealed_files inside the journey test.

The change makes the fixture explicit and fails loudly when it disappears:

  1. Manifest entries — backend/data/fixtures/manifest.json gains eleven kind: "file" items, one per file mini-arkit/packet.json references, each with its own bytes and sha256. The diff is purely additive (121 insertions, 0 deletions); every existing hash, including mini-arkit's own packet.json entry (9f59f921…, 13734 bytes), is byte-identical. Each new hash was verified equal to the matching sha256 already recorded inside packet.json's files array — the manifest restates what the packet asserts, nothing new is invented.
  2. Validation seam — backend/scripts/validate_fixtures.py gains capture_journey_errors(fixtures_dir), wired into _validate_committed after the packet checks. It cross-checks three sources and fails with named errors when they disagree: the manifest items under the packet directory, packet.json's files array (by path and index), and the bytes on disk. It reports missing input <repo_path> naming both the manifest item and the packet files index, does not match for size/hash drift, and references <path>, which the fixture manifest does not name when the packet references a file the manifest omits. It also re-verifies the packet_dir entry's hash of packet.json itself. Nothing was loosened: no threshold moved, and missing laser/GPU assets still read as absent (_score_self still exits 2 on a missing PLY; the guard adds a check, it removes none).

Why a guard instead of implicit directory discovery

Implicit discovery (globs + copytree) has no opinion about which files the fixture needs, so it cannot say what went missing — it fails where it happens to touch the gap, with a filesystem error and no name. The explicit manifest guard moves the decision to validation time, before any test runs: make validate now exits 1 and prints the exact missing input, its manifest item, and its packet.json index. That protects the app path because the journey test exercises the same sequence the iOS uploader sends — create → register → PUT → commit → finalize → events — so a silently incomplete fixture would fail CI with an opaque FileNotFoundError (or worse, pass if the test's skip-path happened to cover the missing file) instead of telling a reviewer the fixture is broken. With the guard, the fixture contract is checked where every other golden fixture's contract is checked.

How I verified it

All commands run from backend/ in a fresh clone at ~/work/burst5-20261015-bs-fixture-manifest (BaseScanning repo sandbox, option a; the sandbox's pre-existing checkout was not touched):

  • uv sync --frozen — clean install, Python 3.12.
  • uv run --frozen python scripts/validate_fixtures.py — validate ok scenes=5 verdicts=16 labels=61 packets=36, with the guard active. Same counts as the inventory PR ([obvious-fleet] Backend inventory after the port into backend/: checks run, one post-move path bug fixed #248) reported.
  • Missing-file proof: moved keyframes/k00003.jpg out of the tree, re-ran validate — exit 1 with capture-journey fixture guard failed: mini-arkit: missing input data/fixtures/packets/mini-arkit/keyframes/k00003.jpg (manifest item mini-arkit-file-keyframes-k00003-jpg, packet.json /files/4); restored the file — exit 0. (Note the pre-existing check-packets --no-files still prints ok for the packet in that state; the guard is what catches it.)
  • uv run --frozen --extra api pytest -q tests/test_capture_fixture_manifest.py tests/test_capture_fixture_flow.py — 7 passed (5 new guard tests + the 2 journey tests). New tests: committed fixture passes the guard; a temporary copy missing a keyframe, and one missing a stream, each fail naming the input; a packet file absent from the manifest fails naming the path; a tampered file fails naming the drift.
  • uv run --frozen ruff check . — all checks passed. uv run --frozen ruff format --check . — 461 files already formatted (one reformat applied to the new seam code first). uv run --frozen lint-imports — contracts 5 kept, 0 broken.
  • Full backend suite uv run --frozen --extra api pytest -q — result appended in a comment below once the run finished.

What is not verified

  • The laser/GPU fixture families remain unavailable, same as the inventory PR recorded, and this change does not touch them: eth3d-*-laser / eth3d-*-occluders / colmap-* points (committed: false, fetched from the fixture store, which is unreachable here) and the GPU weights for the recon/detection tracks. make validate-full, make criteria-golden and the breadth tracks were therefore not runnable — the committed make validate target, which this PR changes, was.
  • The full-suite run's known pre-existing failures (SSR hygiene, expired eval-gate window) are baseline failures, not introduced here; see the appended result.
  • iOS-side packet handling is untouched and unverified (no Xcode in this sandbox).

Assumptions

  • "The fixture used by the capture journey" means data/fixtures/packets/mini-arkit — the only packet test_capture_fixture_flow.py uses, and the one validate_fixtures.py already named explicitly (SD-43).
  • The manifest entries restate the hashes packet.json itself asserts (verified equal); the packet_dir entry's existing hash of packet.json is preserved untouched.
  • New item ids follow the mini-arkit-file-<path> pattern and sit adjacent to the existing mini-arkit entry, matching the manifest's identifier style; the manifest's existing (loose) ordering convention is unchanged.
  • Worked on the existing BaseScanning repo sandbox in a private fresh clone; used git add -f for manifest.json because backend/data is gitignored for real captures while data/fixtures/manifest.json is tracked.
  • No workflow or deploy config touched; never merged, never pushed to obv/burst5-20261015-bs-capture-fixture.

…st entries

The committed packet check ran with --no-files, so the keyframes and streams
under data/fixtures/packets/mini-arkit were never re-verified: a file that
left the repository passed make validate and only surfaced as a raw
FileNotFoundError inside the capture journey test. The manifest now names
the packet and all eleven files its packet.json references, and
validate_fixtures runs a guard that fails loudly, naming the missing or
drifted input, when the manifest, packet.json and disk disagree.

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f885d432-503c-4e4b-8a85-bcc82fbb74d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size:L 100-499 effective changed lines. label Oct 15, 2026
@obvious-autobuild

Copy link
Copy Markdown
Author

Full backend suite on this branch (ab2d086):

uv run --frozen --extra api pytest -q
5 failed, 1210 passed, 17 skipped, 7 warnings in 224.70s

The 5 failures are the pre-existing baseline, unchanged from the base branch: tests/test_ssr_overlap_check.py::test_repo_has_no_ssr_shingle_overlap (SSR hygiene) and 4 server/eval/tests/test_gates.py tests (the expired SD-96 known-failure window the inventory PR #248 recorded as follow-up 1). None touch the files this PR changes; the capture-journey and manifest-guard tests all pass.

@obvious-autobuild

Copy link
Copy Markdown
Author

Independent review (Obvious, Autobuild + Max)

Verdict: ready — no blocking findings. Everything in the body that could be checked reproduced exactly; nothing failed to reproduce.

What I verified

All commands run from backend/ in a fresh clone at ab2d086a.

  • The manifest restates the packet byte-for-byte. All 11 new kind: "file" items carry the same bytes/sha256 as the matching entries in mini-arkit/packet.json's files array (all 11 pairs compared); the mini-arkit packet_dir entry (9f59f921…, 13734 B) is untouched. Manifest diff vs base is 121 added / 0 deleted lines — purely additive. All 12 packet files are tracked and present in a fresh clone.
  • The motivating gap is real, and the guard closes it. On base 4e828aeb, with keyframes/k00003.jpg removed, uv run --frozen python scripts/validate_fixtures.py exits 0. At head the same state exits 1 with capture-journey fixture guard failed: mini-arkit: missing input data/fixtures/packets/mini-arkit/keyframes/k00003.jpg (manifest item mini-arkit-file-keyframes-k00003-jpg, packet.json /files/4) — and the pre-existing check-packets --no-files still prints ok in that state, confirming the guard is the only thing that catches it. Restoring the file returns exit 0.
  • The new tests bite. The 5-test file copied onto base errors 5/5 (capture_journey_errors does not exist there). Head code with the base manifest restored: 4/5 fail (committed-passes, missing-keyframe, missing-stream, tampered); the not-named-by-manifest test still passes, correctly, since the packet→manifest direction needs only the packet_dir entry. At head: uv run --frozen --extra api pytest -q tests/test_capture_fixture_manifest.py tests/test_capture_fixture_flow.py → 7 passed (5 new + 2 journey), matching the body.
  • Full suite uv run --frozen --extra api pytest -q → 5 failed, 1210 passed, 17 skipped in 211s, matching the appended comment exactly. The failures are the documented baseline (SSR hygiene + 4 eval-gate tests); the same two files give 5 failed / 95 passed on base, so they are not PR-caused.
  • Validate gate: uv run --frozen python scripts/validate_fixtures.py → exit 0, validate ok scenes=5 verdicts=16 labels=61 packets=36, matching the body's counts.
  • Lint: ruff check all checks passed; ruff format --check 461 files already formatted; lint-imports contracts 5 kept, 0 broken — all as claimed.
  • Scope and safety: 3 files (manifest, validate seam, tests), no workflow changes. The guard only adds checks: _score_self still exits 2 on a missing laser PLY and --full routes through the same _validate_committed. test_capture_fixture_flow.py is the packet's only consumer (copytree in fixture setup), so the body's "fails where it happens to touch the gap" description is accurate.

Non-blocking observations (no change requested)

  1. [low] The tests exercise capture_journey_errors() directly; the _validate_committed wiring (exit 1 on guard errors, backend/scripts/validate_fixtures.py:139-145) is covered only by the manual missing-file proof, not by a test. A regression that un-wires the guard would still surface via test_capture_fixture_flow's raw FileNotFoundError — just less legibly. Smallest fix if wanted: one test asserting the script exits 1 with capture-journey fixture guard failed: on a tampered copy.
  2. [low, boundary note] The guard checks every manifest item under packets/mini-arkit/ regardless of kind/committed (backend/scripts/validate_fixtures.py:75-96), so a future committed: false, store-backed item placed under that prefix would fail on fresh clones. Correct for today's shape — packet.json is the contract for that directory — but worth remembering if the packet ever grows store-backed files.

Could not verify

  • make validate-full / make criteria-golden and the laser/GPU fixture families (committed: false, fixture store unreachable from this sandbox) — the body already discloses this.
  • iOS-side packet handling (no Xcode here). CI on this branch remains the pre-existing repo-wide gap: label check + bots only, no backend workflow.

@SamGu-NRX
SamGu-NRX changed the base branch from obv/burst5-20261015-bs-capture-fixture to alpha October 16, 2026 04:30
@SamGu-NRX
SamGu-NRX marked this pull request as ready for review October 16, 2026 04:30
@SamGu-NRX
SamGu-NRX merged commit 0f9155b into alpha Oct 16, 2026
4 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T04:33:05.409062Z ab2d086 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codemetrics-ai

Copy link
Copy Markdown

Note

Review skipped

Codemetrics is installed on this repository, but it hasn't been added to your team yet.

To enable AI code reviews for this repo, go to your dashboard and add BaseScanning to your team's repositories.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] Adds validation for test fixture data integrity.

The PR appears safe to merge, with a non-blocking gap in the new fixture check.

Fix All in Claude CodeFindings

  1. P2 Stale packet details pass validation ▶
Fix with agent prompt
### Issue 1
backend/scripts/validate_fixtures.py:64-65
`capture_journey_errors` checks file bytes against `manifest.json`, but keeps only paths from `packet.json`. If a contributor updates a fixture file and its manifest hash without updating `packet.json`, the guard passes even though the packet declares the old size or hash. The earlier `--no-files` check cannot catch this either.

Compare each packet entry’s `bytes` and `sha256` with the matching manifest entry, and add a test for this disagreement.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR names all eleven capture-journey files in the fixture manifest and adds a guard to committed-fixture validation.

  • The capture fixture's files are checked before the journey runs.

Reviews (1) · Last reviewed commit: "test(backend): guard the capture-journey..." · Reviewed by Greptile

Comment on lines +64 to +65
for index, entry in enumerate(packet["files"]):
packet_files[entry["path"]] = index

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale packet details pass validation

capture_journey_errors checks file bytes against manifest.json, but keeps only paths from packet.json. If a contributor updates a fixture file and its manifest hash without updating packet.json, the guard passes even though the packet declares the old size or hash. The earlier --no-files check cannot catch this either.

Compare each packet entry’s bytes and sha256 with the matching manifest entry, and add a test for this disagreement.

Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/scripts/validate_fixtures.py
Line: 64-65

Comment:
**Stale packet details pass validation**

`capture_journey_errors` checks file bytes against `manifest.json`, but keeps only paths from `packet.json`. If a contributor updates a fixture file and its manifest hash without updating `packet.json`, the guard passes even though the packet declares the old size or hash. The earlier `--no-files` check cannot catch this either.

Compare each packet entry’s `bytes` and `sha256` with the matching manifest entry, and add a test for this disagreement.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant