Repository navigation
[obvious-fleet] test(backend): prove the finalize-to-orchestrator handoff over the API - #259
Conversation
HSS_INGEST_HOOKS=auto (the documented dev hook mode) drives two API-level tests over the committed mini-arkit packet fixture with stage stubs: finalize lands the run's manual_review outcome on the capture, and a controlled scale stage exhaustion lands as a failed event plus recorded failure data instead of a 500.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
Independent review (Obvious, Autobuild + Max) Verdict: Findings (ranked; none blocking)1. Low — head commit is missing the workspace's 2. Low (pre-existing, surfaced by this test) — intermittent un-awaited-coroutine warning in the prewarm path. What I verifiedEnvironment: fresh clone of the repo at head Commands and results:
The tests exercise the real path — mutation evidence (each mutation applied locally, suite run, tree restored):
Body claims spot-checked against source:
What I couldn't verify
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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. |
[Low impact] The PR appears safe to merge.
|
fleet-packet: burst5-20261015-bs-hooks-integration
What changed and why
One new test file,
backend/server/api/tests/test_hooks_orchestrator.py; no production code changed. It proves the handoff from API finalize to the configured local orchestrator end to end, at the HTTP boundary.Synchronous finalize vs the existing handoff.
accept_packet(server/api/finalize.py) does only the fast checks inline — schema, ids, manifest-vs-committed-files — thenschedule_finishhands the rest to a daemon thread (_spawn) so the 202 does not wait. On that thread,finish_readypromotes the files and, once per run id, callssafe_hook("on_finalized", ...). Which hook that is, is decided byresolve_hooks(server/api/hooks.py):HSS_INGEST_HOOKS=nullreturnsNullHooks, so the thread promotes and validates but no run ever exists;HSS_INGEST_HOOKS=auto— the documented dev setting (SD-73: dev deploy runs it withHSS_ORCH_STAGE_STUBS=1) — returnsOrchestratorHooks, whose_bindinstalls the API's state and storage as orchestrator services (services.bind) and calls the realserver.orchestrator.start_run.start_runclaims the run, writes the run/stage records into the API's state store, and drives the eleven stages through the coordinator inline (stubbed stages here), landing the outcome status and events on the capture the API serves.Existing coverage and the gap. Every API test builds its client with
HSS_INGEST_HOOKS="null";api/tests/test_seams.pyproves only the bind contract with a monkeypatchedstart_run;orchestrator/tests/test_local_fixture_proof.pyproves the orchestrator CLI entry point, not the API seam. No test ran the realresolve_hooks → OrchestratorHooks → start_run → coordinatorchain behind a finalize request. This closes that gap. Open PRs #251 (orchestrator review fixes) and #252 (retryable finalize handoff) touch production code in neighboring areas; this change duplicates neither.How you verified it
All commands run in
backend/withuv run --frozen --extra api, memory/local backends, stage stubs,VISION_EXTERNAL_CALLS=off— no Modal, GPU, network or credentials:pytest server/api/tests/test_hooks_orchestrator.py -q→ 2 passedpytest server/api/tests -q→ 125 passed (no existing test regressed)pytest server/orchestrator/tests -q→ 156 passedruff check server/api/tests/test_hooks_orchestrator.py→ clean;ruff format --check→ cleanlint-imports→ Contracts: 5 kept, 0 brokenThe tests drive the committed
data/fixtures/packets/mini-arkit/packet.jsonthrough the real FastAPI app with the documented hook mode: create capture, register/PUT/commit all 11 files,POST /finalize, then poll/eventsand/result. Nothing about the hook is mocked — the production seam (resolve_hooks,_bind,start_run, coordinator, outcome table) is the code under test.Observable success state (
HSS_INGEST_HOOKS=auto, stub verdictmanual_review, outcome table row 14): finalize returns 202processingwith the deterministicrunId(run_+ 16 hex of sha256(capture_id + NUL + packet bytes)); the capture lands onmanual_review— equal to the run'soutcome_kind, not stuck onprocessing; theresultstage is done; the event feed ends with exactly one terminalverdict_ready;GET /resultreturns 200 withoutcome.kind == "manual_review"and the same run id.Observable failure state (the orchestrator's supported test knob
HSS_ORCH_TEST_FAIL_STAGE=scale:compute_error:2, the same knob the orchestrator fixture proof uses): finalize still returns a 202, never a 500 — the failure lands off the request path as recorded data. Row 3 of the outcome table applies: exactly onefailedevent withcode=stage_exhausted:scale:compute_errorandnext=manual_review, run statusfailedwith that failure code, capture statusmanual_review, andGET /resultstill serves the manual result.What is not verified
modal_dictstate backends: outside the stub path this task asked for.Assumptions
mini-arkit(formatVersion 0.4, API-valid, manifest-clean) rather than the orchestrator proof'ssynthetic-meter.tar: that tar'spacket.jsonpredates the API wire contract (noformatVersion, no session block) and finalize would reject it with a 422 before any handoff could be exercised.safe_hookdeliberately swallows hook exceptions (that is its own contract); stage exhaustion exercises the documented failure landing the task asked for.obv/burst5-20261015-bs-hooks-integrationintoobv/burst5-20261015-bs-orchestrator-fixture.Co-authored-by: obvious-autobuild[bot]trailer; adding it after the push would require a force-push, which this task forbids, so the commit is left as pushed.