docs(quality): record the review cost model from measured numbers - #2962
Conversation
Written after a single day of development consumed roughly a month's model budget, from numbers taken off real billing lines rather than estimated: 2.17 EUR per 1M input tokens, 0.22 cached, 13.00 output, and about 25k tokens per reviewed file measured across three thousand file reviews. The useful part is not the prices. Three multipliers made that day expensive, and none of them was the price per token: a memoization store that never persisted (every push re-read every file, and the diagnostics said so on every run while nobody read them), the same audit running on two branches so every review was paid twice, and a ceiling a large change can exhaust — which until Keiko-for-Quality#75 meant such a change could never converge, so a LOWER ceiling made it worse rather than better. Includes the ten-second check for whether memoization is actually working, because its failure mode is invisible: cache.store_loaded reporting zero entries on a pull request that has been pushed to before is the expensive case, and it looks exactly like a healthy first run. The workflow's own budget note is updated: the #75 fix has merged upstream, so the caveat now says what still applies until the pin advances rather than describing the fix as pending.
GitHub Actions
Inactive
|
Keiko for Quality — run summary ✅ complete · head
Budget: 121082 tokens allotted |
GitHub Actions
Inactive
The profile's docs/qa/**/*.md entry wins over the broader prose exclusion, which the first review of this very pull request demonstrated by reporting two reviewable files rather than one. Worth stating where someone predicts what a change will cost: 'it is only documentation' is not the same as 'it is not reviewed'. This commit deliberately leaves the workflow file byte-identical, so the review of this push must report a cache hit for it if memoization works.
GitHub Actions
Inactive
GitHub Actions
Inactive
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abfdd75950
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Warning Review limit reached
Next review available in: 10 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request updates workflow documentation and adds operator guidance for model-review spending. The documentation covers verdict persistence, pricing, caching, incomplete runs, diagnostics, token budgets, artifact verification, and emergency shutdown procedures. ChangesReview documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/qa/review-cost-model.md`:
- Around line 58-61: Label the diagnostic code fence surrounding the cache
metrics example as text by adding the text language tag to its opening fence,
while leaving the example contents unchanged.
- Around line 63-66: Update the documentation around the entries: 0 diagnostic
to qualify it with the store identity and enabled-store state before calling it
a persistence failure. Distinguish legitimate cold starts caused by profile,
model, protocol, or action-pin changes, retention expiry, or disabled storage
from a failure, and require the same store identity across pushes for the
failure condition.
- Around line 41-44: Revise the “A memoization store that never persisted”
paragraph so “full price” refers specifically to workflow memoization rather
than provider billing. Do not claim the reviews were “uncached” or fully billed
unless supported by billing data; preserve the documented distinction that
provider prompt caching may still reduce repeat costs.
- Line 68: Correct the cache-count example in the review cost model so it uses a
file-set variable such as N and reports an unchanged re-push as hits: N, misses:
0, or explicitly documents the separate miss before using hits: N-1. Also verify
the memoization acceptance criterion against the pinned tooling and actual
cache.hits counts in keiko-for-quality.yml, updating the documented contract if
needed.
- Around line 80-82: Update the documentation around vars.KEIKO_QUALITY_ENABLED
to limit the emergency-stop statement to the keiko-for-quality workflow’s review
job. Remove the repository-wide claim that nothing else can spend model budget,
unless the documented switch is verified to disable every referenced
model-backed path; preserve the existing secret-materialization and spending
behavior for that workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: de425b83-d95e-4b62-a387-3af087ea487c
📒 Files selected for processing (2)
.github/workflows/keiko-for-quality.ymldocs/qa/review-cost-model.md
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Cross-platform smoke (ubuntu-latest)
- GitHub Check: Cross-platform smoke (macos-latest)
- GitHub Check: Cross-platform smoke (windows-latest)
- GitHub Check: ui
- GitHub Check: workflow hygiene
- GitHub Check: Coverage shard (packages 2/3)
- GitHub Check: Coverage shard (packages 3/3)
- GitHub Check: Core quality
- GitHub Check: Coverage shard (packages 1/3)
- GitHub Check: Coverage suite (keiko-ui)
- GitHub Check: Build, scan, SBOM, smoke
- GitHub Check: Coverage suite (scripts)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,mjs,cjs,json,md,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Use English only in code, comments, identifiers, documentation, commit messages, issues, and pull requests.
Files:
docs/qa/review-cost-model.md
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Pin every GitHub Action to a full 40-hex commit SHA and retain a version comment; tag or branch refs are prohibited.
Files:
.github/workflows/keiko-for-quality.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Treat workflow changes as supply-chain and merge-authority changes. Require full 40-hex
action pins with version comments, minimum permissions, bounded timeouts, locked installs,
redacted output, and exact-current-head evidence. Never recommend bypassing a gate.
Files:
.github/workflows/keiko-for-quality.yml
🪛 LanguageTool
docs/qa/review-cost-model.md
[uncategorized] ~3-~3: The official name of this software platform is spelled with a capital “H”.
Context: ...t makes it expensive Operator note for keiko-for-quality.yml. Writt...
(GITHUB)
[style] ~51-~51: Consider using a shorter alternative to avoid wordiness.
Context: ...d never converge, and a lower ceiling made that worse rather than better by turning more pull...
(MADE_IT_JJR)
🪛 markdownlint-cli2 (0.23.1)
docs/qa/review-cost-model.md
[warning] 58-58: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
docs/qa/review-cost-model.md (1)
1-40: LGTM!Also applies to: 46-57, 70-79
.github/workflows/keiko-for-quality.yml (1)
444-451: 🔒 Security & PrivacyNo action needed.
The pinned commit is exact
v0.10.0and remains behind Keiko-for-Quality#76, so the stated release boundary is accurate.
Five substantive findings, all of them right, all taken. The emergency stop was described as one switch. It is two: the variable stops new jobs, but a run already requested, queued, or in progress keeps its credentials and can spend until the job timeout. Describing the variable alone as ensuring "nothing is spent" would prolong exactly the budget incident this document exists to contain. Both steps are now stated, with the commands. "entries: 0 means the store is not persisting" was wrong in several ordinary cases. A cold start is correct after a profile, model, protocol, or pin change, after retention expiry, after a previous run that persisted nothing, and when the store is disabled — the first four because they deliberately begin a fresh partition. Persistence is suspect only when a prior run under the SAME identity completed inside the retention window. "hits: M-1, misses: 1" used M both as the miss count and as a file total, and asserted a fixed one-miss rule that would make an operator diagnose a healthy store as broken. Hits and misses count files: three changed files of twenty reads hits 17, misses 3. The 25k-tokens-per-file figure did not reconcile with the token counts in the same document — 4.6k per file by those. Both are real and measure different windows: 25k comes from the day's total ledger spend, while only about 18% of that usage had materialized in the billing API when this was written. The document now says so instead of leaving a 5x gap unexplained. And the workflow note claimed advancing the pin would end the repeated cost. It would not: the hand-off and the signing job both gate on `outcome == 'complete'`, so a truncated run's store would be written here and stranded here. Keiko-for-Quality#78 adds a `store_written` output for exactly this, and the note now says the adopting repin must move both conditions to it.
GitHub Actions
Inactive
GitHub Actions
Inactive
CodeRabbit is right that 'full price' overstated what the diagnostics prove. cache.store_loaded and cache.hits measure THIS workflow's memoization; they say nothing about what the provider billed, and the provider's prompt cache demonstrably applied — the 7.4M cached tokens in this same document are where it shows. The repeats were cheaper than a first review, just not free. The paragraph now says the model call was dispatched again rather than claiming it was billed uncached, and states the relationship the two mechanisms actually have: caching makes a call cost less, memoization removes it.
GitHub Actions
Inactive
![keiko-for-quality[bot]](https://avatars.githubusercontent.com/in/4290143?s=60&v=4){kind=small}
GitHub Actions
Inactive
Keiko for Quality found this on its own adoption, and it is the sharpest kind of documentation defect: text that is true upstream and false in the repository the reader is standing in. The paragraph described non-convergence in the past tense, as something Keiko-for-Quality#75 ended. In this repository the pin is v0.10.0, which does not carry that fix, and the workflow still gates both the store hand-off and the signing job on outcome == 'complete'. An operator reading the runbook would have diagnosed a large pull request as healthy and looked for the cost somewhere it is not. It now says the condition still holds here, names both steps adoption requires — advance the pin AND move both conditions onto store_written, since budget exhaustion continues to settle INCOMPLETE — and gives the budget response that is actually available today: split the change.
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/qa/review-cost-model.md`:
- Around line 80-84: Update the workflow documentation to state that stores are
uploaded and signed only when outcome == 'complete'; budget-exhausted runs may
persist partial data locally but do not make it available to the next push, so
entries: 0 is expected. Distinguish this gate from schema or engine failures
that write nothing, and incorporate the condition into the existing
same-identity retention-window persistence guidance.
- Around line 118-120: Update the gh run list command in the active-run
cancellation snippet to retrieve all non-completed keiko-for-quality runs, using
an explicit sufficiently large --limit or repeating the query until none remain.
Preserve the existing jq filtering and gh run cancel invocation so every active
run is cancelled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1fc768df-4c75-4abe-961e-621d96e659ed
📒 Files selected for processing (2)
.github/workflows/keiko-for-quality.ymldocs/qa/review-cost-model.md
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: workflow hygiene
- GitHub Check: review
- GitHub Check: Secret scan
- GitHub Check: ui
- GitHub Check: Core quality
- GitHub Check: Coverage shard (packages 1/3)
- GitHub Check: Coverage shard (packages 3/3)
- GitHub Check: Coverage suite (scripts)
- GitHub Check: Coverage shard (packages 2/3)
- GitHub Check: Build, scan, SBOM, smoke
- GitHub Check: Semantic duplication
- GitHub Check: Coverage suite (keiko-ui)
- GitHub Check: Change scope
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Pin every GitHub Action to a full 40-hex commit SHA and retain a version comment; tag or branch refs are prohibited.
Files:
.github/workflows/keiko-for-quality.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Treat workflow changes as supply-chain and merge-authority changes. Require full 40-hex
action pins with version comments, minimum permissions, bounded timeouts, locked installs,
redacted output, and exact-current-head evidence. Never recommend bypassing a gate.
Files:
.github/workflows/keiko-for-quality.yml
**/*.{ts,tsx,js,jsx,mjs,cjs,json,md,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Use English only in code, comments, identifiers, documentation, commit messages, issues, and pull requests.
Files:
docs/qa/review-cost-model.md
🪛 LanguageTool
docs/qa/review-cost-model.md
[style] ~58-~58: Consider using a shorter alternative to avoid wordiness.
Context: ...d never converge, and a lower ceiling made that worse rather than better by turning more pull...
(MADE_IT_JJR)
[uncategorized] ~77-~77: The official name of this software platform is spelled with a capital “H”.
Context: ... or leaves none to find: - a change to .github/keiko-for-quality.json, the model id, ...
(GITHUB)
[locale-violation] ~122-~122: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...ble procedure, including what to record afterwards, is in [keiko-for-quality.md](keiko-f...
(AFTERWARDS_US)
🔇 Additional comments (8)
docs/qa/review-cost-model.md (7)
50-51: Use workflow-memoization wording for the cost multiplier.
cache.store_loadedandcache.hitsshow that the workflow store did not reuse entries. They do not show that provider prompt caching was absent. Replace “full-price” with “without workflow memoization” unless the ledger separately proves uncached billing.
15-18: LGTM!Also applies to: 20-21, 25-33
53-59: LGTM!
63-72: LGTM!
86-92: LGTM!
101-103: LGTM!
105-113: LGTM!.github/workflows/keiko-for-quality.yml (1)
444-459: LGTM!
… cold-start list Three findings on the incident path, which is the one place a documentation error costs money. The cancellation snippet was a simplified copy of the procedure in keiko-for-quality.md and wrong in two ways the reviewers named: it selected the workflow by NAME where the authoritative runbook uses the file, and it trusted `gh run list`'s default window of twenty runs, so an incident with more runs than that would have left live ones spending after the "emergency stop". The real procedure checks the limit rather than trusting it, loops until nothing live remains, covers all five live statuses, and treats every failed call as containment NOT established. None of that survives being copied. So it is not copied: this document now states the two steps and points at the one procedure, which removes the whole class of drift rather than patching this instance of it. The cold-start list also said a previous incomplete run persists nothing "for a reason that persists nothing". In this repository that qualifier is wrong for a different reason: the hand-off and the signing job both gate on outcome == 'complete', so even a run that persisted verdicts locally never uploads them. entries: 0 after ANY incomplete run is expected here until the workflow adopts store_written.
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea2941bc77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…perators find it Two corrections, the first of them to an over-correction of my own. I had widened "entries: 0 after an incomplete run" to ANY incomplete run. That is wrong, and wrong in the dangerous direction: the locator sorts every same-named artifact by creation time and takes the newest eligible one, so an earlier COMPLETE store still inside the seven-day retention window is found even after an incomplete run uploads nothing. Calling a cold start expected there would have masked a genuine lookup or authentication regression and left the repository paying for full reviews. It is expected only when no such artifact remains. And the emergency procedure named the variable as `vars.KEIKO_QUALITY_ENABLED`. `vars.` is the Actions expression prefix, not part of the name — an operator following it literally during a budget incident would search for a variable that does not exist. The bare name is used, matching the authoritative procedure.
GitHub Actions
Inactive
…um of 6M Owner decision: take the ceiling to the highest defensible value. That value is determined by the code rather than chosen — the action's own allotment formula clamps at ALLOTMENT_CEILING = 6,000,000 before this input is applied, as `min(token_budget, clamp(...))`. Anything larger here is inert and would only suggest headroom the reviewer cannot use. Why raising it is the cheaper direction, not a relaxation: a run that exceeds its allotment settles incomplete, an incomplete run hands off no store, and a pull request that never completes therefore never seeds one — it pays in full on every push. A ceiling low enough to be hit is self-defeating; one high enough for the first review to finish pays once and replays afterwards. The 2M cap set yesterday under budget pressure was made against an inert store, and that premise is gone: a production run has now recorded a cache hit. What the edge costs, stated rather than left to be discovered: the allotment reaches 6M at about 115 reviewable files, and a run consuming all of it costs roughly 7 to 13 EUR depending on how much the provider serves from its prompt cache. That is one first review of a change larger than any this repository has produced, and whether it recurs is decided by the store, not by the ceiling. Both thresholds in the cost model are recomputed: the allotment meets the cap around 115 files, truncation lands around 240.
GitHub Actions
Inactive
GitHub Actions
Inactive
Keiko for Quality read this comment as resting on the upstream truncated- store fix, and concluded that raising the ceiling overstates the improvement. The premise it attributed is not the one the comment makes — the argument is that an incomplete run persists NOTHING here, which is precisely why a ceiling low enough to be hit is a false economy. But the misreading was earned: the block had grown across three ceiling changes and described 2M, 4M and 6M at once, including a line I left saying dispatch stops past 4M. Rewritten once, for the current state, in the order a reader needs it: what the ceiling is and why 6M is the maximum that does anything (the allotment formula clamps there, so more is inert); why a high ceiling is the cheap setting, stated explicitly as an argument that does NOT depend on any upstream change and holds at the pin this workflow runs today; what the upstream fix would add and the two things that must land together before it does anything here; and what the edge costs. No behaviour change. The pin and the hand-off conditions deliberately do not move in this pull request: adopting them requires a released v0.11.0, which requires the qualification double run, and doing it inside a documentation change would skip that gate.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
* fix(settle): a fully memoized change is complete, not incomplete The memoization success case was being punished, and it got worse the better the store worked. Production evidence, oscharko-dev/Keiko#2962: two reviewable files, two cache hits, zero misses. Every path was answered from the store, so the engine was handed nothing to dispatch and reported `skipped` — correctly. The status check read that as a failed run and settled incomplete, so a pull request that was in fact fully answered received a blocking "this change was not fully reviewed" notice. Every push where the store did its job produced one. With nothing dispatched, the engine's own status says nothing about this change's coverage: every reviewable path carries a replayed verdict by construction. So a run with no unmemoized paths settles complete, at the one place both settlement modes pass through. The disqualifiers still apply. Nothing dispatched does not mean nothing to object to — an unlisted warning or an implausible finding count is about the run, not about dispatch — and a pinned case covers that. A second pin covers the boundary: a store that answered only SOME paths still refuses. Proven red by removing the guard. * fix(test): use the real EngineWarning shape in the disqualifier pin The new all-hits pin constructed a warning with a message field that EngineWarning does not have. vitest transpiles without checking types, so the suite went green while tsc did not — the trap this repository's own guidance names, and I walked into it while proving a fix for a different one. Worse, my verification flow reported it and continued: npm run verify was piped into grep, and the failure did not stop the commit. That is the same class as verifying through a pipe, one layer up: a gate whose result is read but not acted on is not a gate. The flow now writes the log to a file and fails on a non-zero exit before anything is committed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bd3850a41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…to the pull request Two corrections, and the first is the third time I have dropped the same qualifier while rewriting the same paragraph. So it is no longer written as a qualifier: the fact is stated once, up front, and given a name the rest of the block leans on. An incomplete run uploads no replacement store, but the locate step scans every same-named artifact and takes the newest eligible one — so a pull request that has completed at least once under its current identity keeps replaying that store until retention expires. "Starts cold on every push" is true only of a pull request that has NEVER completed under its current identity. That is the cold set, and the ceiling argument is about moving pull requests out of it. The producer-run lookup filtered by branch alone. A reused head branch mixes in the previous pull request's runs, and artifacts partition by pull-request number — so a complete run belonging to the old pull request would read as a producer for the new one, turning an expected empty store into a phantom lookup or HMAC failure. The step now verifies the candidate run's pull-request association before applying the retained-store predicate.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bebd7830fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…uld not find their target The 135-file row said 3.40 EUR. That was 3.4M tokens written as euros — a units error, and it sat below the floor of both derivations this document already contains. The ledger average of 96 EUR over roughly 3,000 file reviews gives 4.32; scaling the 7 to 13 EUR that a full 6M run costs down to 3.375M gives 3.94 to 7.31. The row now reads 4.30 to 7.30 and the derivation is written out, so the next reader can check it rather than trust it. The same row also still said "under 4M" after the ceiling moved to 6M. Two diagnostics could not retrieve what the runbook then asks the operator to act on: The producer-run check demanded that a candidate run's pull_requests array contain this pull request's number. The workflow's own locator accepts a run whose association GitHub omitted, falling back to head branch and repository — so the diagnostic was stricter than the code it diagnoses and would dismiss a run the reviewer itself would have used. It now mirrors the locator exactly. And the reason lookup searched only settlement.* codes while the resolution table includes inventory.unclassified_path, which fails a run without emitting any settlement code at all. The one reason whose remedy is a profile change was unreachable through the runbook's own commands.
GitHub Actions
Inactive
GitHub Actions
Inactive
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6631bf0ec3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `settlement.incomplete.coverage_gap` | Compare the engine's coverage against the inventory — a path the profile classifies as reviewable that the engine did not select | | ||
| | `settlement.incomplete.engine_status_not_success` | The engine reported failure; `reviewed`/`expected` on the reason give the size of the gap | | ||
| | `settlement.incomplete.schema_rejected`, `…engine_error` | The manifest is not to be believed — check the engine pin and its output, not the change size | | ||
| | `settlement.incomplete.warning_not_allowlisted` | An unlisted warning; decide whether it belongs in the profile's `benignWarnings` | |
There was a problem hiding this comment.
When settlement.incomplete.warning_not_allowlisted occurs, this remedy invites operators to add the warning to benignWarnings, contradicting the definitive policy in docs/qa/keiko-for-quality.md:365-369 that this profile intentionally has no benign-warning allowlist because it previously allowed a clean verdict over partially inspected code. Following the runbook can therefore convert an incomplete review into a false-clean result; direct operators to repair the engine-side warning cause instead.
AGENTS.md reference: AGENTS.md:L315-L317
Useful? React with 👍 / 👎.
|
|
||
| | Reason | What to do | | ||
| | -------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `settlement.incomplete.budget_exceeded` | Split the change, or raise `token_budget` knowingly | |
There was a problem hiding this comment.
For a run that reports settlement.incomplete.budget_exceeded under this commit, raising token_budget cannot resolve the failure: the workflow already sets it to 6,000,000, while .github/workflows/keiko-for-quality.yml:426-433 documents that the action clamps the allotment to 6M before applying this input. An operator following this alternative can therefore retry the same oversized change, spend another full 6M tokens, and still receive an incomplete settlement; at the current ceiling the runbook should direct them to split the change or adopt the partial-store support.
Useful? React with 👍 / 👎.
1 participant
Operator documentation for what a model-backed review actually costs, written from real billing lines after a day of development consumed roughly a month's model budget.
The prices are derived, not estimated: €2.17/1M input, €0.22/1M cached input, €13.00/1M output, and about 25k tokens per reviewed file measured across three thousand file reviews.
The useful part is not the prices. Three multipliers made that day expensive, and none of them was the price per token:
cache.store_loadedzero entries,cache.hitszero) and nobody was reading them. One 135-file pull request was reviewed twenty-one times in a day.So the document leads with the ten-second check for whether memoization is working, because its failure mode is invisible:
entries: 0on a pull request that has been pushed to before looks exactly like a healthy first run.The workflow's own budget note is updated in the same change — the #75 fix has merged upstream, so the caveat now states what still applies until the pin advances, rather than describing the fix as pending.
Note on this pull request: it doubles as the live proof that memoization works end to end. The reviewer has never once recorded a cache hit in production — the store was inert before #2931 and has not been exercised since — so the cost argument above rests on an unproven link. The second push here changes only
docs/qa/review-cost-model.md, which the profile excludes from review, leaving the workflow file byte-identical. If the store works, the second run must report a hit. Cost of the proof: a few cents.🤖 Generated with Claude Code