feat(dashboard): show in-flight progress and the deadline a call ran under #94

Merged

rcsheets merged 1 commit from feat/inflight-pass-progress into main

2026-08-03 11:03:20 +00:00

rcsheets commented

2026-08-03 11:00:36 +00:00

Owner

Rows are now opened when a call is issued and closed when it returns
(migration 000014 adds status/started_at; tracker.RecordPass becomes an
upsert on the pass id). repocontext reports its discovery and extract
calls through a new Inputs.OnPass observer for the same reason a list
handed back at the end could not serve: the value is in seeing the call
that is currently out. Result.Passes went away with its last consumer.

The page grows a pipeline strip covering stages that haven't started —
"where is this review right now" is not answerable from per-call rows
alone — an in-progress banner, live elapsed counters, and a 5s refresh
that stops once nothing is running. Configs mid-call get a column: shadow
event rows aren't minted until their outcome lands, so a fan-out was
rendering one column while a second model was still working.

Every call also records the deadline it ran under, the smaller of the
HTTP client timeout and the remaining context deadline (both are real
bounds and "context deadline exceeded" names neither). A timeout now
reads "Failed after 120.0s against a 120.0s limit" instead of leaving the
limit to be inferred from the duration. Calls well under their deadline
keep it in a tooltip rather than inline, where it would be clutter.

A row left open by a process that died looks the same as a slow one in
the store, so age separates them: past its deadline plus a grace period a
call is reported as never having answered, and the page stops polling for
a result that is not coming.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

The comparison page could only describe a review after it was over. Pass rows were written when a call returned, so while a review ran the page showed a finished review's layout filled with zeroes — 0+0 tokens, 0.0s — which reads as a review that produced nothing rather than one that hasn't answered yet. A hung review told you nothing about which of its nine calls was hanging. Rows are now opened when a call is issued and closed when it returns (migration 000014 adds status/started_at; tracker.RecordPass becomes an upsert on the pass id). repocontext reports its discovery and extract calls through a new Inputs.OnPass observer for the same reason a list handed back at the end could not serve: the value is in seeing the call that is currently out. Result.Passes went away with its last consumer. The page grows a pipeline strip covering stages that haven't started — "where is this review right now" is not answerable from per-call rows alone — an in-progress banner, live elapsed counters, and a 5s refresh that stops once nothing is running. Configs mid-call get a column: shadow event rows aren't minted until their outcome lands, so a fan-out was rendering one column while a second model was still working. Every call also records the deadline it ran under, the smaller of the HTTP client timeout and the remaining context deadline (both are real bounds and "context deadline exceeded" names neither). A timeout now reads "Failed after 120.0s against a 120.0s limit" instead of leaving the limit to be inferred from the duration. Calls well under their deadline keep it in a tooltip rather than inline, where it would be clutter. A row left open by a process that died looks the same as a slow one in the store, so age separates them: past its deadline plus a grace period a call is reported as never having answered, and the page stops polling for a result that is not coming. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

rcsheets added 1 commit

2026-08-03 11:00:36 +00:00

feat(dashboard): show in-flight progress and the deadline a call ran under

ci / check (pull_request) Successful in 41s

Details

b9ef54e56b

The comparison page could only describe a review after it was over. Pass
rows were written when a call returned, so while a review ran the page
showed a finished review's layout filled with zeroes — 0+0 tokens, 0.0s —
which reads as a review that produced nothing rather than one that hasn't
answered yet. A hung review told you nothing about which of its nine
calls was hanging.

Rows are now opened when a call is issued and closed when it returns
(migration 000014 adds status/started_at; tracker.RecordPass becomes an
upsert on the pass id). repocontext reports its discovery and extract
calls through a new Inputs.OnPass observer for the same reason a list
handed back at the end could not serve: the value is in seeing the call
that is currently out. Result.Passes went away with its last consumer.

The page grows a pipeline strip covering stages that haven't started —
"where is this review right now" is not answerable from per-call rows
alone — an in-progress banner, live elapsed counters, and a 5s refresh
that stops once nothing is running. Configs mid-call get a column: shadow
event rows aren't minted until their outcome lands, so a fan-out was
rendering one column while a second model was still working.

Every call also records the deadline it ran under, the smaller of the
HTTP client timeout and the remaining context deadline (both are real
bounds and "context deadline exceeded" names neither). A timeout now
reads "Failed after 120.0s against a 120.0s limit" instead of leaving the
limit to be inferred from the duration. Calls well under their deadline
keep it in a tooltip rather than inline, where it would be clutter.

A row left open by a process that died looks the same as a slow one in
the store, so age separates them: past its deadline plus a grace period a
call is reported as never having answered, and the page stops polling for
a result that is not coming.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

pr-reviewer-bot commented

2026-08-03 11:00:38 +00:00

Collaborator

Automated review by pr-reviewer v0.43.0 | Safety Check | Claude Sonnet 5 | tracking id r-7074d6-9592b0
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

⚠️ The primary review (Mistral Small) failed. The review below is from Claude Sonnet 5, which reviewed the same diff.

✅ Verdict: LGTM — The PR is a large, well-tested feature addition with consistent id-based upsert semantics, safe additive migrations, and no exposed secrets or obvious logic errors.

Reviewed the diff for obvious bugs, security issues, and accidentally committed content.

Observations:

The migration (000014) is additive (ADD COLUMN IF NOT EXISTS) with a sensible down migration; backfill logic for status/started_at looks correct and non-destructive.
RecordPass moves from ON CONFLICT (id) DO NOTHING to a real upsert with an explicit column list in DO UPDATE SET, and deliberately excludes started_at/comparison_group_id from the update — consistent with the stated intent (row's original start time is preserved). The passCols placeholder count was bumped in lockstep and the corresponding test constant (16 → 19) updated, so no mismatch there.
fanOut now writes an opening pass row before the review call runs, keyed by a pre-generated passID, and recordReviewPass reuses that same ID to close it — matches the upsert semantics. Tests (TestPassRowIsOpenedBeforeTheCall, passFromRecord tests) explicitly check the id continuity, which is the correct thing to protect given the upsert-by-id design.
EffectiveTimeout correctly takes min(HTTP client timeout, remaining context deadline) and falls back sanely when there's no context deadline.
splitPasses/buildStages/staleness logic have reasonable guards: negative elapsed clamps to 0 (clock skew), no deadline recorded never marks stale, grace period around the deadline avoids false "abandoned" reports. All covered by direct unit tests.
No secrets, credentials, or large binaries appear in the diff. No obviously dangerous SQL (all parameterized). No auth/authz code touched.
repocontext.Result.Passes removal and the shift to Inputs.OnPass is a clean refactor with its only consumer (runner.go) updated in the same PR; recordPass/resultMu locking around shared result mutation still guards recordSkipped, and pass recording moved to the callback which doesn't touch res at all, so no lock is needed there — consistent.
The refresh meta-tag and elapsed-time JS ticking are template-only, low risk; head block was added to layout.html for other pages to opt into, doesn't break existing pages since it's guarded by {{if .InFlight}}.
One minor logic note, not alarming: buildStages' State for a stage can end up "running" even if some of its calls in that kind failed and one is still running — that seems intended per the switch statement ordering (running takes priority), matches the test TestBuildStagesMarksUnstartedStagesPending.

This is a large but well-tested, internally consistent PR (dashboard/live-progress feature). I don't see anything that looks like an accidental commit, an obvious bug, or a security issue.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.43.0 | Safety Check | Claude Sonnet 5 | tracking id `r-7074d6-9592b0`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — The PR is a large, well-tested feature addition with consistent id-based upsert semantics, safe additive migrations, and no exposed secrets or obvious logic errors. Reviewed the diff for obvious bugs, security issues, and accidentally committed content. Observations: - The migration (000014) is additive (ADD COLUMN IF NOT EXISTS) with a sensible down migration; backfill logic for `status`/`started_at` looks correct and non-destructive. - `RecordPass` moves from `ON CONFLICT (id) DO NOTHING` to a real upsert with an explicit column list in `DO UPDATE SET`, and deliberately excludes `started_at`/`comparison_group_id` from the update — consistent with the stated intent (row's original start time is preserved). The `passCols` placeholder count was bumped in lockstep and the corresponding test constant (16 → 19) updated, so no mismatch there. - `fanOut` now writes an opening pass row before the review call runs, keyed by a pre-generated `passID`, and `recordReviewPass` reuses that same ID to close it — matches the upsert semantics. Tests (`TestPassRowIsOpenedBeforeTheCall`, `passFromRecord` tests) explicitly check the id continuity, which is the correct thing to protect given the upsert-by-id design. - `EffectiveTimeout` correctly takes min(HTTP client timeout, remaining context deadline) and falls back sanely when there's no context deadline. - `splitPasses`/`buildStages`/staleness logic have reasonable guards: negative elapsed clamps to 0 (clock skew), no deadline recorded never marks stale, grace period around the deadline avoids false "abandoned" reports. All covered by direct unit tests. - No secrets, credentials, or large binaries appear in the diff. No obviously dangerous SQL (all parameterized). No auth/authz code touched. - `repocontext.Result.Passes` removal and the shift to `Inputs.OnPass` is a clean refactor with its only consumer (`runner.go`) updated in the same PR; `recordPass`/`resultMu` locking around shared result mutation still guards `recordSkipped`, and pass recording moved to the callback which doesn't touch `res` at all, so no lock is needed there — consistent. - The refresh meta-tag and elapsed-time JS ticking are template-only, low risk; `head` block was added to layout.html for other pages to opt into, doesn't break existing pages since it's guarded by `{{if .InFlight}}`. - One minor logic note, not alarming: `buildStages`' `State` for a stage can end up "running" even if some of its calls in that kind failed and one is still running — that seems intended per the switch statement ordering (running takes priority), matches the test `TestBuildStagesMarksUnstartedStagesPending`. This is a large but well-tested, internally consistent PR (dashboard/live-progress feature). I don't see anything that looks like an accidental commit, an obvious bug, or a security issue.

rcsheets merged commit 100c613a4e into main

2026-08-03 11:03:20 +00:00

rcsheets deleted branch feat/inflight-pass-progress

2026-08-03 11:03:20 +00:00

rcsheets referenced this pull request from a commit

2026-08-03 11:03:21 +00:00

Merge pull request 'feat(dashboard): show in-flight progress and the deadline a call ran under' (#94) from feat/inflight-pass-progress into main