Repository navigation
fix: report prompt size when synthesis fails - #243
Conversation
[Low impact] The PR appears safe to merge. Reviews (1) · Last reviewed commit: "no-mistakes(document): Document synthesi..." · Reviewed by Greptile |
|
Speaking as Kun's firstmate. I reviewed the diff (no workflow, network, or exec changes) and approved fork CI. Everything is green, including no-mistakes, and the PR is CLEAN. It closes #76: a failed synthesis prompt (edit or annotate) now reports the measured UTF-8 byte size of the prompt file it sent, without printing the prompt and without claiming size was the cause, and pinned-agent failures keep that line instead of cutting the message to its first line. I checked that every VISION per-rule:
Contract-class: restore. The failure path is already promised to name its condition; this adds the measured size to an existing failure message and leaves the successful path and the agent ladder unchanged. Merging. |
|
Speaking as Kun's firstmate: this is merged. Thank you @WTF-Am-ID — really appreciate you taking the time on this. |
Intent
1 p1-p7 codex only no cross model review
Context: this task is upstream scan 3 pick P6: upstream issue #76 in kunchenguid/backpass. The owner approved fixing scan picks P1-P7 upstream through PRs, written and reviewed by Codex only, with no cross-model review. Scan notes for this pick:
Issue: #76
A failed synthesis call does not report the assembled prompt's measured size, hiding an oversized-input diagnosis.
Size S.
Contract class: restore.
Recorded triage.
Current-source evidence: Pinned Backpass
src/synthesize.js:647-648writes the assembled edit prompt, and:733-744rethrows prompt failure after closing the session. The inspected path adds no measured byte/token-size diagnostic.Competing-work check: No exact target among all 18 proposal bodies/titles. File inventories and the synthesize/acpx diffs were checked for the model-catalog and ambiguous-anchor proposals. They resolve model families or reject individual edits, not failure-time prompt-size reporting. The analysis failure-trace proposal concerns a different phase.
Closure test: With a fake ACP adapter force synthesis failure for both a small prompt and an oversized synthetic prompt. The error must name the synthesis phase, measured UTF-8 bytes and a clearly labelled token estimate if supplied, without printing prompt contents. Check a non-ASCII prompt so bytes and characters are not confused. Keep the original error class/cause and success behavior. Do not infer that size caused every failure or introduce an unrequested hard ceiling.
What Changed
Review was single-family (Codex) by the owner's temporary choice.
Risk Assessment
✅ Low: The bounded change preserves measured prompt bytes through failure reporting without changing successful execution or adding prompt disclosure or size limits.
Testing
Focused tests passed, and the regression failed against baseline on the missing diagnostic. Live Codex/acpx fault probes verified byte counts, pinned-error propagation, annotation sizing and unchanged empty-proposal behavior. CLI transcripts were retained; no provider inference, full suite, lint or static analysis ran. Disposable data was removed.
Evidence: Verification report and live CLI transcripts
Evidence: Oversized pinned synthesis failure
Evidence: Annotation failure diagnostic
Evidence: Baseline regression reproduction
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
src/synthesize.js:715- Pinned synthesis failures still hide the size diagnostic. With--synthesis-agent codex, an edit prompt exiting 1 reaches the new\nSynthesis prompt size: ...suffix here through src/synthesize.js:742. AgentResolver.withFallthrough then wraps it at src/agents.js:525, and pinnedFailureError discards everything after the first newline at src/agents.js:570. CLI output therefore omits bytes for both small and oversized prompts, contradicting the required criterion: "The error must name the synthesis phase, measured UTF-8 bytes". The replacement resolver in test/synthesize.test.js:229 bypasses this production behavior. Preserve the diagnostic across the resolver's error boundary and cover the real pinned path.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
node --import ./test/helpers/hermetic-config.js --test test/synthesize.test.js test/synthesis-cli.test.jsnode .validation-76/probe.mjswith isolated Codex/acpx configuration and dummy authenticationnode .validation-76/drive.mjsfor real CLI process failures, timeouts, annotation failure and empty-proposal behaviornode .validation-76/drive-baseline.mjsand comparison of baseline/candidate proposal fieldsnode --import ./test/helpers/hermetic-config.js --test --test-name-pattern="failed synthesis reports" test/synthesize.test.jsagainst disposable baseline sourceClosed disposable acpx sessions, removed.validation-76, and confirmed a clean worktree at the target revision✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.