Repository navigation
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the prior review is a documentation-only README note, and the documented recovery behavior matches the implementation ( Files Reviewed (1 file)
Previous Review Summaries (4 snapshots, latest commit 862d23c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 862d23c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change resolves both prior findings: the recovered container output is now stamped with the paid call's ledgered Overview
Files Reviewed (10 files)
Previous review (commit ce7a817)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe new re-measure path resolves the earlier engine-identity concern by re-measuring argv-affected cases without a version bump; two narrower issues remain — a recovered container output is stamped with the current transport revision, and the dry-run planner re-materializes each long-context snapshot once per result row. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (16 files)
Note: the previously reported engine-identity WARNING is not carried forward — the new Previous review (commit 4e029d1)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit only retitles a test, clarifies a doc comment, and adds a container test proving prompts reach Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit 4a594ab)Status: 1 Issue Found | Recommendation: Address before merge Overview
The change correctly fixes the missing workspace snapshot for long-context decider cases and routes the prompt through stdin to avoid Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Note: the container smoke test used a Reviewed by deepseek-v4.1-flash · Input: 36.2K · Output: 3.7K · Cached: 222.7K Review guidance: REVIEW.md from base branch |
…ses without an engine bump
…, price snapshots once, and reserve drain space for re-measures
… in the re-measure runbook
| const [failed] = await orm.batch([ | ||
| failOrphanedRunningProfilesStatement(orm, failureReason, olderThanIso, nowIso), | ||
| orm | ||
| .delete(profileRemeasures) |
There was a problem hiding this comment.
bot: A failed settlement can discard the queued remeasurement request
Suggested fix: When stale claims belong to a completed run whose profile still has old-revision results, release the remeasurement row back to the unclaimed queue (or preserve an operator-visible failed state) instead of deleting it. Alternatively, make run completion conditional on successful settlement and provide a retry path. Add a test where settlement fails, the run completes, and orphan cleanup runs; verify the repair remains discoverable and retryable.
|
bot: Closing as superseded. The head of this branch is an ancestor of the #7793 head that merged to main, so every change here is already on main. |
Summary
The production decider runner never materialized long-context cases, so they ran without their workspace snapshot. The offline sweep and candidate screen already materialize them. The runner also passed every prompt as a
kilo runargument, which the CLI rewrites.deciderCliPrompt()incli-runner.tsis the one function that builds a case's prompt; it materializes long-context cases withmaterializeLongContext.container/server.mjswrites the prompt to the CLI's stdin and no longer passes it as an argument:MAX_ARG_STRLEN,E2BIG).buildRunMessage(packages/opencode/src/kilocode/cli/cmd/run-message.ts:8-11) wraps an argument containing a space in double quotes and backslash-escapes inner quotes, so every case so far reached the model in that rewritten form. The CLI is unchanged; benchmark prompts stop using argv.kilo runreads stdin to EOF and sends it verbatim (loadInput→readPipedStdin({ bound: false })→resolveRunInput("", piped)inpackages/opencode/src/cli/cmd/run.ts, present in v7.8.12, the CLI the image installs). The npmkilowrapper spawns the binary withstdio: "inherit", so stdin passes through.BENCHMARK_ENGINE_VERSIONis unchanged, by decision. Instead, a targeted re-measure replaces only the results the old transport damaged:case_results,pending_decider_outputsandscheduler_executionsrow recordsprompt_revision(old rows: 1, new: 2,PROMPT_TRANSPORT_REVISIONinsrc/prompt-transport.ts). A container result recovered after its caller died is stamped with the revision the paid call was opened under, so output a previous deploy produced is re-measured instead of trusted.argvAffectedCases(router)is the affected set: every long-context case, plus efficient cases whose prompt has an inner"and whose check isexact,contains_allorregex. Frontier inner-quote cases join behindREMEASURE_FRONTIER_INNER_QUOTE_CASES(off).POST /admin/registry/remeasure-prompt-transport({router?, action: dry-run | request | cancel}): the dry run writes nothing and reports affected pairs, results and projected spend (measured cost of each affected result, plus snapshot prompt tokens at the catalog prompt price);requestqueues the pairs in a newprofile_remeasurestable;cancelremoves unclaimed rows.readyon its previous run, so the platform table, owner tables and status keep serving. The run copies the previous run's results except the affected old-revision rows, so only those cases are scheduled under the existing caps. On completion the profile'srun_idmoves to the new run and the table republishes; a failed run, dead lane or orphaned claim keeps the previous measurement.Rollout
The re-measure is an operator step after the whole stack is merged and deployed (#7784, #7793, #7790, #7791, #7809, #7808, #7817); the migration applies on deploy. Run the dry run first and queue only after reviewing the projected spend.
Verification
oxfmtclean.server.mjswith akilostand-in received a 655,398-byte prompt on stdin with identical byte length and SHA-256; spawning the same prompt as an argument fails withE2BIG.Risks