Hub truth fixes: events-fallback timeline, honest integrity count, Proof Rail names its run (#496, #497, #498) - #511
Conversation
The timeline section read only the derived collections (run.timeline / run.activityTimeline, populated for index entries since #296), so legacy runs, fixture-shaped runs, and index entries predating the field declared "no recorded timeline events" while events.jsonl sat populated in the same snapshot — the Live Feed rendered those exact events one page over. readableTimeline now derives a minimal timeline from events when the derived collections are empty (ts/type/task_id/stage_id, time-ordered, capped to the most recent 120 — the index's own timeline cap), and the section carries its source. The page labels the fallback ("Derived from events.jsonl — this run has no persisted timeline"), matching the 3D Studio's "events.jsonl projection" honesty convention. Derived timelines keep precedence (no event double-merge) and the honest empty state remains when both sources are empty — all pinned, verified failing first. Closes #496. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
buildDiagnostics aggregated integrity detail over fully-parsed runs only; index-served runs carry the has_integrity_errors boolean (persisted at INDEX_VERSION 5 by the #316 lite-full parity fix) but not the integrity[] detail array — so a damaged run served from the rollup index contributed nothing, and Diagnostics read "Data integrity errors: 0" with the damaged run in plain view (observed live in the 2026-07-30 tour). The #316 class one level deeper: the badge boolean made it into the index, the detail array feeding Diagnostics did not. Index-served damaged runs now count and appear in the problem list naming their run, with an honest entry that per-file detail requires a full parse ("rollup index entry — run reports damaged files"). Fully-parsed detail is unchanged and never double-counts via its own boolean. Pinned by unit cases plus a real two-build round-trip (first build parses fully and persists the index, the second serves the damaged run from it — the exact path that reported zero), verified failing first. Persisting the compact detail into index entries rides the next INDEX_VERSION bump, noted on the issue. Closes #497. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
At all-runs scope the hero cited one run's readiness verdict ("blocked,
coverage 85%") while the Proof Rail silently rendered a DIFFERENT run's
stages — 15 unqualified "Not Started" beside a non-zero coverage verdict
read as the dashboard disagreeing with itself (observed live in the
2026-07-30 tour: the verdict cited run-fx-blocked, the rail showed
run-fx-stale).
Two server-owned changes in the overview projection: focusRun now prefers
the run the verdict's first blocker cites (when that run is in scope), so
the rail reflects the run the hero is talking about; and the projection
exposes stagesSource {runId, goal} so the rail labels which run it shows
("Stage proof from <goal> · <runId>") — pages render the label, they
never re-derive it. No-blocker verdicts keep the existing focus
selection, citations outside the scope fall back safely, and no-runs
scopes invent nothing — all pinned, verified failing first.
Live-verified at all-runs scope: verdict, goal chip, and rail now speak
about the same run, with the source label under the rail title.
Closes #498.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR changes diagnostics integrity counting to include index-served runs with synthetic entries, updates overview run selection to prioritize readiness-blocker-referenced runs and expose ChangesDashboard integrity, selection, and timeline updates
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Readiness
participant OverviewProjection as buildOverviewProjection
participant FocusRun as focusRun
participant CommandCenter as command-center.js
OverviewProjection->>FocusRun: pass readiness, runs
FocusRun->>FocusRun: check first blocker sourceRef.runId
FocusRun-->>OverviewProjection: selected run
OverviewProjection-->>CommandCenter: stagesSource (runId, goal or null)
CommandCenter->>CommandCenter: render or hide overview-proof-source
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
PR Summary by QodoDashboard truth fixes: timeline fallback, integrity counting, Proof Rail source
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/overview-proof-rail-498.test.js (1)
84-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify the rail source label through rendered DOM state.
assert.match(commandCenterScript, /stagesSource/)only checks that the script contains the property name. Add or extend the browser regression to call the renderer with an overview projection includingstagesSource, then assert#overview-proof-sourcetext and hidden state instead of relying on a string presence check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/overview-proof-rail-498.test.js` around lines 84 - 87, Replace the source-string assertion in the rail-label test with a browser regression that invokes the renderer using an overview projection containing stagesSource, then verify the rendered `#overview-proof-source` element has the expected text and hidden state. Keep the test focused on the rendered DOM behavior rather than merely checking commandCenterScript for the property name.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/diagnostics-integrity-497.test.js`:
- Around line 62-66: Update the test around buildFullState and the
run-fx-damaged lookup to make the fixture deterministically use the rollup-index
path, then assert that served.fromIndex is present instead of conditionally
calling t.skip. Keep the subsequent integrityErrorCount assertion so the test
always validates Diagnostics through the indexed serving path.
In `@tests/run-workspace-timeline-496.test.js`:
- Around line 41-71: The current renderer test only matches the string
"events.jsonl" in the bundle but does not verify it appears in the rendered
output when source is 'events' and omits it when source is 'persisted'. Add a
test that renders the workspace through the DOM regression path to verify the
source note actually displays for source 'events' but not for source
'persisted'. Additionally, add a test case with 121 ordered events to verify
that only the newest 120 entries are retained after capping, ensuring the
boundary enforcement is tested through the real rendering path rather than unit
assertions alone.
---
Nitpick comments:
In `@tests/overview-proof-rail-498.test.js`:
- Around line 84-87: Replace the source-string assertion in the rail-label test
with a browser regression that invokes the renderer using an overview projection
containing stagesSource, then verify the rendered `#overview-proof-source` element
has the expected text and hidden state. Keep the test focused on the rendered
DOM behavior rather than merely checking commandCenterScript for the property
name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e69a5387-ec07-4f65-8074-31f8d373d792
📒 Files selected for processing (9)
src/observability/dashboard/state/layers.jssrc/observability/dashboard/state/overview.jssrc/observability/dashboard/state/run-workspace.jssrc/observability/dashboard/ui/pages/command-center.jssrc/observability/dashboard/ui/pages/index.jssrc/observability/dashboard/ui/pages/run-workspace.jstests/diagnostics-integrity-497.test.jstests/overview-proof-rail-498.test.jstests/run-workspace-timeline-496.test.js
| const second = await buildFullState(root, { includeRegistry: false }); | ||
| const served = second.runs.find((run) => run.runId === 'run-fx-damaged'); | ||
| assert.ok(served, 'the damaged run is in scope'); | ||
| if (!served.fromIndex) t.skip('run was fully parsed again — index path not exercised in this environment'); | ||
| assert.ok(second.diagnostics.integrityErrorCount >= 1, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not skip the index-served assertion.
This test can pass without exercising the rollup-index path. Make the fixture deterministic, then require served.fromIndex before asserting the Diagnostics result.
Proposed test change
- if (!served.fromIndex) t.skip('run was fully parsed again — index path not exercised in this environment');
+ assert.equal(served.fromIndex, true,
+ 'the second build must serve the damaged run from the rollup index');As per coding guidelines, “Run regression tests against the real enforcement path where possible.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const second = await buildFullState(root, { includeRegistry: false }); | |
| const served = second.runs.find((run) => run.runId === 'run-fx-damaged'); | |
| assert.ok(served, 'the damaged run is in scope'); | |
| if (!served.fromIndex) t.skip('run was fully parsed again — index path not exercised in this environment'); | |
| assert.ok(second.diagnostics.integrityErrorCount >= 1, | |
| const second = await buildFullState(root, { includeRegistry: false }); | |
| const served = second.runs.find((run) => run.runId === 'run-fx-damaged'); | |
| assert.ok(served, 'the damaged run is in scope'); | |
| assert.equal(served.fromIndex, true, | |
| 'the second build must serve the damaged run from the rollup index'); | |
| assert.ok(second.diagnostics.integrityErrorCount >= 1, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/diagnostics-integrity-497.test.js` around lines 62 - 66, Update the
test around buildFullState and the run-fx-damaged lookup to make the fixture
deterministically use the rollup-index path, then assert that served.fromIndex
is present instead of conditionally calling t.skip. Keep the subsequent
integrityErrorCount assertion so the test always validates Diagnostics through
the indexed serving path.
Source: Coding guidelines
| test('a run with events but no derived timeline gets an events-sourced timeline (#496)', () => { | ||
| const workspace = buildRunWorkspace(baseRun()); | ||
| const timeline = workspace.sections.timeline; | ||
| assert.equal(timeline.available, true, 'the timeline is available'); | ||
| assert.equal(timeline.items.length, 5, 'every event becomes an entry'); | ||
| assert.equal(timeline.source, 'events', 'and the section names its fallback source'); | ||
| assert.equal(timeline.items[0].ts, '2026-07-01T01:00:00.000Z', 'entries stay time-ordered'); | ||
| assert.equal(timeline.items[0].type, 'task_started'); | ||
| assert.equal(timeline.items[0].task_id, '07-code'); | ||
| }); | ||
|
|
||
| test('a derived timeline keeps precedence — no event double-merge (#496)', () => { | ||
| const workspace = buildRunWorkspace(baseRun({ | ||
| timeline: [{ ts: '2026-07-01T05:00:00.000Z', type: 'stage_completed', stage_id: '07-code' }], | ||
| })); | ||
| const timeline = workspace.sections.timeline; | ||
| assert.equal(timeline.available, true); | ||
| assert.equal(timeline.items.length, 1, 'derived entries only — events never merge in beside them'); | ||
| assert.equal(timeline.source, 'persisted'); | ||
| }); | ||
|
|
||
| test('a run with neither derived timeline nor events keeps the honest empty state (#496)', () => { | ||
| const workspace = buildRunWorkspace(baseRun({ events: [] })); | ||
| const timeline = workspace.sections.timeline; | ||
| assert.equal(timeline.available, false, 'no invented timeline'); | ||
| }); | ||
|
|
||
| test('the workspace page renders the events-fallback source label (#496)', () => { | ||
| const bundle = commandCenterScript + runWorkspaceScript; | ||
| assert.match(bundle, /events\.jsonl/, 'the renderer names the fallback source in the timeline section'); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test the rendered source note and the cap boundary.
Line 70 matches events.jsonl in the explanatory comment in runWorkspaceScript. The test still passes if the section.source === 'events' branch is removed.
Run the renderer through the dashboard browser or DOM regression path. Assert that the note appears for source: 'events' and does not appear for source: 'persisted'. Add a case with 121 ordered events and assert that only the newest 120 entries remain.
As per coding guidelines, “Run regression tests against the real enforcement path where possible, including cross-process or browser behavior when those are the affected boundaries.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/run-workspace-timeline-496.test.js` around lines 41 - 71, The current
renderer test only matches the string "events.jsonl" in the bundle but does not
verify it appears in the rendered output when source is 'events' and omits it
when source is 'persisted'. Add a test that renders the workspace through the
DOM regression path to verify the source note actually displays for source
'events' but not for source 'persisted'. Additionally, add a test case with 121
ordered events to verify that only the newest 120 entries are retained after
capping, ensuring the boundary enforcement is tested through the real rendering
path rather than unit assertions alone.
Source: Coding guidelines
Code Review by Qodo
Context used✅ Compliance rules (platform):
300 rules✅ Skills:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo (convention, PR #490 precedent): the events-timeline depth cap and the rail-label goal length are named constants (EVENTS_TIMELINE_CAP — matching the rollup index's own timeline depth — and PROOF_SOURCE_GOAL_CHARS). Refs #496 #498 (PR #511 review follow-up). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Qodo's single insight addressed in the follow-up commit (it reported Bugs (0); Strix: 0 findings; CodeRabbit reviewed clean — no rate limit this time): the |
2 participants
The last three bugs from the 2026-07-30 tour's batch — all dashboard-truthfulness fixes, one bisected commit per issue, TDD throughout (every test verified failing on unfixed code first).
Closes #496
Closes #497
Closes #498
Commit 1 — #496: the timeline falls back to events.jsonl
The Run Workspace timeline read only the derived collections (
run.timeline/run.activityTimeline, populated for index entries since #296) — legacy and fixture-shaped runs declared "no recorded timeline events" whileevents.jsonlsat populated in the same snapshot. The projection now derives a minimal timeline from events when the derived collections are empty (time-ordered, capped at the index's own 120), and the page labels it — "Derived from events.jsonl — this run has no persisted timeline" — the same honesty convention as the 3D Studio's lifecycle timeline. Derived timelines keep precedence (no double-merge); the honest empty state remains when both sources are empty.Commit 2 — #497: the integrity signal is never a false zero
buildDiagnosticsaggregated integrity over fully-parsed runs only; index-served runs carryhas_integrity_errors(INDEX_VERSION 5, the #316 parity fix) but not the detail array — a damaged run served from the rollup index contributed nothing, and Diagnostics read "0" with the damaged run in plain view. Index-served damage now counts and names its run ("rollup index entry — run reports damaged files — open the run for per-file detail"); fully-parsed detail unchanged, no double-counting. Pinned by unit cases plus a real two-build round-trip (first build persists the index, second serves the damaged run from it — the exact false-zero path). Persisting compact detail into index entries rides the next INDEX_VERSION bump (noted on the issue).Commit 3 — #498: the Proof Rail names and follows its run
At all-runs scope the hero cited one run's verdict while the rail silently rendered a different run's stages — 15 unqualified "Not Started" beside "coverage 85%" read as the dashboard disagreeing with itself. Server-owned fix in the overview projection:
focusRunprefers the run the verdict's first blocker cites (in-scope only, safe fallbacks), andstagesSource {runId, goal}labels the rail ("Stage proof from Blocked fixture — exhausted retries · run-fx-blocked"). Pages render the label; they never re-derive it.Verification
run-fx-damagedthrough its index entry; the Command Center hero's verdict, goal chip, and rail all speak about the same run with the source label visible. Zero console errors.Merge-order note
state/overview.jsis also touched by #508 (#493's rationale fix) — the hunks are adjacent (both inbuildOverviewProjection), so whichever merges second may hit a trivial conflict (keep both: #508'srationalefield + this PR'sstagesSourcefield and readiness-first ordering). Everything else is disjoint from #504/#508/#509.Held for Richardson's batch review — please verify before merge.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
events.jsonl.Bug Fixes