[obvious-autobuild]

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

What changed and why

The ported backend (backend/server/api) had unit-style route tests and an
e2e uploader test that drives the flow through scripts/e2e_dev, but no test
that performs the device's own journey end to end over a committed packet
fixture. This adds backend/tests/test_capture_fixture_flow.py:

• It copies the committed data/fixtures/packets/mini-arkit packet (the
  smallest committed fixture that exercises real packet validation: 11 files
  with real bytes and hashes, arkit tier, streams/features/keyframe roles,
  conventions block) into a private per-test copy, so the tracked fixture is
  never mutated.
• It starts create_app() with memory state, local volume storage, static
  auth (HSS_AUTH_MODE=static with one device token) and a test
  UPLOAD_URL_HMAC_SECRET, matching the README's local-volume recipe.
• It then sends the same sequence the iOS app sends (the shapes in
  ios/HouseScanKit/Sources/HouseScanKit/Upload/CaptureAPI.swift): POST /v1/captures with Idempotency-Key, POST .../files registration, PUT
  bytes to the signed /v1/uploads/{token} URLs (no bearer, content type and
  MD5 from the signed headers), POST .../files:commit, POST .../finalize
  with raw packet.json plus X-Packet-Sha256, and GET .../events.
• Assertions: persisted bytes under captures/{captureId}/ for every file and
  captures/{captureId}/packet.json for the manifest; stored sha256 hashes
  equal the fixture hashes; capture status processing with
  filesCommitted == 11; events arrive in seq order with a
  files_committed event and a stage=validate, status=running event
  carrying the finalize runId. A second test skips one upload: commit
  reports the path in notFound, finalize returns 202 with
  status="awaiting_files" and missing=[path], the capture stays
  awaiting_files, and the skipped path is absent from storage.

In-process ASGI client vs launching uvicorn

The tests use Starlette's TestClient (in-process ASGI), not a spawned
uvicorn subprocess. The differences that matter here:

• What a subprocess would add: real socket/HTTP framing, uvicorn's chunked
  transfer and concurrency behavior, and process-level env wiring. None of
  those are in question at this stage — the port's risk is the app's route,
  state, storage and auth logic, not ASGI server plumbing, and the README's
  uvicorn recipe is already exercised manually per its local-volume section.
• What in-process gives: deterministic access to app.state.deps
  (drain_finalize, the storage backend, the state backend) so persisted
  bytes, hashes and status can be asserted directly, plus speed (the whole
  file runs in about one second) and no port/CI flakiness. It also covers the
  same middleware path a real request takes — create_app's _Access
  middleware buffers bodies, checks auth and caps sizes inside the ASGI app,
  not inside uvicorn.

For this stage (verifying the ingest contract and the fixture journey) the
in-process client is sufficient; the multipart 501 path and Modal deployment
remain outside its reach either way.

How I verified it

Worked in a fresh clone (~/work/burst5-20261015-bs-capture-fixture) on
obv/burst5-20261015-bs-capture-fixture from feat/port-house-scanning-server.

• uv run --extra api pytest tests/test_capture_fixture_flow.py -q → 2 passed
• uv run --extra api pytest tests/test_e2e_upload.py tests/test_capture_fixture_flow.py -q
  → 6 passed (existing upload suite green alongside the new test)
• uv run --extra api pytest server/api/tests -q → 123 passed
• uv run ruff check tests/test_capture_fixture_flow.py → All checks passed
• uv run ruff format --check . (backend) → 461 files already formatted
  (one reformat of the new file was applied first)
• Open PRs checked before starting: [obvious-fleet] Backend inventory after the port into backend/: checks run, one post-move path bug fixed #248–fix(backend): retryable finalize handoff and lease-guarded file-index merge #252 are the burst4 backend drafts
  (inventory, finalize state, orchestrator review, security, review page) —
  none adds a capture-fixture integration test, so this does not duplicate them.

What is not verified

• No real uvicorn subprocess was launched; the in-process path is asserted
  above. Socket-level behavior (chunked uploads, timeouts) is unverified.
• The orchestrator stages behind ingest are unavailable by design:
  HSS_INGEST_HOOKS=null (the README's default — the server.orchestrator
  stubs still raise), so after finalize the run is created and the capture
  reaches exactly processing with a validate stage event; no
  reconstruction/scene/criteria work runs and no /result outcome is
  produced. Multipart upload targets still answer 501 multipart_not_implemented.
• backend/tests/test_ssr_overlap_check.py::test_repo_has_no_ssr_shingle_overlap
  fails on this branch, but it fails on the unmodified base branch in a fresh
  clone too: it needs data/hygiene/ssr_shingles.sha256, which lives under the
  git-ignored data/ tree and is generated locally. Pre-existing, unrelated.
• iOS side: no Xcode here, so CaptureAPI.swift itself was only read as the
  request-shape contract, not compiled.

Assumptions

• mini-arkit is the intended fixture: the other committed packets under
  data/fixtures/packets/ are larger (320K–900K), and examples/ holds only
  packet JSON with no bytes on disk, so it cannot drive a real upload.
• The test reuses the shared ApiClient helper and token from
  server/api/tests/conftest.py for consistency with the suite's conventions.
• The packet copy is created under tmp_path per test; the tracked fixture is
  read-only input, per the task's "limited to the fixture flow test and its
  private fixture copy".
• Static auth uses a single device: principal, the smallest identity the
  app's journey needs.

[coderabbitai]

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: 70fd55b5-8b85-463d-b4da-5ef71799c7b6

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[obvious-autobuild]

Independent review (Obvious, Autobuild + Max)

Verdict: ready — the PR does what it claims, nothing regresses, and the tests bite on real-path mutations. Two low/non-blocking notes below.

Findings

1. [low] The finalized-bytes assertions cannot detect a change to the captures/ key layout — backend/tests/test_capture_fixture_flow.py:176 and :179 read storage through final_key(...)/packet_key(...) — the same helpers the app writes with — so writer and reader mutate together. Evidence: mutating final_key to return captures/{capture_id}/moved/{path} (backend/server/api/keys.py:9) leaves both new tests green, and running pytest server/api/tests tests -q under that mutation gives 193 passed with only the pre-existing SSR failure — nothing in the run suite pins the literal prefix. That prefix is a real contract (keys.py's own docstring: "Uploads expire; captures/ is what every later stage reads"), and the body claims to assert "persisted bytes under captures/{captureId}/". Smallest fix: pin it once before line 176, e.g. assert final_key(capture_id, entry["path"]).startswith(f"captures/{capture_id}/"), or read one file through storage.get_bytes(f"captures/{capture_id}/{path}") directly.

2. [nit] Body count off by one — the body reports ruff format --check . → "461 files already formatted"; with ruff 0.16.9 (the dev pin) I measure 460 on head 4e828aeb. Immaterial; likely the author's worktree counted an extra file.

3. [context, pre-existing] backend/** has no CI workflow — every workflow under .github/workflows/ filters by paths and none includes backend/** (server.yml covers server/** only), so this test — and the whole ported backend suite — never runs in CI; the PR's checks are label/bot only. Not this PR's defect. Smallest fix when someone picks it up: add backend/** to server.yml's paths or a dedicated backend.yml.

What I verified (fresh clone at head 4e828aeb, base parent 57f57ff8 = tip of origin/feat/port-house-scanning-server)

• PR claims, run exactly as written: uv run --extra api pytest tests/test_capture_fixture_flow.py -q → 2 passed; ... pytest tests/test_e2e_upload.py tests/test_capture_fixture_flow.py -q → 6 passed; ... pytest server/api/tests -q → 123 passed; uv run ruff check tests/test_capture_fixture_flow.py → All checks passed. All four match the body.
• Fixture: data/fixtures/packets/mini-arkit has 11 packet files + packet.json on disk (55K, tier arkit, roles stream×5/keyframe×4/features×2) — matches the body's description, and it is indeed the smallest committed packet with real bytes (yt-12Ks1idWj40 is packet JSON only, no media).
• No regression: full backend suite (uv run --extra api pytest -q) on head = 5 failed / 1205 passed / 17 skipped, on base = 5 failed / 1203 passed / 17 skipped. Failure sets are byte-identical (tests/test_ssr_overlap_check.py + 4 × server/eval/tests/test_gates.py; the SSR test needs the git-ignored data/hygiene/ssr_shingles.sha256, absent in fresh clones — the body's "pre-existing, unrelated" claim is accurate). Head adds exactly +2 passes: the two new tests.
• Tests exercise the real path (mutation checks — edit, run, git checkout -- restore; tree clean after each):
  • commit treats a never-PUT file as committed (server/api/routes/files.py:306-308) → test_finalize_reports_files_that_never_uploaded FAILS.
  • finalize marks a capture with missing files processing (server/api/finalize.py:297) → same test FAILS.
  • final_key → captures/{capture_id}/moved/{path} (server/api/keys.py:9) → 2 passed, not caught (finding 1).
• Shapes match the routes I read: create 201 with Idempotency-Key; commit response committed/notFound/mismatch; PUT checks Content-Type/Content-MD5 against the HMAC token and needs no bearer (app.py _needs_auth exempts /v1/uploads/); finalize 202 with Idempotency-Key == packetId + X-Packet-Sha256; events seq/next polling. The body's middleware claim is accurate — _Access buffers bodies, checks auth and caps sizes inside the ASGI app, so in-process TestClient covers it.
• iOS contract: CaptureUploader.swift:439 and :591 set Idempotency-Key, :592 sets X-Packet-Sha256 — the test's request shapes match the app's.
• No duplication: the only other mini-arkit test at head (server/api/tests/test_replay_mini_arkit.py) exercises replay metadata, not the device upload journey.

What I couldn't verify

• Socket-level behavior (real uvicorn, chunked transfer, timeouts) — the tests are in-process by design; the body discloses this honestly.
• iOS compile/runtime (no Xcode here); CaptureAPI.swift/CaptureUploader.swift were read as the request-shape contract only.
• The author's "461 files" format count (I measure 460, finding 2).

[chatgpt-codex-connector]

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:32:34.688129Z	4e828ae	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]

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]

Confidence Score: 4/5

[Low impact] Adds an end-to-end test for the capture flow.

The PR appears safe to merge, with one non-blocking gap in the event assertion.

Findings

1.  Wrong progress status passes ▶

Fix with agent prompt

### Issue 1
backend/tests/test_capture_fixture_flow.py:188-192
The event assertion checks `stage` and `runId` but never checks `status`. A `validate` event with a missing or wrong `status` would pass, so the test does not prove that the phone receives the expected running event. Add a check for `status == "running"` to the same predicate.

```suggestion
    assert any(
        (event.get("data") or {}).get("stage") == "validate"
        and (event.get("data") or {}).get("status") == "running"
        and (event.get("data") or {}).get("runId") == payload["runId"]
        for event in stage_events
    ), stage_events
```

---

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

Summary

Adds two in-process capture journey tests using a private copy of the committed mini-arkit packet.

• A backend test now follows a real packet through capture upload.

_{Reviews (1) · Last reviewed commit: "test(backend): drive the full capture jo..." · Reviewed by Greptile}

[greptile-apps]

Wrong progress status passes

The event assertion checks stage and runId but never checks status. A validate event with a missing or wrong status would pass, so the test does not prove that the phone receives the expected running event. Add a check for status == "running" to the same predicate.

Suggested change

	assert any(
	(event.get("data") or {}).get("stage") == "validate"
	and (event.get("data") or {}).get("runId") == payload["runId"]
	for event in stage_events
	), stage_events
	assert any(
	(event.get("data") or {}).get("stage") == "validate"
	and (event.get("data") or {}).get("status") == "running"
	and (event.get("data") or {}).get("runId") == payload["runId"]
	for event in stage_events
	), stage_events

Prompt To Fix With AI

This is a comment left during a code review.
Path: backend/tests/test_capture_fixture_flow.py
Line: 188-192

Comment:
**Wrong progress status passes**

The event assertion checks `stage` and `runId` but never checks `status`. A `validate` event with a missing or wrong `status` would pass, so the test does not prove that the phone receives the expected running event. Add a check for `status == "running"` to the same predicate.

```suggestion
    assert any(
        (event.get("data") or {}).get("stage") == "validate"
        and (event.get("data") or {}).get("status") == "running"
        and (event.get("data") or {}).get("runId") == payload["runId"]
        for event in stage_events
    ), stage_events
```

---

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