[obvious-autobuild]

Stacked on #247 (targets its head branch). Addresses the four findings from the Greptile apex review; each is pinned by a regression test in server/orchestrator/tests/test_greptile_followup.py.

Fixes

1. Amendment runs hydrated the base packet (artifacts.py:202) — start_run accepted packet_uri but RunRecord dropped it and hydrate_packet always fetched captures/{capture_id}/packet.json, so an amendment run ran against the original packet and manifest-listed files resolved under the wrong prefix. RunRecord now stores packet_uri, stage_context passes it through, and _packet_file_jobs derives the storage prefix from the packet's own key (captures/{cap}/amendments/{amd}/ for amendments).

2. Device confirmations never applied (api.py:173) — on_confirmation passed the capture_id string where confirm() expects a Reads record; every call raised inside safe_hook while the endpoint returned success. The hook now loads the newest run's reads/reads.json via the stage-artifact overlay, applies the answer with confirm(), and persists the updated record so later results read the correction.

3. Retries started already timed out (coordinator.py:266) — _spawn_one reset started_at but left the previous attempt's entered_at, which _sweep_timeouts uses as the attempt budget, so a replacement worker could be cancelled immediately. _spawn_one clears entered_at when a new attempt begins; the worker's first heartbeat re-sets it on stage entry.

4. Final status read the wrong rules profile (coordinator.py:571) — _apply_success always read verdicts/verdict.C.json even when the run selected profile A/B/S. It now reads the run's own profile verdict (C fallback) before deciding the outcome.

Checks

• make lint clean (import-boundary contracts kept).
• Full suite: 1206 passed; the 5 remaining failures (tests/test_ssr_overlap_check.py, 4 × server/eval/tests/test_gates.py) reproduce on the pristine base commit and are unrelated — verified by stashing this change.
• The 4 new regression tests fail on the unfixed code and pass with it; jobs.v0.schema.json regenerated for the new packet_uri field.

Authored by Sam Gu 127461594+SamGu-NRX@users.noreply.github.com.

[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: 5b038487-964b-44d3-830a-cd59f2ec467c

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.}

[SamGu-NRX]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review skipped.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[obvious-autobuild]

Independent review (Obvious, Autobuild + Max)

I reviewed this draft as an outside reader: read the body, comments and full diff, checked out head 13f9f4a6 locally (base 2d0911f8), re-derived each claimed finding from the base, ran the tests, and dry-run merged against #250.

Verdict: ready

The PR does what the body claims, the four regression tests genuinely pin their findings, and I found no regression. Findings below are minor; none block.

Findings (ranked by severity)

1. Minor — on_confirmation read-modify-write can lose a correction under concurrency (backend/server/orchestrator/api.py:169-185). The hook loads reads/reads.json, applies confirm(), and put_bytes the whole record back with no lock; two near-simultaneous confirmations, or a concurrent reads-stage write of the same key, can silently drop one update. Not a regression — the base code never applied corrections at all — and the memory/local backends are dev-scoped. Smallest fix if you want it: wrap the load-apply-store in the same per-capture lock the state backend uses for event appends.
2. Minor (process) — missing co-author trailer. Commit 13f9f4a6 is authored Obvious <obvious@obvious.ai> without the Co-authored-by: Sam Gu <sgu07966@gmail.com> trailer this repo's convention asks for; the body's "Authored by Sam Gu" line covers intent, but the trailer makes attribution machine-readable.
3. Note — prefix derivation is silent about malformed packet_uri. _packet_file_jobs builds the file prefix as packet_key.rsplit("packet.json", 1)[0] or f"captures/{capture_id}/" (artifacts.py:260). Safe today — packet_uri is server-derived (api/keys.py:16) and always ends in packet.json, and _safe_rel blocks traversal — but a key not ending in packet.json would quietly produce a wrong prefix instead of failing. An assertion there would pin the invariant.

Notes, not findings: the hook resolves reads from the newest run only, so if the newest run has no reads/reads.json (the reads stage is optional=True, graph.py:119) a confirmation is dropped with a 2xx — strictly better than base, which dropped all of them. And #250 also edits hydrate_packet: the conflict is textual, not semantic. Both drafts cut from the same base tip (2d0911f8); git merge-tree conflicts in artifacts.py only over the docstring/early-return lines. #250's tombstone early-return happens before any download; #251's packet_key only changes which keys are downloaded. Whichever merges second rebases trivially.

What I verified (commands + results)

• Body claims 1-4 vs code: each fix present at the cited line; start_run already accepted packet_uri on base and dropped it; confirm()'s signature takes reads: Reads (base call passed a capture_id string → always raised); _sweep_timeouts skips entered_at=None and both state backends set entered_at on the stage-start substep, so clearing it is safe; the criteria stage writes per-profile verdicts/verdict.{A,B,C,S}.json (stubs/criteria.py:17, graph.py:153-156) and result.py:85 already reads the run's own profile — the coordinator now matches it.
• Lint/schemas (the make lint targets): ruff check pass; ruff format --check 460 files clean; lint-imports 5 kept / 0 broken; python -m server.contracts.export_schemas --check exit 0 — the committed schema matches a fresh export.
• Touched suites: pytest server/orchestrator/tests server/contracts server/reads → 268 passed.
• Regression tests fail without the fix: git checkout 2d0911f8 -- the five fix files (test file kept) → test_greptile_followup.py 4 failed (distinct failure modes: RunRecord rejects unknown packet_uri; stale entered_at observed on the retry; confirmation not persisted; capture not needs_views). Restored head → 4 passed. (I reverted all fixes together, not one-by-one; each test's failure mode is distinct enough to attribute.)
• Full suite: pytest -q on the head → 1207 passed, 17 skipped, 5 failed in 5m33s.
• The 5 failures are pre-existing, as the body claims: the same five tests (test_ssr_overlap_check.py + 4 × test_gates.py) fail identically on the pristine base commit in a separate worktree (5 failed / 95 passed in those two files). Unrelated to this PR.
• CI: this repo runs no test/lint CI on PRs (only a size labeler) — all evidence above is from my local run of the head.

What I could not verify

• The production (non-stub) criteria path — the per-profile verdict writer was verified via the stub, the stage graph, and result.py, not executed end-to-end (needs weights/fixtures).
• Modal/GCS/R2 backends (state/storage variants were read, not run).
• The body reports 1206 passed; my run observed 1207 — one-test drift between environments, immaterial since both runs show the identical five failures. My environment: Python 3.12.15 via uv sync --frozen --extra api.

[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.

[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:31:51.164386Z	13f9f4a	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.

[greptile-apps]

Confidence Score: 2/5

[High impact] Modifies orchestrator packet handling and confirmation flow.

Fix amendment filename collisions and confirmation timing gaps before merging.

Findings

1.  Amendments still use old photos ▶
2.  Early corrections never reach analysis ▶
3.  Results ignore later corrections ▶

Fix with agent prompt

### Issue 1
backend/server/orchestrator/artifacts.py:280
When an amendment reuses a base filename such as `stills/meter.jpg`, `_packet_file_jobs` copies the base image and adds its path to `seen`. The manifest loop then skips that filename before using the new amendment prefix. The replacement image is copied under `amendments/{id}/` instead of the path named in the manifest, so the worker analyzes the original photo.

Resolve committed records relative to the selected packet. Add a regression test with matching base and amendment filenames.

### Issue 2
backend/server/orchestrator/api.py:169-171
The endpoint accepts confirmations during upload and processing, but this hook returns when `reads/reads.json` does not exist. Nothing applies the saved answer when the reads stage finishes. Retrying the same request also returns before calling the hook, so a successful correction can leave the later analysis using the original value.

Apply saved confirmations when reads are written, and test a confirmation submitted before the run starts.

### Issue 3
backend/server/orchestrator/api.py:184-185
Captures in `needs_views` or `manual_review` still accept confirmations, but an answer submitted after the result is built changes only `reads/reads.json`. The result endpoint serves the existing `result/result.json`, whose meter value was copied from reads earlier. This hook neither rebuilds that result nor reruns criteria that already used the old reads, so users still see the old value after a successful correction.

Refresh the affected outputs after applying the answer, and test a correction followed by a result fetch.

---

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

Summary

The PR stores each run's packet location, applies confirmations to saved reads, resets retry timing, and selects the run's verdict profile.

• Amendment runs load files from their own packet.
• Device confirmations update the newest run’s saved reads record.
• Each retry starts with a fresh stage timeout clock.
• Final status follows the run’s selected rules profile.

obvious-autobuild[bot] reports that five unrelated suite failures also occur on the base commit.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Confirmation accepted] --> B{Reads exist?}
  B -->|No| C[Answer saved but not replayed]
  B -->|Yes| D[Update reads]
  D --> E{Result already built?}
  E -->|No| F[Later stages can read correction]
  E -->|Yes| G[Saved result keeps old value]

Loading

_{Reviews (1) · Last reviewed commit: "fix(backend): address Greptile review fi..." · Reviewed by Greptile}

[greptile-apps]

Amendments still use old photos

When an amendment reuses a base filename such as stills/meter.jpg, _packet_file_jobs copies the base image and adds its path to seen. The manifest loop then skips that filename before using the new amendment prefix. The replacement image is copied under amendments/{id}/ instead of the path named in the manifest, so the worker analyzes the original photo.

Resolve committed records relative to the selected packet. Add a regression test with matching base and amendment filenames.

Prompt To Fix With AI

This is a comment left during a code review.
Path: backend/server/orchestrator/artifacts.py
Line: 280

Comment:
**Amendments still use old photos**

When an amendment reuses a base filename such as `stills/meter.jpg`, `_packet_file_jobs` copies the base image and adds its path to `seen`. The manifest loop then skips that filename before using the new amendment prefix. The replacement image is copied under `amendments/{id}/` instead of the path named in the manifest, so the worker analyzes the original photo.

Resolve committed records relative to the selected packet. Add a regression test with matching base and amendment filenames.

---

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

[greptile-apps]

Early corrections never reach analysis

The endpoint accepts confirmations during upload and processing, but this hook returns when reads/reads.json does not exist. Nothing applies the saved answer when the reads stage finishes. Retrying the same request also returns before calling the hook, so a successful correction can leave the later analysis using the original value.

Apply saved confirmations when reads are written, and test a confirmation submitted before the run starts.

Prompt To Fix With AI

This is a comment left during a code review.
Path: backend/server/orchestrator/api.py
Line: 169-171

Comment:
**Early corrections never reach analysis**

The endpoint accepts confirmations during upload and processing, but this hook returns when `reads/reads.json` does not exist. Nothing applies the saved answer when the reads stage finishes. Retrying the same request also returns before calling the hook, so a successful correction can leave the later analysis using the original value.

Apply saved confirmations when reads are written, and test a confirmation submitted before the run starts.

---

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

[greptile-apps]

Results ignore later corrections

Captures in needs_views or manual_review still accept confirmations, but an answer submitted after the result is built changes only reads/reads.json. The result endpoint serves the existing result/result.json, whose meter value was copied from reads earlier. This hook neither rebuilds that result nor reruns criteria that already used the old reads, so users still see the old value after a successful correction.

Refresh the affected outputs after applying the answer, and test a correction followed by a result fetch.

Prompt To Fix With AI

This is a comment left during a code review.
Path: backend/server/orchestrator/api.py
Line: 184-185

Comment:
**Results ignore later corrections**

Captures in `needs_views` or `manual_review` still accept confirmations, but an answer submitted after the result is built changes only `reads/reads.json`. The result endpoint serves the existing `result/result.json`, whose meter value was copied from reads earlier. This hook neither rebuilds that result nor reruns criteria that already used the old reads, so users still see the old value after a successful correction.

Refresh the affected outputs after applying the answer, and test a correction followed by a result fetch.

---

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