feat(mt#4557): Reviewer-cost page shell — blocked on mt#4546 accessor + chart-library decision - #3348
Open
minsky-ai[bot] wants to merge 8 commits into
Open
feat(mt#4557): Reviewer-cost page shell — blocked on mt#4546 accessor + chart-library decision#3348minsky-ai[bot] wants to merge 8 commits into
minsky-ai[bot] wants to merge 8 commits into
Conversation
…oute Page renders through the standard WidgetModule ok|degraded contract (mt#2757 discipline: explicit error, never a zero). The widget's fetch() deliberately does not query review_timing directly -- mt#4546's own domain-side accessor has not landed (stale/abandoned session, no code), which mt#4557 SC3 requires reading through with no second query layer. Filed ask#10301 (routed to operator) to resolve ownership; filed ask#10299 (routed to operator) for the reserved chart-library decision (mt#2601's no-dependency constraint vs adopting recharts/visx). Page renders its explicit-error branch until ask#10301 resolves; the "ok" branch (daily-spend table, per-config cohort table, cap-pin tile, outlier tail) is written against the payload contract and will light up once the accessor ships -- tests to follow. No confound caveat is rendered on the cohort table per SC2: mt#4569's per-PR-parity interleaved assignment makes the split a genuine controlled comparison.
Widget test locks the mt#4546 coordination boundary itself: fetch() must resolve degraded (never ok, never throw) while the accessor is unwired -- a regression test against a future edit querying review_timing directly from this widget. Page tests cover the failure path first (SC4/AT3): a degraded widget renders an explicit error and no happy-path section underneath it -- this is mt#2757's exact prior failure mode (healthy zeros rendered for five weeks under a query failure). The "ok" branch is exercised against fabricated data ahead of mt#4546 landing: daily-spend table, cap-pin tile, cohort table (including the null-fingerprint "unknown configuration" row, and no confound caveat per SC2), and the outlier tail's PR link.
Minsky Reviewer StatusReview in progress... |
Cockpit PreviewPreview URL: https://cockpit-preview-production.up.railway.app Deployed from commit Updated by Cockpit Preview. |
![minsky-reviewer[bot]](https://avatars.githubusercontent.com/u/52695?s=60&v=4){kind=small}
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
This PR adds the /reviewer/cost page, a new reviewer-cost widget, a TanStack Query hook, tests, and registry wiring. The code deliberately returns a degraded state pending mt#4546 (accessor) and ask#10299 (charting), with rendering for all four SC1 sections implemented and tested against fabricated data. However, the route is mounted in App.tsx while the hook throws on any non-ok state and the widget always returns degraded, so navigating to the page will perpetually show an error banner — a shipped broken nav surface. This is a BLOCKING gating issue: add a feature flag or dev-only guard, or render a non-error "blocked on mt#4546 / ask#10301" informational state, or defer route registration until the accessor ships. SC4 is met (explicit error; no silent zeros). SC1–SC3 are Not Met by design and recorded as such in spec verifications. Pending the gating fix, request changes.
Findings
- [BLOCKING] src/cockpit/web/hooks/useReviewerCost.ts:26 — Hook throws on non-ok state while widget is intentionally always degraded — page route will perpetually render an error, violating AT1 (live chart) and SC1 readiness without a documented feature flag or route gating.
fetchReviewerCost()throws whendata.state !== "ok"(src/cockpit/web/hooks/useReviewerCost.ts:23-31). GivenreviewerCostWidget.fetch()always returns{state:"degraded"}(src/cockpit/widgets/reviewer-cost.ts:120-153), navigating to/reviewer/costwill always render the error branch. The PR mounts this route inApp.tsx(src/cockpit/web/App.tsx:458-469), exposing a permanently failing page in production. The task spec’s AT1 requires a live chart; while deferred is acceptable, shipping an always-erroring route without a feature flag, dev-only guard, or route gating is a UX regression and creates a broken nav entry. Suggested fix: gate the route behind a feature flag or render an explicit “Blocked on mt#4546 / ask#10301” informational state from the hook/page without throwing (keep SC4 discipline for true failures, but distinguish known-blocked from transient errors). Alternatively, defer route registration until mt#4546 lands or provide a static “coming soon” shell that doesn’t surface as an application error.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| SC1: A cockpit page renders daily spend (30 days, stacked by token class) with a $/review line overlaid; a per-config cohort table; cap-pin share as a prominent number; and the outlier tail (10 most expensive reviews), all from live data. | Not Met | The page and sections exist, but the primary chart is deferred to a table pending ask#10299 and the data path is intentionally degraded pending mt#4546. Evidence: src/cockpit/web/pages/ReviewerCostPage.tsx:44-75 (chart rendered as table per comment), src/cockpit/web/hooks/useReviewerCost.ts:23-31 (throws on non-ok), and src/cockpit/widgets/reviewer-cost.ts:105-153 (fetch() always returns {state:"degraded"}). Tests explicitly assert degraded/no-data paths: src/cockpit/web/pages/ReviewerCostPage.test.tsx:24-66, 118-176. Follow-ups filed in PR body (mt#4587, ask#10299). |
| SC2: Lever effects are shown by cohort split on config fingerprint (mt#4556), not by time; no confound caveat rendered; the cohort query must carry no time predicate. | Not Met | Rendering layer honors the no-caveat requirement (asserted in test), but there is no live cohort query yet (mt#4546 unwired). Evidence: src/cockpit/web/pages/ReviewerCostPage.tsx:122-170 (CohortTableSection with no confound caveat), src/cockpit/web/pages/ReviewerCostPage.test.tsx:146-154 (asserts no confound text). However, the widget has no query and returns degraded (src/cockpit/widgets/reviewer-cost.ts:120-153), so the cohort split is not sourced from live data and no query constraints can be verified. Marking Not Met with dependency on mt#4546 for actual query implementation. |
| SC3: The page reads through mt#4546's accessor; no second query layer. | Not Met | The implementation deliberately avoids any direct read and does not wire mt#4546 (which is not implemented yet). The widget returns a degraded state citing mt#4546/ask#10301. Evidence: src/cockpit/widgets/reviewer-cost.ts:120-153 (explicit degraded with mt#4546 reference), src/cockpit/widgets/reviewer-cost.test.ts:14-34 (test locks this boundary). Until mt#4546 ships and is consumed here, SC3 remains unmet. |
| SC4: Data-freshness is visible on the page. A stale or failed query renders an explicit error, never as a zero. | Met | The hook throws on degraded; the page renders a visible error banner and asserts that no happy-path sections render underneath. Evidence: src/cockpit/web/hooks/useReviewerCost.ts:23-31 (throw on non-ok), src/cockpit/web/pages/ReviewerCostPage.tsx:211-228 (error alert), and tests at src/cockpit/web/pages/ReviewerCostPage.test.tsx:67-117 verifying explicit error and absence of zero-like fallbacks. |
Documentation impact
- blocking-needs-update — This PR adds a new public cockpit route (/reviewer/cost), a new widget id ("reviewer-cost"), and user-visible page states (error, no-data, ok). I did not sweep all docs; I briefly checked docs/cockpit-ui.md earlier but it contains no mention of the Reviewer Cost page. Because this introduces a new operator-facing surface, the operator guide (cockpit UI docs) needs an entry describing the page purpose, sections, and current blocked behavior (if shipped gated) or live behavior (once mt#4546 lands). Absent such updates, the docs omit a surfaced route. If existing docs elsewhere still claim there is no reviewer cost page, they become false.
Affected: docs/cockpit-ui.md
#3348 R1) Reviewer-bot BLOCKING finding: useReviewerCost threw a generic error for any non-ok widget state, so the permanently-degraded "mt#4546 unwired" case rendered identically to a genuine live outage -- an operator visiting the route (or a future maintainer once the accessor lands) would have no way to tell "not built yet" from "something's broken." Fixes: the widget exports a stable, matchable reason prefix (NOT_YET_WIRED_REASON_PREFIX); the hook throws a distinct ReviewerCostNotYetAvailableError for that known case and a plain Error for anything else; the page renders a neutral "Not yet available" notice for the former and reserves the urgent red error panel for the latter. Both new tests pass fabricated reasons through the real hook/page (not just asserting the widget's raw output), so the distinction is exercised end to end -- including a dedicated test for what a genuine live failure looks like once mt#4546 is wired.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verified the R1 BLOCKING finding was addressed: useReviewerCost now distinguishes the known “not yet wired” degraded reason via a dedicated ReviewerCostNotYetAvailableError (keyed off NOT_YET_WIRED_REASON_PREFIX) from genuine live failures, and the page renders distinct states for each. The fix is covered by new tests, alongside ok/no-data branches. I found no new critical defects introduced by this change. The remaining SC/AT gaps are explicitly deferred to mt#4546 and the charting follow-up, and the code guards against zero-filled fallbacks per SC4. Approving this iteration.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: - Daily spend, 30 days, stacked by token class (uncached input / cached input / output, with reasoning marked as a share of output), with a |
Not Met | The page shell and renderers exist (src/cockpit/web/pages/ReviewerCostPage.tsx:1-306) and tests exercise fabricated OK data (src/cockpit/web/pages/ReviewerCostPage.test.tsx:109-200). However, the SC requires rendering these from live data; the widget (src/cockpit/widgets/reviewer-cost.ts:112-166) intentionally returns state: "degraded" pending mt#4546, and the hook distinguishes that state into a neutral not-yet-available notice (src/cockpit/web/hooks/useReviewerCost.ts:36-64). The bar+line chart is also deferred to mt#4587 (currently a table placeholder). Follow-ups named in PR body: mt#4546 and mt#4587. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis — time annotations are confounded by the traffic shifts this page exists to expose. | Not Met | Rendering honors SC2 (no confound caveat; see src/cockpit/web/pages/ReviewerCostPage.tsx:97-135 and test ReviewerCostPage.test.tsx:162-170), but the criterion also requires the cohort query carry no time predicate. No query exists yet — the widget is deliberately unwired and returns degraded (src/cockpit/widgets/reviewer-cost.ts:112-166). Enforcement of the no-time-predicate property must be implemented when wiring mt#4546; currently deferred. |
| The page reads through mt#4546's accessor; no second query layer. | Not Met | By design the widget avoids a second query layer and returns degraded until mt#4546 ships (src/cockpit/widgets/reviewer-cost.ts:33-58,112-166). The page does not read through mt#4546 yet because that accessor is absent. This is explicitly deferred to mt#4546 in code comments and tests. |
| Data-freshness is visible on the page. A stale or failed query renders as an explicit error, never as a zero — this corner of the cockpit rendered healthy zeros for five weeks (mt#2757) and a smoke script reported PASS on 15/15 query failures. | Met | Distinct error handling is implemented: the hook throws a ReviewerCostNotYetAvailableError for the known NOT_YET_WIRED case (src/cockpit/web/hooks/useReviewerCost.ts:22-34,36-49), and the page renders a neutral notice for that (src/cockpit/web/pages/ReviewerCostPage.tsx:214-230) versus an urgent error panel for genuine failures (:232-245). Tests assert both behaviors and that no happy-path sections render under error states (src/cockpit/web/pages/ReviewerCostPage.test.tsx:39-108). |
| Acceptance Test 1: Loading the page shows the 30-day stacked chart with the $/review line, populated from live data. | Not Met | The chart is deferred (renders a table placeholder) and live data is not wired (widget returns degraded). See src/cockpit/web/pages/ReviewerCostPage.tsx:25-44 and src/cockpit/widgets/reviewer-cost.ts:33-58,112-166. |
| Acceptance Test 2: Re-running the underlying aggregates as SQL reproduces the rendered numbers. | Not Met | No aggregates run yet; the widget intentionally does not query and returns degraded (src/cockpit/widgets/reviewer-cost.ts:112-166). |
| Acceptance Test 3: Killing the DB connection renders a visible error state, not zeros. | Met | Error-state rendering is implemented and covered by tests simulating a degraded reason for a live failure (ReviewerCostPage.test.tsx:79-108), asserting the urgent error panel is shown and no zeros/happy-path sections appear. |
| Acceptance Test 4: Two configurations present in the window appear as separate cohort rows. | Met | The OK-branch test fabricates two cohort rows (one with configFingerprint set and one null) and asserts both render distinctly, with the null row labelled "unknown configuration" (src/cockpit/web/pages/ReviewerCostPage.test.tsx:121-171). |
| Acceptance Test 5: A screenshot of the finished page at a realistic viewport is attached to the PR (the render-path evidence gate), uncropped. | Unverifiable | The PR body references an out-of-repo path /tmp/reviewer-cost-screenshot.png for the screenshot. This repository diff does not carry the image, and the path is outside the repo; reviewer cannot verify attachment presence here. |
Documentation impact
- no-update-needed — This PR adds the reviewer-cost page shell and internal widget/hook behavior but deliberately renders a "not yet available" state until mt#4546 and the charting decision land. No user-facing docs for cockpit internals were updated or invalidated here; behavior is gated and matches the PR description. I skimmed for dedicated cockpit docs in
docs/and found none referenced by this change in the diff.
…odule (PR #3348 R2) The R1 fix (52f0152) imported NOT_YET_WIRED_REASON_PREFIX as a VALUE from reviewer-cost.ts into the client-side hook. That module imports db-providers.ts (Node-only: process.env, @minsky/domain/persistence), so the value import pulled that whole graph into the Vite client bundle and crashed the live page with "process is not defined" -- invisible to component tests (bun's test DOM has `process` defined; a real browser doesn't). Caught by re-screenshotting the actual dev server after the R1 fix, not by the passing test suite. Fix: extract the shared types + NOT_YET_WIRED_REASON_PREFIX into reviewer-cost-contract.ts, a deliberately dependency-free module both the server-side widget and the client-side hook can import safely. reviewer-cost.ts re-exports from it for backward compatibility. Verified against the live dev server: the page now renders the neutral "Not yet available" notice with no console error, not a crash.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
The R1 BLOCKING issue (client importing a value from the server-only widget and pulling Node-only deps into the browser bundle) is resolved by introducing a dependency-free shared contract module and switching the client hook to import from it. The hook now cleanly distinguishes the known mt#4546-not-wired degraded reason from genuine failures, preserving the explicit-error semantics required by SC4. I verified the split: reviewer-cost-contract.ts has zero imports; useReviewerCost.ts imports only from it and the browser-safe widget client; and the server widget re-exports the contract without leaking Node deps client-side. I found no new critical issues introduced by this fix. One pre-existing nit remains (duplicate React keys when configFingerprint is null in the cohort table), but it is not introduced by this commit. With the blocker addressed and no new critical defects, this PR is approved.
Findings
- [PRE-EXISTING] src/cockpit/web/pages/ReviewerCostPage.tsx:121 — Potential duplicate React keys for unknown fingerprints in cohort table
InCohortTableSectionthe row key iskey={c.configFingerprint ?? "unknown"}(src/cockpit/web/pages/ReviewerCostPage.tsx:121). Multiple rows with anullconfigFingerprint(pre-mt#4556 rows) will share the key"unknown", which can cause React reconciliation glitches. Prefer a stable unique key per row, e.g., prefixing with an index for nulls or combining with another stable field:key={(c.configFingerprint ?? "unknown") + ":" + idx}. This concern is PRE-EXISTING — the current fix commit did not modify this file.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: - Daily spend, 30 days, stacked by token class with a $/review line overlaid. - A per-config cohort table. - Cap-pin share as a prominent number. - Outlier tail of the ten most expensive reviews linking to their PRs. | Unverifiable | This round’s diff only adds the client hook src/cockpit/web/hooks/useReviewerCost.ts and splits a shared contract module src/cockpit/widgets/reviewer-cost-contract.ts. The rendering page/component and routing were not part of this fix commit’s changes, so their presence/shape cannot be verified from the shown diff. Verifying SC1 would require re-reading the page component outside this diff scope. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis. The cohort table needs no confound caveat, and the cohort query must carry no time predicate. | Unverifiable | This criterion concerns the query layer and the page’s cohort rendering. The fix commit under review does not modify query code or the page/table; it introduces a dependency-free contract and updates the widget’s degraded behavior. Verification would require reading the query accessor (mt#4546) and the page rendering, which are outside this diff. |
| The page reads through mt#4546's accessor; no second query layer. | Unverifiable | Per the current diff, src/cockpit/widgets/reviewer-cost.ts explicitly returns a degraded state and defers wiring until mt#4546 ships, citing ask#10301. mt#4546’s accessor does not exist yet (referenced spec shows it as in scope but not implemented). Because the accessor artifact lives outside this diff and is not present, end-to-end compliance cannot be verified here. |
| Data-freshness is visible on the page. A stale or failed query renders as an explicit error, never as a zero. | Met | src/cockpit/widgets/reviewer-cost.ts:68-87 returns { state: "degraded", reason: ... } instead of synthesizing zero data; src/cockpit/web/hooks/useReviewerCost.ts:49-60 throws an error (or a ReviewerCostNotYetAvailableError for the known mt#4546 blocker) when the widget is not ok, ensuring an explicit error path rather than zeros. |
Documentation impact
- no-update-needed — This commit is an internal refactor/fix: it splits a server-only widget module’s shared constants/types into a dependency-free contract to prevent bundling Node-only deps into the browser, and adds a client hook that distinguishes a known not-yet-wired degraded reason from real errors. It does not alter user-visible routes, commands, or documented behavior; page semantics (blocked state vs ok) remain as described by the task spec. No docs under
docs/appear related to this internal module split.
…r stage docker-build-smoke (services/cockpit/Dockerfile) and the Railway deploy check both failed on this PR's HEAD: useReviewerCost.ts is the first cockpit-web module to import across the src/cockpit/web -> src/cockpit/widgets boundary (deliberately, per the R2 fix -- it imports the dependency-free reviewer-cost-contract.ts, not the widget module itself), and the Dockerfile's builder stage only ever copied src/cockpit/web, never src/cockpit/widgets. Vite/Rollup failed to resolve the import at build time: "Could not resolve '../../widgets/reviewer-cost-contract'". The runtime stage was already fine (it COPYs the whole src tree); only the builder stage needed the addition. Only reviewer-cost-contract.ts is actually imported by the client bundle (zero-dependency by design), so this doesn't drag any Node-only widget code (db-providers.ts et al.) into the SPA build.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verified the sole new change since the last review: services/cockpit/Dockerfile now COPYs src/cockpit/widgets into the builder stage so Vite/Rollup can resolve the cross-boundary import from the SPA (useReviewerCost → reviewer-cost-contract). This addresses the reported build failure without changing runtime behavior. The runtime stage already COPYs the entire src tree, so no additional change is needed there. I found no new defects introduced by this fix and no spec-impacting changes in this commit. Approving to merge.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: - Daily spend, 30 days, stacked by token class with a $/review line; - A per-config cohort table; - Cap-pin share; - Outlier tail with PR links. | N/A | This fix commit only updates the Docker builder stage to COPY src/cockpit/widgets so Vite can resolve a cross-boundary import (services/cockpit/Dockerfile:87-96). It does not alter page rendering or data logic. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis; no time predicate in the cohort query and no confound caveat rendered. | N/A | No query or rendering code was changed in this commit; only Dockerfile builder inputs were adjusted (services/cockpit/Dockerfile:87-96). |
| The page reads through mt#4546's accessor; no second query layer. | N/A | No data-access code is touched by this commit. The only change is adding src/cockpit/widgets to the Vite build context (services/cockpit/Dockerfile:87-96). |
| Data-freshness is visible on the page; a stale or failed query renders as an explicit error, never as a zero. | N/A | This commit affects only the Docker build inputs, not runtime behaviour or UI states (services/cockpit/Dockerfile:87-96). |
| Acceptance Test 1: Loading the page shows the 30-day stacked chart with the $/review line, populated from live data. | N/A | Out of scope for this commit; only Dockerfile builder COPY additions were made (services/cockpit/Dockerfile:87-96). |
| Acceptance Test 2: Re-running the underlying aggregates as SQL reproduces the rendered numbers. | N/A | No aggregate or UI changes in this commit; only Dockerfile edit (services/cockpit/Dockerfile:87-96). |
| Acceptance Test 3: Killing the DB connection renders a visible error state, not zeros. | N/A | No runtime code changed here; Dockerfile builder-stage COPY only (services/cockpit/Dockerfile:87-96). |
| Acceptance Test 4: Two configurations present in the window appear as separate cohort rows. | N/A | Unchanged by this commit; only Docker build inputs adjusted (services/cockpit/Dockerfile:87-96). |
| Acceptance Test 5: A screenshot of the finished page at a realistic viewport is attached to the PR (render-path evidence gate), uncropped. | N/A | This commit is a Dockerfile fix and does not affect evidence assets; verification of attachments is out of scope for this specific change. |
Documentation impact
- no-update-needed — This commit only amends the cockpit Dockerfile builder stage to include
src/cockpit/widgetsso Vite can resolve a cross-boundary import during the SPA build (services/cockpit/Dockerfile:87-96). It does not change any user-facing behavior, routes, or CLI surface, and no docs reference this internal build detail. No documentation updates are needed.
…-4557 The reviewer correctly marked AT5 Unverifiable: the PR body referenced an out-of-repo /tmp path nobody but the authoring session could open. Separately, that referenced screenshot was stale -- captured 18:10Z, before the R1 fix (~18:23Z), so it showed the urgent red "Data unavailable" error banner: a picture of the bug the R1 BLOCKING finding was about, not of the fix. Fix: re-captured against current HEAD (18:38Z, unchanged frontend since R2) and committed the PNG under docs/evidence/mt-4557/, following the established convention (docs/evidence/mt-4251, mt-2917, mt-3246, et al.) rather than an unreachable path. README documents all four captures across this PR's history with distinct md5 sums, so the progression from bug to fix is verifiable rather than asserted.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Reviewed the final delta: only documentation evidence (README + uncropped screenshot) was added under docs/evidence/mt-4557 to satisfy AT5. No production code changed and no new defects were introduced. Prior BLOCKING concerns remain addressed. Live-behavior criteria (SC1–SC4, AT1–AT4) were not re-verified in this docs-only update. Approving.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: (a) 30-day stacked daily-spend by token class with a $/review overlay line; (b) a per-config cohort table; (c) cap-pin share; (d) an outlier tail of the ten most expensive reviews, each linking to its PR. | Unverifiable | This criterion depends on live data rendering and the unresolved chart-library decision (ask#10299) and mt#4546 accessor. The current commit only adds documentation evidence files under docs/evidence/mt-4557/. Verifying runtime rendering requires a live server and data source outside this diff. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis; no time predicate in the cohort query and no confound caveat rendered. | Unverifiable | Compliance requires verifying the query layer (mt#4546) and rendered cohort table behavior at runtime. This commit adds only documentation assets; mt#4546 remains unresolved per PR description. Not verifiable from this diff alone. |
| The page reads through mt#4546's accessor; no second query layer. | Unverifiable | The presence/usage of mt#4546’s accessor is a cross-module runtime contract not changed in this commit. The PR description indicates mt#4546 is not yet implemented; this commit touches only docs/evidence. Cannot be verified from these changes. |
| Data-freshness is visible on the page. A stale or failed query renders as an explicit error, never as a zero. | Unverifiable | Requires exercising failure paths in a running environment. This commit adds documentation evidence, not code. Verification of error vs zero behavior is outside the scope of this docs-only delta. |
| AT1: Loading the page shows the 30-day stacked chart with the $/review line, populated from live data. | Unverifiable | Live-data rendering cannot be confirmed from this docs-only change. The PR body also notes the chart decision is pending and mt#4546 is not wired yet. |
| AT2: Re-running the underlying aggregates as SQL reproduces the rendered numbers. | Unverifiable | Requires access to the live DB and aggregates; no such verification artifacts are in this commit, which only adds documentation assets. |
| AT3: Killing the DB connection renders a visible error state, not zeros. | Unverifiable | A live-failure simulation is outside the scope of this diff and cannot be verified from the added docs. |
| AT4: Two configurations present in the window appear as separate cohort rows. | Unverifiable | Depends on live data spanning two configurations and the mt#4556/mt#4569 mechanics; this commit adds only documentation files. |
| AT5: A screenshot of the finished page at a realistic viewport is attached to the PR (render-path evidence), uncropped. | Met | docs/evidence/mt-4557/reviewer-cost-not-yet-available.png added (binary) and documented in docs/evidence/mt-4557/README.md, including capture details and viewport (1440×900). |
Documentation impact
- updated-in-pr — This commit adds render-path evidence under docs/evidence/mt-4557/: a README describing capture details and an uncropped screenshot PNG. This directly addresses Acceptance Test 5 for mt#4557 without changing runtime behavior.
Affected: docs/evidence/mt-4557/README.md, docs/evidence/mt-4557/reviewer-cost-not-yet-available.png
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-verified the prior BLOCKING concern: the hook now distinguishes the known “not yet wired” state via a dedicated error class and the page renders a neutral notice distinct from real failures. The subsequent regression called out in R2 (client pulling Node-only deps) is addressed by extracting a dependency-free contract module and switching both sides to it; the Dockerfile now copies src/cockpit/widgets/ so the SPA build resolves the contract import. Widget is registered and the new route/page render the expected loading/error/no-data/ok branches with tests covering each. I found no new critical defects introduced in this iteration. One minor nit: a test still type-imports from the server-side widget module instead of the contract — suggest pointing it at reviewer-cost-contract to prevent future accidental value-imports. Otherwise, the PR is ready to merge pending CI.
Findings
- [NON-BLOCKING] src/cockpit/web/pages/ReviewerCostPage.test.tsx:23 — Test imports
ReviewerCostPayloadfrom the server-side widget module; prefer the contract module to avoid future accidental value-imports
The test currently doesimport type { ReviewerCostPayload } from "../../widgets/reviewer-cost". Whileimport typeis safe at runtime, it still couples the test to the server-only widget module. If a future edit removestypeor adds a value import nearby, it risks reintroducing the R2 regression shape (pulling Node-only deps into the client/test context). Suggest switching to../../widgets/reviewer-cost-contractfor the type — it’s dependency-free by design and is what the hook uses.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: (a) 30-day stacked daily-spend by token class with a $/review overlay line; (b) a per-config cohort table; (c) cap-pin share; (d) an outlier tail of the ten most expensive reviews, each linking to its PR. | Unverifiable | Rendering components and tests for all four sections exist — see src/cockpit/web/pages/ReviewerCostPage.tsx (DailySpendSection, CohortTableSection, CapPinShareTile, OutlierTailSection) and ReviewerCostPage.test.tsx happy-path assertions. However live data rendering is deferred pending mt#4546 and ask#10299; the widget currently returns degraded (see src/cockpit/widgets/reviewer-cost.ts). Without the accessor/chart decision landed, this cannot be verified against live data. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis; no confound caveat is rendered and no time predicate is used for the cohort split. | Unverifiable | The page renders a cohort table without any confound caveat (see ReviewerCostPage.tsx — CohortTableSection and test ReviewerCostPage.test.tsx which asserts no confound text). The query layer enforcing "no time predicate" is deferred to mt#4546 and not present in this PR; therefore end-to-end compliance cannot be verified from this diff. |
| The page reads through mt#4546's accessor; no second query layer. | Unverifiable | src/cockpit/widgets/reviewer-cost.ts explicitly avoids querying review_timing and instead returns a degraded state until mt#4546 ships. There is no second query layer in this PR, but the accessor itself does not exist yet in-repo to verify integration against. |
| Data-freshness is visible on the page. A stale or failed query renders as an explicit error, never as a zero. | Met | src/cockpit/web/hooks/useReviewerCost.ts throws a distinct ReviewerCostNotYetAvailableError for the known not-yet-wired case and a plain Error for genuine failures; ReviewerCostPage.tsx renders two distinct branches (data-testid="reviewer-cost-not-yet-available" vs reviewer-cost-error). Tests in ReviewerCostPage.test.tsx cover both failure modes and assert no zero-filled sections render in error states. |
| Acceptance Test 1 — Loading the page shows the 30-day stacked chart with the $/review line, populated from live data. | Unverifiable | Charting is deferred (ask#10299) and the widget is degraded pending mt#4546; only a table placeholder renders for the chart section. Live-data rendering cannot be verified from this diff. See ReviewerCostPage.tsx comment and reviewer-cost.ts. |
| Acceptance Test 2 — Re-running the underlying aggregates as SQL reproduces the rendered numbers. | Unverifiable | No aggregates are executed in this PR (mt#4546 accessor is not yet implemented). The widget returns degraded. Cannot be verified from this diff. |
| Acceptance Test 3 — Killing the DB connection renders a visible error state, not zeros. | Met | useReviewerCost.ts converts non-prefixed degraded reasons into thrown Error, and ReviewerCostPage.tsx renders the urgent error panel (data-testid="reviewer-cost-error"). Test "a GENUINE live query failure ... renders the urgent error panel" in ReviewerCostPage.test.tsx verifies this behavior. |
| Acceptance Test 4 — Two configurations present in the window appear as separate cohort rows. | Unverifiable | The rendering for multiple cohorts is implemented and tested with fabricated data (ReviewerCostPage.test.tsx asserts two rows including a null-fingerprint row). Live data via mt#4546 is not wired, so this cannot be verified against real data. |
| Acceptance Test 5 — A screenshot of the finished page at a realistic viewport is attached to the PR (uncropped). | Met | docs/evidence/mt-4557/reviewer-cost-not-yet-available.png added, with provenance and md5 hashes documented in docs/evidence/mt-4557/README.md. The route and current neutral-notice state are shown; viewport and capture method are recorded. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| reviewerCostWidget | function | src/cockpit/widget-registry.ts:54 — registered under key "reviewer-cost" for API/widget routing | Adopted | Widget is wired into the central registry; the SPA hook fetches from /api/widget/reviewer-cost/data via widget-client. |
Documentation impact
- no-update-needed — This PR introduces a new cockpit page route (
/reviewer/cost) and corresponding widget wiring, but the page is a shell pending mt#4546 and ask#10299. User-facing docs were not previously asserting availability of this page, and this PR adds only render-path evidence underdocs/evidence/mt-4557/(internal verification artifacts). No existing documentation is invalidated, and no public CLI/API behavior changed.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-verified the R1 BLOCKING concern: the hook now emits a dedicated ReviewerCostNotYetAvailableError for the known not-yet-wired state keyed by a shared reason prefix, and the page renders a neutral notice distinct from genuine failures. The R2 regression shape (client pulling Node-only deps) is addressed by extracting a dependency-free contract module and updating both sides; the Dockerfile now copies src/cockpit/widgets/ so the SPA build resolves the contract import. Widget is registered, the route is added, and loading/error/no-data/ok branches are implemented with tests covering each. I see no new critical issues introduced by this iteration. Approving pending CI.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: (a) 30-day stacked daily-spend by token class with a $/review line; (b) a per-config cohort table; (c) cap-pin share; (d) an outlier tail of the ten most expensive reviews, each linking to its PR. | Unverifiable | UI and wiring are present: route added in src/cockpit/web/App.tsx:454-465; page implemented in src/cockpit/web/pages/ReviewerCostPage.tsx (sections DailySpendSection, CohortTableSection, CapPinShareTile, OutlierTailSection); widget registered in src/cockpit/widget-registry.ts:51; hook in src/cockpit/web/hooks/useReviewerCost.ts. However charting is deferred (ask#10299) and live data is blocked on mt#4546; the widget returns degraded by design (src/cockpit/widgets/reviewer-cost.ts:77-100). Cannot verify live rendering of chart or data from this diff. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis — no confound caveat is rendered and no time predicate is used for the cohort split. | Unverifiable | The page renders a cohort table without any confound caveat (src/cockpit/web/pages/ReviewerCostPage.tsx:107-162) and tests assert its presence with fabricated data (src/cockpit/web/pages/ReviewerCostPage.test.tsx:132-159). The query layer enforcing "no time predicate" for the cohort split is deferred to mt#4546; no accessor exists in this PR to verify end-to-end compliance. |
| The page reads through mt#4546's accessor; no second query layer. | Unverifiable | src/cockpit/widgets/reviewer-cost.ts explicitly avoids querying review_timing and returns a degraded state until mt#4546 ships, to prevent a parallel query layer (module doc and fetch() body at lines 41-76, 83-100). There is no second query layer in this PR, but the accessor itself is not present to verify integration. |
| Data-freshness is visible on the page. A stale or failed query renders as an explicit error, never as a zero. | Met | src/cockpit/web/hooks/useReviewerCost.ts:49-60 throws a dedicated ReviewerCostNotYetAvailableError for the known not-yet-wired case (matched via NOT_YET_WIRED_REASON_PREFIX), and a plain Error otherwise. ReviewerCostPage.tsx:224-259 renders two distinct branches: neutral notice (data-testid="reviewer-cost-not-yet-available") vs urgent error panel (data-testid="reviewer-cost-error"). Tests cover both (ReviewerCostPage.test.tsx:85-131 and :133-159). |
| Acceptance Test 1 — Loading the page shows the 30-day stacked chart with the $/review line, populated from live data. | Unverifiable | Chart renders as a table placeholder pending ask#10299 (DailySpendSection comment at ReviewerCostPage.tsx:38-49), and the widget is degraded until mt#4546 exists (src/cockpit/widgets/reviewer-cost.ts:77-100). Live-data chart cannot be verified in this diff. |
| Acceptance Test 2 — Re-running the underlying aggregates as SQL reproduces the rendered numbers. | Unverifiable | No aggregates are executed in this PR; mt#4546 accessor is not implemented here. The widget returns degraded (src/cockpit/widgets/reviewer-cost.ts). |
| Acceptance Test 3 — Killing the DB connection renders a visible error state, not zeros. | Met | Non-prefixed degraded reasons are surfaced as thrown Error in the hook (useReviewerCost.ts:53-60) and the page renders the urgent error panel (ReviewerCostPage.tsx:249-259). Test "a GENUINE live query failure..." at ReviewerCostPage.test.tsx:112-131 exercises this path. |
| Acceptance Test 4 — Two configurations present in the window appear as separate cohort rows. | Unverifiable | Rendering for multiple cohorts is implemented and tested with fabricated data (ReviewerCostPage.test.tsx:141-159 asserts two rows including a null-fingerprint "unknown configuration" row). Live data path via mt#4546 is not wired in this PR. |
| Acceptance Test 5 — A screenshot of the finished page at a realistic viewport is attached to the PR (uncropped). | Met | docs/evidence/mt-4557/reviewer-cost-not-yet-available.png added; provenance and md5s recorded in docs/evidence/mt-4557/README.md. The screenshot shows the page's current neutral-notice state. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| reviewerCostWidget | function | src/cockpit/widget-registry.ts:51 — registered under key "reviewer-cost" for API/widget routing, src/cockpit/web/hooks/useReviewerCost.ts:65 — fetches via widget-client using key "reviewer-cost", src/cockpit/web/pages/ReviewerCostPage.tsx:210 — page consumes hook wired to this widget | Adopted | Widget is registered in the central registry and reached by the SPA hook through /api/widget/reviewer-cost/data. |
Documentation impact
- no-update-needed — This iteration adds the reviewer-cost page route and widget wiring plus test and evidence assets, but functionality remains a shell pending mt#4546 and the chart decision. No existing user-facing docs assert availability of this page, and no public CLI/API behavior changed. The only docs changes are internal verification artifacts under docs/evidence/mt-4557/.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-verified the prior BLOCKING concern: the hook now distinguishes the known “not yet wired” state via a dedicated error class and a shared reason-prefix, and the page renders a neutral notice distinct from genuine failures. The R2 regression shape (client pulling Node-only deps) is prevented by extracting a dependency-free reviewer-cost-contract module consumed on both sides; the Dockerfile now copies src/cockpit/widgets/ so the SPA build resolves that import. Widget is registered, the /reviewer/cost route is added, and loading/error/no-data/ok branches are implemented with tests covering each. I found no new critical issues introduced by this iteration. One minor nit remains in tests (type-importing from the server widget module instead of the contract); it’s non-blocking. Approving pending CI.
Findings
- [NON-BLOCKING] src/cockpit/web/pages/ReviewerCostPage.test.tsx:26 — Test type-imports from the server-side widget module; prefer the dependency-free contract module to avoid future accidental value-imports
This test doesimport type { ReviewerCostPayload } from "../../widgets/reviewer-cost". Whileimport typeis erased at runtime, it couples the test to the server-only widget module. A future edit that adds or converts to a value import here could reintroduce the R2 regression shape (pulling Node-only deps into client/test context). Suggest switching the type import to../../widgets/reviewer-cost-contract— it’s intentionally dependency-free and is what the client hook uses.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| A cockpit page (not a home-page widget) renders, at minimum: (a) 30-day stacked daily-spend by token class with a $/review line; (b) a per-config cohort table; (c) cap-pin share; (d) an outlier tail of the ten most expensive reviews, each linking to its PR. | Unverifiable | UI scaffolding for all four sections is implemented and tested — see src/cockpit/web/pages/ReviewerCostPage.tsx (DailySpendSection, CohortTableSection, CapPinShareTile, OutlierTailSection) and src/cockpit/web/pages/ReviewerCostPage.test.tsx (happy-path assertions against fabricated data). However, live data rendering is deferred pending mt#4546 (accessor) and ask#10299 (chart library). The widget currently returns degraded by design (src/cockpit/widgets/reviewer-cost.ts). The chart is represented as a table placeholder pending the decision. |
| Lever effects are shown by cohort split on config fingerprint (mt#4556), not by deploy markers on a time axis — time annotations are confounded by traffic shifts; no confound caveat is rendered and no time predicate is used for the cohort split. | Unverifiable | Rendering shows a cohort table without any confound caveat (see CohortTableSection in ReviewerCostPage.tsx and the assertion in ReviewerCostPage.test.tsx). The query layer that enforces "no time predicate on the cohort split" is deferred to mt#4546 and is not implemented in this PR; therefore end-to-end compliance cannot be verified. |
| The page reads through mt#4546's accessor; no second query layer. | Unverifiable | src/cockpit/widgets/reviewer-cost.ts explicitly avoids querying review_timing and returns degraded until mt#4546 ships, honoring the "no second query layer" constraint. The accessor itself does not exist yet in-repo to verify integration against, so compliance cannot be fully verified. |
| Data-freshness is visible on the page. A stale or failed query renders as an explicit error, never as a zero. | Met | src/cockpit/web/hooks/useReviewerCost.ts throws a dedicated ReviewerCostNotYetAvailableError when the reason starts with the shared NOT_YET_WIRED_REASON_PREFIX and a plain Error otherwise. src/cockpit/web/pages/ReviewerCostPage.tsx renders a neutral "Not yet available" notice for the former and an urgent error panel for the latter. Tests in ReviewerCostPage.test.tsx cover both failure modes and assert that no happy-path sections render underneath error states. |
| Acceptance Test 1 — Loading the page shows the 30-day stacked chart with the $/review line, populated from live data. | Unverifiable | Charting is deferred by ask#10299 and the widget is intentionally degraded until mt#4546 lands. The section renders as a data table placeholder for now (DailySpendSection), not a chart with live data. |
| Acceptance Test 2 — Re-running the underlying aggregates as SQL reproduces the rendered numbers. | Unverifiable | This PR does not implement aggregates or the mt#4546 accessor; reviewer-cost.ts returns degraded. There are no live aggregates to re-run and compare. |
| Acceptance Test 3 — Killing the DB connection renders a visible error state, not zeros. | Met | Error handling distinguishes known-not-wired from genuine failures; the latter path renders an urgent error panel (data-testid="reviewer-cost-error"). ReviewerCostPage.test.tsx explicitly tests a "database connection failed" degraded reason mapping to the urgent error panel and verifies no zero-filled sections render. |
| Acceptance Test 4 — Two configurations present in the window appear as separate cohort rows. | Unverifiable | Rendering supports multiple cohorts and the test fabricates two rows including a null-fingerprint row (ReviewerCostPage.test.tsx). Live data via mt#4546 is not wired, so this cannot be verified against real data. |
| Acceptance Test 5 — A screenshot of the finished page at a realistic viewport is attached to the PR (uncropped). | Met | docs/evidence/mt-4557/reviewer-cost-not-yet-available.png added with provenance and md5 history documented in docs/evidence/mt-4557/README.md. The screenshot shows the current neutral-notice state at 1440×900 for /reviewer/cost. The earlier stale/out-of-repo reference was corrected. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| reviewerCostWidget | function | src/cockpit/widget-registry.ts:54-60 — registered under key "reviewer-cost" for API/widget routing, src/cockpit/web/hooks/useReviewerCost.ts:63 — client fetches /api/widget/reviewer-cost/data via widget-client | Adopted | Widget is wired into the central registry and consumed by the SPA via the TanStack Query hook; route /reviewer/cost renders the page using that hook. |
Documentation impact
- no-update-needed — This PR adds a new cockpit page route and widget wiring but deliberately ships as a shell pending mt#4546 and ask#10299. No user-facing docs previously asserted availability of this page; the only added docs are internal evidence files under
docs/evidence/mt-4557/(README + screenshot). No existing public docs are invalidated by the current neutral-notice state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the
/reviewer/costcockpit page skeleton for mt#4557 ("where is the reviewer's money going?"): route,WidgetModule(reviewer-cost), TanStack Query hook, and a page component with all four SC1 sections (30-day daily-spend table, per-config cohort table, cap-pin share tile, outlier tail) wired against a typed payload contract, plus loading/error/no-data/ok states and tests for each.This PR does not fully satisfy every success criterion yet — two decisions are reserved to the principal and were filed as
direction.decideasks (routed to the operator inbox) rather than decided unilaterally, per this dispatch's explicit instructions. Work that does not depend on either answer is complete; work that does is deferred with a marker below.Blockers filed as asks (not decided in this PR)
## Open decision for the principalreserves this: the cockpit has no chart library (only hand-rolledSparkline.tsx/RingGauge.tsxunder mt#2601's no-dependency constraint). SC1's primary view (stacked bar + $/review line) needs one or the other.review_timingaccessor) has no code. SC3 requires this page to read through mt#4546's domain-side accessor with no second query layer. I verified mt#4546's live state rather than trusting its IN-PROGRESS task status: its session was created 2026-08-25T03:22 and never touched again (stale, no commits, no PR, no accessor/command code anywhere in the repo). I did not implement mt#4546 myself and did not stand up a parallel query layer —reviewer-cost.ts'sfetch()explicitly returns adegradedstate citing the blocker. Proceeding on the ask's Option B (page shell now, wire the accessor once it ships) absent an operator reply.Both asks are still suspended in the operator's inbox as of this update.
Success-criteria coverage
[sc1-deferred: mt#4587]— chart rendering follow-up task, blocked on ask#10299 (currently a data table, not the bar+line chart).[sc1-deferred: mt#4546]— live numbers for all four sections (daily spend, cohort table, cap-pin, outlier tail) are blocked on the accessor; the rendering code for all four is built and tested against fabricated data.created_atpredicate to the cohort split. The module docblock documents this constraint for whoever wires mt#4546's accessor in.[sc2-deferred: mt#4546]for the actual query enforcing it.[sc3-deferred: mt#4546]. The widget deliberately does not queryreview_timing.Review-fix history on this PR
NOT_YET_WIRED_REASON_PREFIX; hook throws a distinctReviewerCostNotYetAvailableErrorfor that known case; page renders a neutral "Not yet available" notice instead of the urgent red error panel for it, reserving that panel for real failures. New tests cover both branches.db-providers.ts) into the Vite client bundle and crashed the page at runtime withprocess is not defined. Fixed by extracting the shared contract into a dependency-freereviewer-cost-contract.tsboth sides import safely.docker-build-smoke (services/cockpit/Dockerfile)and the Railwaydeploycheck both failed because the Dockerfile's builder stage never copiedsrc/cockpit/widgets/, and this PR's hook is the first cockpit-web module to import across that boundary. Fixed by adding theCOPYline./tmppath reference nobody but the authoring session could open (reviewer marked AT5 Unverifiable, correctly), AND that referenced screenshot was itself stale — captured before R1 landed, so it showed the exact always-erroring red banner R1's BLOCKING finding was about. Fixed by re-capturing against current HEAD and committing the PNG underdocs/evidence/mt-4557/(the repo's established convention), with all four captures across this PR's history md5-summed in the README. Full check set confirmed green on this commit (22/22) before the branch fell behind.mergeable_state: "behind". Brought current viasession_update; verified the push independently (git rev-parse HEADon the session matchesgit ls-remote origin refs/heads/task/mt-4557— not just a bare success flag). Reviewer bot re-approved on the new head in 2.3s (18:48:22Z, before an unrelated OpenAI-credits outage that hit ~18:55Z), 0 findings.Execution evidence
Test run (
bun scripts/run-related-tests.ts src/cockpit/web/pages/ReviewerCostPage.test.tsx src/cockpit/widgets/reviewer-cost.test.ts) on the pre-R5 commit:SPA-router + server-fallback probes (dev server, per directive 21):
/reviewer/cost-> text/html (200);/api/widget/reviewer-cost/data-> JSON (200);/main.tsx-> text/javascript (200) — non-SPA paths unaffected.CI status on current head (
7b81b3ea6, post session_update): a fresh workflow run triggered by the merge from main is in progress — the R4 commit's full-green result (22/22) does not carry forward to this head. Do not treat this PR as CI-green until re-confirmed on7b81b3ea6specifically.Render-path evidence (AT5)
Full uncropped screenshot at 1440×900, showing the page's real current state (the neutral "Not yet available" notice — no chart-library or mt#4546 answer yet, so there is no live data to show):
Provenance, method, and md5 sums for all four captures across this PR's review history (including the two that were wrong, and why):
docs/evidence/mt-4557/README.md. The frontend hasn't changed since this capture (R5 was a merge-from-main sync, no cockpit code touched by main's incoming commits as far as this page is concerned), so it remains representative of the current render.The "ok" branch (daily-spend table, cohort table with the null-fingerprint "unknown configuration" row, cap-pin tile, outlier-tail PR link, quality-not-shown disclaimer) is exercised by
ReviewerCostPage.test.tsxagainst fabricated data and will render live once both asks resolve.Testing
src/cockpit/widgets/reviewer-cost.test.ts— locks the mt#4546 boundary itself:fetch()always resolvesdegraded, neverok, never throws, and its reason always starts with the sharedNOT_YET_WIRED_REASON_PREFIX.src/cockpit/web/pages/ReviewerCostPage.test.tsx— failure path first (both the neutral "not yet available" branch AND a genuine-live-failure branch, asserted as mutually exclusive), no-data path, then theokbranch against fabricated data.Follow-up
Scope note
Per bounds given for this dispatch, mt#4546, mt#4555, mt#4569, and mt#4577 were not touched (mt#4577 is mid-run again on its own branch). Not merging this PR myself — that decision belongs to the operator.
[no-deploy-impact]