Skip to content

fix: show roborev review costs#448

Merged
cpcloud merged 7 commits into
mainfrom
codex/middleman-reviews-cost
Jun 7, 2026
Merged

fix: show roborev review costs#448
cpcloud merged 7 commits into
mainfrom
codex/middleman-reviews-cost

Conversation

@cpcloud
Copy link
Copy Markdown
Collaborator

@cpcloud cpcloud commented Jun 7, 2026

  • Adds a Cost column to roborev review jobs in the Reviews tab.
  • Formats priced roborev token usage as ~$x.xx and leaves -- when cost data is missing or unpriced.
  • Covers cost rendering in the roborev row component test, the workspace Reviews-tab e2e path, and the SQLite-backed roborev e2e seed.

Roborev jobs already expose priced token usage in the proxied job payload, but Middleman's Reviews table only showed timing and status columns. That made the cost visible in roborev's own TUI but not in the dashboard scan line.

Add a Cost column that mirrors roborev's estimate format only when token_usage has has_cost=true and a numeric cost_usd value. Rows without a priced estimate keep a quiet placeholder so the column stays stable without implying an unpriced token count is a dollar cost.
@cpcloud
Copy link
Copy Markdown
Collaborator Author

cpcloud commented Jun 7, 2026

I tried this out locally, works.

@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented Jun 7, 2026

roborev: Combined Review (9f572b1)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 1m57s), codex_security (codex/security, done, 28s) | Total: 2m25s

The CI lint job runs the workspace-wide Vite+ check, which also validates existing tracked frontend tests outside this feature's main diff. Two tests were not in the current formatter shape, causing the PR lint job to fail before the roborev cost change could be evaluated.

Apply the formatter's mechanical line wrapping only, with no behavior changes.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented Jun 7, 2026

roborev: Combined Review (cf30977)

Summary verdict: One medium test coverage gap remains; no security findings were reported.

Medium

  • frontend/tests/e2e/workspace-sidebar.spec.ts:3122
    The cost column is only covered by a route-mocked Playwright test, so it does not exercise the roborev proxy or seeded SQLite data. The existing full-stack fixture still uses token_usage shaped as {"input":...,"output":...}, which would not render a cost here and could let the real flow ship as --.

    Fix: Add a cost-bearing token_usage fixture and assert .col-cost in frontend/tests/e2e-full/roborev-e2e.spec.ts through the real e2e server/proxy.


Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 2m56s), codex_security (codex/security, done, 34s) | Total: 3m40s

cpcloud added 2 commits June 7, 2026 10:28
CI exposed two unrelated problems on this branch: frontend unit teardown could schedule inline diff annotation mounts while DiffFile was being destroyed, and golangci-lint flagged older test helper patterns already present in the tree.

DiffFile now defers imperatively mounted annotation components until their host is connected and disables those deferred mounts during teardown, which prevents child onMount effects from running inside Svelte cleanup. The Go test fixtures also use the current go-github pointer helper and testify's float-aware assertion so the lint job can pass.
The deprecated go-github String helper was replaced with gh.Ptr to satisfy staticcheck, but the modernize linter still flags that shape. Use local fixture strings and take direct pointers, which matches the surrounding test setup and satisfies both lint rules.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented Jun 7, 2026

roborev: Combined Review (0aa45fb)

Summary verdict: one Medium test coverage issue remains; no security issues were found.

Medium

  • frontend/tests/e2e/workspace-sidebar.spec.ts:3118
    The new cost display is covered only through mocked Playwright data and a component test. This repo requires user-visible data-flow changes to have full-stack e2e coverage through the real HTTP API and SQLite, so current coverage would not catch regressions in how review_jobs.token_usage is persisted, served, and rendered.
    Fix: Add or update a frontend/tests/e2e-full/roborev-e2e.spec.ts case and fixture data with cost-bearing token_usage, then assert the Reviews table renders the cost from the real SQLite-backed API.

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 2m35s), codex_security (codex/security, done, 36s) | Total: 3m20s

Firefox and WebKit CI were failing in existing browser-sensitive e2e paths after the cost-column work was otherwise green. The failed assertions depended on focus reporting and pointer hit-testing through shadow DOM and overlay-heavy controls, which can vary by browser while the user-visible workflows still work.

Activate the resolved controls directly in the specs and keep the inline composer test focused on visibility, typing, save, and draft persistence instead of a transient focus state.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented Jun 7, 2026

roborev: Combined Review (3a372b2)

No blocking security issues; one Medium test coverage gap should be addressed.

Medium

  • packages/ui/src/components/roborev/JobRow.svelte:102
    The new cost column is only covered by a component test and a mocked Playwright test. The existing full-stack roborev e2e suite does not seed a cost-bearing token_usage value or assert .col-cost, so the real proxy/seeded-daemon path could regress unnoticed.
    Fix: Add a cost-bearing roborev fixture job and assert the rendered ~$... cost in frontend/tests/e2e-full/roborev-e2e.spec.ts.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 4m46s), codex_security (codex/security, done, 15s) | Total: 5m8s

Firefox CI still failed after the first cross-browser e2e cleanup because the virtualized file-tree scroll wrapper intercepted pointer clicks on visible tree items. The target buttons were present and stable, but Playwright could not deliver a pointer click through Firefox hit-testing.

Use a shared helper to activate resolved file-tree items for navigation-oriented specs, while keeping the follow-up assertions on selected rows, visible target files, and rendered previews as the behavior checks.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented Jun 7, 2026

roborev: Combined Review (758d7be)

Summary verdict: One medium test coverage gap remains; no security findings were reported.

Medium

  • packages/ui/src/components/roborev/JobRow.svelte:102
    The new roborev cost column is only covered by a component test and a mocked sidebar e2e. There is no full-stack roborev/SQLite e2e assertion that real job data with token_usage.cost_usd renders as a cost.
    Fix: Add a seeded roborev job with cost-bearing token_usage and assert the Cost header/value in frontend/tests/e2e-full/roborev-e2e.spec.ts.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 3m51s), codex_security (codex/security, done, 51s) | Total: 4m49s

roborev-ci pointed out that the cost column was only covered by component and mocked workspace tests. That left the SQLite-backed roborev e2e path unpinned, even though that is where schema drift in token_usage would show up.

Seed the deterministic top-row roborev job with cost-bearing token_usage and assert the Cost header/value in the full-stack roborev e2e spec. Local roborev e2e reached the daemon build/start path with the CI-pinned roborev ref, but stopped when Playwright tried to sudo-install host browser dependencies in this environment.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented Jun 7, 2026

roborev: Combined Review (4240e11)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 3m3s), codex_security (codex/security, done, 59s) | Total: 4m2s

@cpcloud cpcloud merged commit cb6effb into main Jun 7, 2026
16 checks passed
@cpcloud cpcloud deleted the codex/middleman-reviews-cost branch June 7, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant