Repository navigation
[3440 by Claude Opus 5.5] Report cost per merged PR by model and effort - #3494
Conversation
…s down by implementer model and effort. Only the API typechecks; the CLI and UI typecheck too, and lint passes for the API and UI files I changed with `--max-warnings 0` (the CLI package has no ESLint config, so nothing lints its files). The new and existing tests pass: - API: the review-score suites plus the nearby analytics, stats, cost-export and MCP overview suites. - CLI: 4/4. - UI: the Vitest tests and the 15 Playwright tests in `analytics-timeframe.pw.ts`. API (`packages/api/routes/reviewScoreStats.ts`) - The summary has a new `by_effort` list. Each row has the same figures as a `models` row, for one model at one effort level, plus `implementer_reasoning_level`. - A PR's effort comes from the same score row that gives its model. PRs with no recorded level, including empty strings and historical data, go under an explicit `unknown` value and are never dropped. Each model's effort rows add up to its `models` row. - Rows follow the order of `models`, then effort from lowest to highest, with `unknown` last. - `models` keeps its meaning, so the per-model totals and the overview's score figures are unchanged. - If a database doesn't have the `implementer_reasoning_level` column yet, every PR is reported as `unknown` instead of the query failing. - CSV format change: the CSV now has one row per model and effort, with a new `implementer_reasoning_level` column after `implementer_agent`. Anything that reads this CSV expecting one row per model will need updating; the per-model totals are still in the JSON `models` list. CLI (`packages/cli`) - The TypeScript type now includes `score_delta`, `runs_to_merge` and the effort rows. - `propr stats review-scores` prints one row per model and effort, with new Effort, Merged, and Runs to merge columns. If the server is older and doesn't send effort rows, it falls back to per-model rows with the effort shown as `(all)`. UI (`ReviewQualityByModel.tsx`) - A model with more than one effort level gets an indented row per level under its totals, with "Unknown effort" last. - A model with exactly one known level gets a small tag next to its name instead, such as "Medium effort". - Untracked work and older servers look the same as before. - I didn't add a cost-per-merged-PR column to this table; it didn't have one before, and cost is in the CSV and CLI. The issue only asked for effort as a secondary grouping here. Tests and docs - New tests cover grouping, ordering, the `unknown` bucket, the missing-column case, the CSV columns, and the CLI rows and table. The existing fixtures now include effort levels, and the e2e mock includes effort rows. - The API tests for the effort split are in a new file, `packages/api/test/reviewScoreEffort.test.ts`, because adding them to the existing test file would have pushed it over the 400-line lint limit. - `metrics.md`, `propr-cli.md` and `CHANGELOG.md` are updated. A few of the related API suites only run with `PROPR_DEMO_MODE=true` in this container. Without it they stop at the GitHub-auth message, which is an environment issue, not caused by this change. The preview is a screenshot of the Analytics pane with mocked data, captured by the existing e2e test.
|
/ultrafix goal=8 max=10 |
|
🔄 Ultrafix loop started (goal: 8/10, max cycles: 10) First action:
|
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR adds effort-level reporting across the API, CSV, CLI, and UI while preserving per-model JSON totals. The implementation is coherent and merge-ready within scope, conditional on the two pending checks passing. ✅ Consistent aggregation — Effort groups reuse the same PR facts and summary function as model totals, keeping metric definitions consistent. ✅ Historical-data compatibility — Missing, empty, and whitespace-only effort values resolve to ✅ Client compatibility — The CLI and UI handle older responses without This review used static analysis of the supplied diff and context. No commands or tests were run. The authoritative check summary reports no failures, 32 passed, and two pending. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Test changing attribution across reviewsAdd a fixture where successive reviews of one PR record different effort values, including a historical null value. Assert that the PR belongs to exactly one effort bucket and that its effort comes from the same selected score row as its model. The existing fixtures exercise grouping well but keep attribution constant within each PR. This is optional regression coverage; the changed implementation reads both attributes from ScoreThe change preserves existing aggregation semantics, handles compatibility explicitly, and includes focused tests across all affected surfaces. No merge-blocking defect was identified; final readiness remains conditional on the pending checks. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
CI failed: Validate Changes Please investigate and fix this CI failure.
Failure evidence
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit 40cbe21 Processed 2 comments:
Summary of ChangesI fixed the UI lint failure. I shortened the file without changing any data or what the tests check:
Checks I ran:
The "Slow Test" note about 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…ame.pw.ts` had 405 counted lines against the 400-line `max-lines` limit, because this branch added effort rows to its mock data. It now passes. I fixed the UI lint failure. `propr-ui/e2e/analytics-timeframe.pw.ts` had 405 counted lines against the 400-line `max-lines` limit, because this branch added effort rows to its mock data. It now passes. I shortened the file without changing any data or what the tests check: - **Review-score mock:** the model and agent fields are now defined once, as `claude` and `gpt`, and a small `effort()` helper builds the four effort rows. The `models` and `by_effort` lists now take five lines instead of nine. - **Drill-down test:** three `expect(...)` calls that were split across two lines are now on one line each. Checks I ran: - `eslint e2e/analytics-timeframe.pw.ts --max-warnings 0` and the full `npm run lint` in `propr-ui` both pass. - `tsc --noEmit` passes. - All 15 Playwright tests in `analytics-timeframe.pw.ts` pass. The "Slow Test" note about `creation-short-window.pw.ts` in the CI summary is only a timing report, not a failure, and this branch doesn't touch that file. Nothing changes on screen, so there is no new visual preview. PR: #3494 Comment by: @github-actions[bot] (ID: 6828733695) Comment by: @propr-dev[bot] (ID: 6828733526) Model: Claude Opus 5.5
AI Implementation Summary
Closes #3440
Branch:
3440/claude-opus-5-5-report-cost-per-merged-pr-20261112-1447-a37Commits: ✅ Changes committed (4641e7b)
AI Processing Completed
Execution Details:
Summary:
Issue #3440 is implemented: the review-score report now breaks results down by implementer model and effort. Only the API typechecks; the CLI and UI typecheck too, and lint passes for the API and UI files I changed with
--max-warnings 0(the CLI package has no ESLint config, so nothing lints its files). The new and existing tests pass:analytics-timeframe.pw.ts.API (
packages/api/routes/reviewScoreStats.ts)by_effortlist. Each row has the same figures as amodelsrow, for one model at one effort level, plusimplementer_reasoning_level.unknownvalue and are never dropped. Each model's effort rows add up to itsmodelsrow.models, then effort from lowest to highest, withunknownlast.modelskeeps its meaning, so the per-model totals and the overview's score figures are unchanged.implementer_reasoning_levelcolumn yet, every PR is reported asunknowninstead of the query failing.implementer_reasoning_levelcolumn afterimplementer_agent. Anything that reads this CSV expecting one row per model will need updating; the per-model totals are still in the JSONmodelslist.CLI (
packages/cli)score_delta,runs_to_mergeand the effort rows.propr stats review-scoresprints one row per model and effort, with new Effort, Merged, and Runs to merge columns. If the server is older and doesn't send effort rows, it falls back to per-model rows with the effort shown as(all).UI (
ReviewQualityByModel.tsx)Tests and docs
unknownbucket, the missing-column case, the CSV columns, and the CLI rows and table. The existing fixtures now include effort levels, and the e2e mock includes effort rows.packages/api/test/reviewScoreEffort.test.ts, because adding them to the existing test file would have pushed it over the 400-line lint limit.metrics.md,propr-cli.mdandCHANGELOG.mdare updated.A few of the related API suites only run with
PROPR_DEMO_MODE=truein this container. Without it they stop at the GitHub-auth message, which is an environment issue, not caused by this change. The preview is a screenshot of the Analytics pane with mocked data, captured by the existing e2e test.Detailed Logs:
9de77243-566d-4275-99f7-4ad0d90c6df8Log files stored at:
/usr/src/app/logs/executions/issue-3440-2026-11-12T15-05-35-517Z-conversation.json/usr/src/app/logs/executions/issue-3440-2026-11-12T15-05-35-517Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3440.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!
Visual preview
Agent efficacy by model, with effort
Analytics 'Agent efficacy by model' pane at 1440px: Claude Opus 5.5's totals with indented rows per effort (High, XHigh, Unknown last); GPT-5.6 runs at a single level, so it gets a 'Medium effort' tag instead (mocked API data)
Connect sign-in required. Original retained until 2027-02-10T15:05:41.115Z.
View the full-resolution original in ProPR Connect