From 4435aef4a1a28102e09b1ec6fcd27d11da2a9b77 Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Thu, 23 Jul 2026 00:55:40 +0800 Subject: [PATCH 1/4] fix: guide models to correct quality-gate rejection recovery path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the blocking pre-commit quality gate rejects a compress call, the old guidance told the model to 'rewrite a more complete summary' — impossible for large ranges (a 243K-token range needs >=9725 chars to pass the 1% retention floor). In dog/opencode-acp#33 this deadlocked glm-5.2 for 23 rejections, re-billing ~320K tokens each time. Three changes make the correct recovery discoverable: - acknowledgeRisk now has a .describe() (it was the only param without one), so the escape hatch is visible in the tool schema. - buildQualityRejectionError branches on range size: ranges >50K tokens get SPLIT guidance (break into smaller ranges), small ranges get denser-summary guidance; both mention summaryMaxChars + acknowledgeRisk. - system prompt gains a COMPRESSION REJECTION HANDLING section listing the three recovery paths in priority order + an anti-loop rule. Gate logic, state schema, and exported signatures unchanged. Refs dog/opencode-acp#33 --- .../DESIGN.md | 85 +++++++++++++++++++ .../REQ.md | 74 ++++++++++++++++ .../WORKLOG.md | 66 ++++++++++++++ lib/compress/message.ts | 7 +- lib/compress/quality-gate/rejection.ts | 50 +++++++++-- lib/compress/range.ts | 7 +- lib/prompts/system.ts | 10 +++ tests/quality-gate-enforcement.test.ts | 57 +++++++++++++ 8 files changed, 348 insertions(+), 8 deletions(-) create mode 100644 devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md create mode 100644 devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md create mode 100644 devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md b/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md new file mode 100644 index 00000000..50c471bb --- /dev/null +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md @@ -0,0 +1,85 @@ +# DESIGN - Quality-gate rejection recovery guidance + +- Task ID: `2026-07-23_quality-gate-rejection-prompt` + +## 1. Goal + +Make the correct compression-rejection recovery path discoverable by the model, +so a non-adaptive model (e.g. glm-5.2) does not deadlock when the blocking +pre-commit quality gate rejects an oversized-range compression. + +## 2. Data flow (unchanged) + +``` +compress(args) → validateArgs → prepareSession → resolveRanges + → evaluatePreCommitQuality ← BLOCKING gate (qualityGate.enabled) + passed → applyCompressionState → finalizeSession → commit + FAILED → buildQualityRejectionError → THROW + state.qualityGateRetryPending = true + (model sees error text, must retry) +``` + +This PR does NOT change the flow. It only changes the **content** the model sees +at three points: the tool schema, the thrown error text, and the system prompt. + +## 3. Changes + +### 3.1 Tool schema — discoverable escape hatch + +`lib/compress/range.ts` + `lib/compress/message.ts`: add `.describe(...)` to +`acknowledgeRisk`. The describe states (a) it overrides a rejection only, never +preemptive, (b) two better fixes to try first (split range / summaryMaxChars). + +### 3.2 Rejection message — recovery path sized to the failure + +`lib/compress/quality-gate/rejection.ts` `buildQualityRejectionError`: +introduce a "large range" branch using `originalTokens` (already computed in +`computeStats`). + +- **Large range** (`originalTokens > 50000` OR `ratio > 50:1`): lead with + **SPLIT** guidance — "break into 2-3 smaller contiguous ranges, compress each + in the same batch call". A single dense summary is impractical at this size. +- **Small range**: keep the existing "write a denser summary" lead. +- **Both paths** additionally mention `summaryMaxChars` (raise the cap) and + `acknowledgeRisk: true` (last resort). Keep the existing metrics block + (Range / Original / Summary / Ratio / Retention / Gate layer / rougeF1 / + top20Recall) and the appended `HOW_TO_COMPRESS_RULES`. + +Threshold rationale: L1 floor is `retentionPct = summaryChars / +(originalTokens*4) >= 1%`, i.e. `summaryChars >= originalTokens*0.04`. At +50K tokens the dense minimum is already 2000 chars and grows linearly; by +~250K it exceeds 9000 chars — impractical for one summary. 50K is a sensible +"crossover" where splitting becomes the better advice. + +### 3.3 System prompt — set expectations up front + +`lib/prompts/system.ts`: add a "COMPRESSION REJECTION HANDLING" section (placed +after the tools / philosophy block, before PERIODIC CONTEXT STATUS). It states: +(1) compress can be rejected when qualityGate is on, (2) the three recovery +paths in priority order (split → denser/longer → acknowledgeRisk), (3) an +explicit anti-loop rule: after two rejections of the same range, change +strategy rather than resubmit an identical summary. + +## 4. Backward compatibility + +- No persisted-state field added/removed. +- No exported signature change. `buildQualityRejectionError(plan, result)` + keeps the same signature; only its output text changes. +- No internal `dcp` tag touched. +- The blocking gate's pass/fail logic is untouched — only guidance text. + +## 5. Testing + +- Existing `tests/quality-gate-enforcement.test.ts` cases (1000-token small + ranges) stay green: header, range, "1000 tokens", `acknowledgeRisk`, + `HOW TO COMPRESS` all still present. +- New case: large range (>50K tokens) → message includes "split" advice. +- New case: small range → message keeps "denser" advice (regression guard). + +## 6. Out of scope (follow-ups) + +- Anti-deadlock counter: after N consecutive rejections of the same range, + auto-downgrade to non-blocking. Requires a `qualityGateConsecutiveRejects` + counter in `SessionState` → persistence migration. Separate PR. +- Making `qualityGate.enabled`'s blocking-vs-non-blocking behavior match the + README (which only documents the non-blocking post-commit gate). Doc PR. diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md b/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md new file mode 100644 index 00000000..9442f7e5 --- /dev/null +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md @@ -0,0 +1,74 @@ +# REQ - Quality-gate rejection: tell the model the correct recovery path + +- Task ID: `2026-07-23_quality-gate-rejection-prompt` +- Home Repo: `opencode-acp` +- Created: 2026-07-23 +- Status: InProgress +- Priority: P1 +- Owner: awork +- References: dog/opencode-acp#33 + +## 1. Background & Problem Statement + +- **Context**: `qualityGate.enabled: true` activates a **blocking pre-commit** + quality gate (`evaluatePreCommitQuality` in `lib/compress/quality-gate/evaluate.ts`, + wired in `lib/compress/range.ts`/`message.ts`). A summary that fails the gate + throws `buildQualityRejectionError` and the compression does not commit. +- **Current behavior (symptom)**: In session `ses_07fa6ea18ffe...` (issue #33), + glm-5.2 tried to compress a ~243K-token range into an 8543-char summary + (0.88% retention, L1 floor is 1%). The gate rejected it 23 times. Each + rejection re-billed ~320K tokens and injected ~15KB of error text. The model + never used the `acknowledgeRisk` escape hatch (0/23) and never split the range. + Result: context stuck at ~320K, ~millions of tokens billed for zero progress. +- **Expected behavior**: After a rejection, the model must be able to discover a + *correct* recovery path. The three valid paths are: (1) split an oversized + range into smaller ranges, (2) write a denser / longer summary (raise + `summaryMaxChars`), (3) `acknowledgeRisk: true` as last resort. +- **Impact**: Deadlock for non-adaptive models → token blowup + unusable + compression when `qualityGate.enabled` is on. + +## 2. Root Cause (prompt-chain audit) + +Three gaps make the correct recovery path undiscoverable: + +1. **Tool schema** (`lib/compress/range.ts:91`, `lib/compress/message.ts:66`): + `acknowledgeRisk: tool.schema.boolean().optional()` has **no `.describe()`** + (sibling fields `dangerous` and `summaryMaxChars` do). The model cannot learn + what the parameter does from the tool definition. +2. **Rejection message** (`lib/compress/quality-gate/rejection.ts:42-75`): tells + the reason (metrics) + mentions `acknowledgeRisk`, but the only recovery hint + is "rewrite a more complete summary" — wrong/impossible for a 243K-token + range (would need ≥9725 chars). Never advises **splitting** the range, never + mentions `summaryMaxChars`. +3. **System prompt** (`lib/prompts/system.ts`): completely silent on the fact + that compression can be rejected and how to recover. The model has no prior + expectation of rejection. + +## 3. Constraints & Non-Goals + +- **Constraints**: + - No `as any` / `@ts-ignore` (AGENTS.md). + - Backward compatible: no persisted-state format change, no internal `dcp` + tag rename (AGENTS.md §2.6). + - L1 gate contract unchanged — we only improve *guidance text*, not the gate + thresholds. + - Follow existing code style (double quotes, no semicolons, 4-space indent). +- **Non-Goals (this PR)**: + - Anti-deadlock auto-downgrade after N consecutive rejections (needs a state + counter → schema change). Tracked as a follow-up. + - Changing gate thresholds or the blocking-vs-non-blocking design. + - Touching the external `context-compress-algorithms` package. + +## 4. Acceptance Criteria + +- [ ] `acknowledgeRisk` has a `.describe()` in both `range.ts` and `message.ts`. +- [ ] `buildQualityRejectionError` gives **split-range** guidance when the range + is large (>50K tokens or ratio >50:1), and keeps denser-summary guidance + for small ranges. Both paths mention `summaryMaxChars` + `acknowledgeRisk`. +- [ ] `system.ts` has a "COMPRESSION REJECTION HANDLING" section listing the + three recovery paths in priority order, and an explicit "do not loop" + rule. +- [ ] New test asserts large-range guidance mentions splitting. +- [ ] Existing tests in `tests/quality-gate-enforcement.test.ts` still pass. +- [ ] `npm run typecheck`, `npm run format:check`, `npm test`, `npm run build` + all pass. diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md b/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md new file mode 100644 index 00000000..76e847fd --- /dev/null +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md @@ -0,0 +1,66 @@ +# WORKLOG - Quality-gate rejection recovery guidance + +- Task ID: `2026-07-23_quality-gate-rejection-prompt` +- Home Repo: `opencode-acp` +- Status: Done +- Updated: 2026-07-23 00:55 + +## 1. Summary + +- **What was done**: Made the quality-gate rejection recovery path discoverable + by the model — added a `.describe()` to `acknowledgeRisk`, branched the + rejection message to advise SPLITTING oversized ranges (vs the old "rewrite + longer" hint that is impossible for 243K-token ranges), and added a + "COMPRESSION REJECTION HANDLING" section to the system prompt. +- **Why**: In dog/opencode-acp#33, glm-5.2 deadlocked — it tried to compress a + 243K-token range into an 8543-char summary, the blocking pre-commit gate + rejected it 23×, and the model never discovered the correct recovery (split + / summaryMaxChars / acknowledgeRisk). Each failure re-billed ~320K tokens. +- **Behavior / compatibility changes**: No. The gate's pass/fail logic, state + schema, persisted format, and exported signatures are unchanged. Only guidance + text (tool schema description, error message body, system prompt) changed. +- **Risk level**: Low. + +## 2. Change Log + +### Commits + +| Commit | Description | +|--------|-------------| +| (this PR) | fix: guide models to the correct quality-gate rejection recovery path | + +### Files + +| File | Change | +|------|--------| +| `lib/compress/range.ts` | `acknowledgeRisk` gains a `.describe()` (was the only param without one) | +| `lib/compress/message.ts` | same `acknowledgeRisk` `.describe()` for message mode | +| `lib/compress/quality-gate/rejection.ts` | `buildQualityRejectionError` now branches: ranges >50K tokens get SPLIT guidance, smaller ranges get denser-summary guidance; both mention `summaryMaxChars` + `acknowledgeRisk` | +| `lib/prompts/system.ts` | new "COMPRESSION REJECTION HANDLING" section (3 recovery paths in priority order + anti-loop rule) | +| `tests/quality-gate-enforcement.test.ts` | +2 tests: large-range → split guidance; small range → denser-summary guidance | + +## 3. Verification + +- `npm run typecheck` — clean. +- `npm test` — 837 pass, 0 fail (includes the 2 new tests). +- `npm run build` — success; `dist/index.js` contains "SPLIT THE RANGE", + "COMPRESSION REJECTION HANDLING", and the new `acknowledgeRisk` describe. +- Existing rejection tests (`buildQualityRejectionError includes range, stats, + and acknowledgeRisk instructions`; ratio/retention computation) still pass. + +## 4. Pre-existing issues noticed (NOT addressed here) + +- `npm run format:check` flags 246 files on clean `github/master` — prettier + config drift, predates this PR, not enforced by CI (`ci.yml` runs only + typecheck + test). Reported separately; this PR keeps its diff to intended + changes only. +- README/AGENTS.md document the quality gate as "non-blocking (logger.warn + only)" but `qualityGate.enabled` also activates a blocking pre-commit gate. + Doc mismatch — follow-up. + +## 5. Follow-ups + +- Anti-deadlock counter (auto-downgrade to non-blocking after N consecutive + rejections of the same range) — needs a state counter → persistence migration. + Separate PR. +- README doc fix for the blocking-vs-non-blocking gate description. diff --git a/lib/compress/message.ts b/lib/compress/message.ts index ccd40fda..1862debd 100644 --- a/lib/compress/message.ts +++ b/lib/compress/message.ts @@ -63,7 +63,12 @@ function buildSchema(maxSummaryLengthHard: number) { .describe( "Set to true ONLY when you are certain the most recent message(s) must be compressed. Required when a range includes the tail of the conversation.", ), - acknowledgeRisk: tool.schema.boolean().optional(), + acknowledgeRisk: tool.schema + .boolean() + .optional() + .describe( + 'Set to true ONLY to override a quality-gate rejection, after you have genuinely tried to fix the summary. Never set it preemptively — it errors if no rejection is pending. Two better fixes to try FIRST: (1) split an oversized range into 2-3 smaller contiguous ranges and compress each separately, or (2) pass "summaryMaxChars" to allow a longer summary.', + ), } } diff --git a/lib/compress/quality-gate/rejection.ts b/lib/compress/quality-gate/rejection.ts index 708d2c9d..6ff7c4e9 100644 --- a/lib/compress/quality-gate/rejection.ts +++ b/lib/compress/quality-gate/rejection.ts @@ -26,6 +26,7 @@ function computeStats(plan: RejectionPlanInfo): { originalTokens: number summaryChars: number ratio: string + ratioNum: number | null retentionPct: string } { let originalTokens = 0 @@ -33,10 +34,49 @@ function computeStats(plan: RejectionPlanInfo): { originalTokens += plan.messageTokenById.get(id) || 0 } const summaryChars = plan.summary.length - const ratio = originalTokens > 0 ? (originalTokens / Math.max(summaryChars / 4, 1)).toFixed(1) : "?" + const ratioNum = + originalTokens > 0 ? originalTokens / Math.max(summaryChars / 4, 1) : null + const ratio = ratioNum !== null ? ratioNum.toFixed(1) : "?" const retentionPct = originalTokens > 0 ? ((summaryChars / (originalTokens * 4)) * 100).toFixed(2) : "?" - return { originalTokens, summaryChars, ratio, retentionPct } + return { originalTokens, summaryChars, ratio, ratioNum, retentionPct } +} + +// A range this large cannot be summarized densely enough in one pass: the L1 +// retention floor (1% of originalTokens*4 chars) would demand a multi-thousand +// char summary. At this size, splitting is the correct recovery, not rewriting. +const LARGE_RANGE_TOKENS = 50_000 + +function buildRecoveryGuidance(stats: ReturnType): string { + const isLargeRange = stats.originalTokens > LARGE_RANGE_TOKENS + + const escapeHatches = `If the hard length limit is too tight for important detail, pass "summaryMaxChars" (e.g. 12000) to raise the cap. +As a LAST RESORT only — after you have genuinely tried the above — add "acknowledgeRisk": true to accept the information loss and force the compression through. Without acknowledgeRisk: true on the retry, the compression will be rejected again.` + + if (isLargeRange) { + return `HOW TO RECOVER — SPLIT THE RANGE + +This range is very large (~${stats.originalTokens} tokens). A single summary almost cannot be dense +enough to pass the retention floor — the failure is structural, not a wording problem. +Do NOT resubmit the same range with a slightly longer summary. Instead: + +1. SPLIT this range into 2-3 smaller contiguous ranges (e.g. first half, second half) and + compress each one separately in the SAME batch "compress" call — give each its own "topic" + and "summary". Smaller ranges are far easier to summarize densely. +2. Only if a sub-range is still rejected, rewrite THAT smaller summary to be denser (full file + paths, signatures, exact errors, decisions + rationale). + +${escapeHatches}` + } + + return `HOW TO RECOVER — WRITE A DENSER SUMMARY + +The range is small enough that one summary can pass. Rewrite it to preserve every load-bearing +detail: full file paths with line numbers, function/type signatures, exact error strings, +decisions WITH their rationale, exact values. Strip only true noise (verbose logs, duplicate +reads, spent exploration). + +${escapeHatches}` } export function buildQualityRejectionError( @@ -65,11 +105,9 @@ Your summary becomes the SOLE record. If it fails, subsequent work is built on a memory loss → wrong assumptions → entire reasoning chain collapse. Treat every compression with maximum care. -${HOW_TO_COMPRESS_RULES} +${buildRecoveryGuidance(stats)} -To retry: rewrite a more complete summary that preserves critical details (file paths, decisions, -exact values, errors). Then add "acknowledgeRisk": true to the compress tool call parameters. -Without acknowledgeRisk: true, the compression will be rejected again.` +${HOW_TO_COMPRESS_RULES}` return new Error(message) } diff --git a/lib/compress/range.ts b/lib/compress/range.ts index b27722f8..e2eee9fc 100644 --- a/lib/compress/range.ts +++ b/lib/compress/range.ts @@ -88,7 +88,12 @@ function buildSchema(maxSummaryLengthHard: number) { .describe( "Set to true ONLY when you are certain the most recent message(s) must be compressed. Required when a range includes the tail of the conversation.", ), - acknowledgeRisk: tool.schema.boolean().optional(), + acknowledgeRisk: tool.schema + .boolean() + .optional() + .describe( + 'Set to true ONLY to override a quality-gate rejection, after you have genuinely tried to fix the summary. Never set it preemptively — it errors if no rejection is pending. Two better fixes to try FIRST: (1) split an oversized range into 2-3 smaller contiguous ranges and compress each separately, or (2) pass "summaryMaxChars" to allow a longer summary.', + ), } } diff --git a/lib/prompts/system.ts b/lib/prompts/system.ts index 54cc0dc9..60105b7b 100644 --- a/lib/prompts/system.ts +++ b/lib/prompts/system.ts @@ -58,6 +58,16 @@ WHEN NOT TO COMPRESS ${HOW_TO_COMPRESS_RULES} +COMPRESSION REJECTION HANDLING + +When \`qualityGate.enabled\` is on, a \`compress\` call can be REJECTED by a quality gate if your summary would lose too much information. The error reports original tokens, summary chars, retention %, ratio, and which gate layer failed. A rejection commits nothing — the original messages stay intact. Recover in this priority order: + +1. SPLIT the range. If the range is large (>50K tokens), one summary can rarely be dense enough to pass. Break it into 2-3 smaller contiguous ranges and compress each separately in the SAME batch \`compress\` call (each entry gets its own \`topic\` and \`summary\`). +2. WRITE a denser / longer summary. For a small range, keep every load-bearing detail (full file paths with line numbers, signatures, exact errors, decisions WITH their rationale). If the hard cap is too tight, pass \`summaryMaxChars\` (e.g. 12000) to allow a longer summary. +3. ACKNOWLEDGE RISK as a last resort. Only after (1) and (2) genuinely failed, add \`"acknowledgeRisk": true\` to accept information loss. This only works immediately after a rejection — it errors if set preemptively. + +Do NOT loop: if the same range is rejected twice, you MUST change strategy (split it). Never resubmit an identical or near-identical summary — it will be rejected again and waste context. + PERIODIC CONTEXT STATUS Periodically, as context grows, the system appends a short status line in a synthetic suffix message. It looks like: diff --git a/tests/quality-gate-enforcement.test.ts b/tests/quality-gate-enforcement.test.ts index 25b99c04..c684f60a 100644 --- a/tests/quality-gate-enforcement.test.ts +++ b/tests/quality-gate-enforcement.test.ts @@ -265,6 +265,63 @@ test("buildQualityRejectionError computes ratio and retention from plan data", ( assert.ok(error.message.includes("100 chars"), "should show summary chars") }) +test("buildQualityRejectionError advises SPLITTING for a large range (>50K tokens)", () => { + const messageIds = ["msg-1", "msg-2"] + const plan = { + startId: "m00472", + endId: "m00793", + summary: "x".repeat(8543), + messageIds, + messageTokenById: buildTokenMap(messageIds, 30000), + } + const result: QualityGateResult = { + passed: false, + layer: "L1-length", + reason: "retention too low", + metrics: [], + } + + const error = buildQualityRejectionError(plan, result) + const msg = error.message + + assert.ok(msg.includes("SPLIT THE RANGE"), "large range should lead with split guidance") + assert.ok( + msg.includes("smaller contiguous ranges"), + "should tell the model to split into smaller ranges", + ) + assert.ok(msg.includes("60000 tokens"), "should report the large token count") + assert.ok(msg.includes("summaryMaxChars"), "should mention the summaryMaxChars escape hatch") + assert.ok(msg.includes("acknowledgeRisk"), "should mention acknowledgeRisk as last resort") +}) + +test("buildQualityRejectionError advises a denser summary for a small range", () => { + const messageIds = ["msg-1"] + const plan = { + startId: "m00001", + endId: "m00002", + summary: "too short", + messageIds, + messageTokenById: buildTokenMap(messageIds, 1000), + } + const result: QualityGateResult = { + passed: false, + layer: "L1-length", + reason: "retention too low", + metrics: [], + } + + const error = buildQualityRejectionError(plan, result) + const msg = error.message + + assert.ok( + msg.includes("DENSER SUMMARY"), + "small range should lead with denser-summary guidance", + ) + assert.ok(!msg.includes("SPLIT THE RANGE"), "small range should not advise splitting") + assert.ok(msg.includes("summaryMaxChars"), "should mention the summaryMaxChars escape hatch") + assert.ok(msg.includes("acknowledgeRisk"), "should mention acknowledgeRisk as last resort") +}) + test("buildPreemptiveAcknowledgeError explains the parameter is invalid without pending rejection", () => { const error = buildPreemptiveAcknowledgeError() assert.ok(error.message.includes("acknowledgeRisk"), "should mention the parameter name") From c3d0abdedc3d2baae01d7739e914446c02d75e14 Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Thu, 23 Jul 2026 01:15:50 +0800 Subject: [PATCH 2/4] fix: allow preemptive acknowledgeRisk so the model isn't forced to fail first The 'ACK only after a rejection' guard (buildPreemptiveAcknowledgeError) backfired: analysis of the session in dog/opencode-acp#33 shows the model doing a 'write-short -> rejected -> retry-with-ACK' dance 7x in a row. The guard intended to force good summaries instead forced a deliberate fake-fail, and over-blocking pushed the model toward avoiding compression entirely -- the actual root of the 330K-token blowup. Remove the guard so acknowledgeRisk bypasses the gate on the FIRST attempt. The gate still blocks non-ACK calls (protection by default + explicit opt-out), so the model is never trapped into avoidance. Post-commit evaluation still runs and warns, so bypassed compressions stay observable. - Remove preemptive-ACK guard in range.ts and message.ts - Remove dead buildPreemptiveAcknowledgeError (fn, export, imports) - Update acknowledgeRisk .describe() + system-prompt ACK bullet: usable on the first attempt; drop the false 'errors if set preemptively' - Flip the preemptive-ACK integration test to assert success Refs dog/opencode-acp#33 --- .../WORKLOG.md | 28 +++++++++++-- lib/compress/message.ts | 6 +-- lib/compress/quality-gate/index.ts | 2 +- lib/compress/quality-gate/rejection.ts | 8 ---- lib/compress/range.ts | 6 +-- lib/prompts/system.ts | 2 +- tests/quality-gate-enforcement.test.ts | 39 +++++++------------ 7 files changed, 42 insertions(+), 49 deletions(-) diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md b/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md index 76e847fd..ed6448e2 100644 --- a/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md @@ -60,7 +60,29 @@ ## 5. Follow-ups -- Anti-deadlock counter (auto-downgrade to non-blocking after N consecutive - rejections of the same range) — needs a state counter → persistence migration. - Separate PR. - README doc fix for the blocking-vs-non-blocking gate description. + +## 6. Round 2 — make `acknowledgeRisk` usable anytime (per issue #33 feedback) + +- **Trigger**: user analyzed the pre-restart session (dog/opencode-acp#33) and + found the model doing a "write-short → rejected → retry-with-ACK" dance 7× in + a row. User's reframe: the "ACK only after a rejection" rule MANUFACTURES the + deliberate-fail pattern, and over-blocking causes the model to avoid + compressing entirely (the actual root of the 330K problem) — worse than + occasional bad compressions. +- **Change**: removed the preemptive-`acknowledgeRisk` guard so ACK bypasses + the gate on the FIRST attempt, no prior rejection needed. The gate still + BLOCKS non-ACK calls (protection by default + explicit opt-out → the model is + never trapped into avoiding compression). Post-commit `evaluateBatchQuality` + still runs + warns → observability retained. +- **Removed**: `buildPreemptiveAcknowledgeError` (now dead code), its export, + and its test. The integration test "preemptive acknowledgeRisk ... is + rejected" was FLIPPED to assert it now succeeds. +- **Wording**: `acknowledgeRisk` `.describe()` and the system-prompt ACK bullet + no longer say "never preemptive"; they say "usable on the first attempt". +- **Verification**: `npm run typecheck` clean; `npm test` 836 pass / 0 fail; + `npm run build` ok; bundle verified (guard text gone, new describe present). +- **Dropped earlier idea**: "constrain ACK / refuse short-summary ACK / + rate-limit" — would worsen over-blocking; explicitly rejected by user. +- **Still out of scope**: making the gate fully advisory (warn-only). This PR + keeps it blocking for non-ACK calls; free-ACK is the middle ground. diff --git a/lib/compress/message.ts b/lib/compress/message.ts index 1862debd..5b10764b 100644 --- a/lib/compress/message.ts +++ b/lib/compress/message.ts @@ -22,7 +22,6 @@ import { import { resolveKeepMarkers } from "./keep-markers" import type { CompressMessageToolArgs } from "./types" import { - buildPreemptiveAcknowledgeError, buildQualityRejectionError, evaluatePreCommitQuality, } from "./quality-gate" @@ -67,7 +66,7 @@ function buildSchema(maxSummaryLengthHard: number) { .boolean() .optional() .describe( - 'Set to true ONLY to override a quality-gate rejection, after you have genuinely tried to fix the summary. Never set it preemptively — it errors if no rejection is pending. Two better fixes to try FIRST: (1) split an oversized range into 2-3 smaller contiguous ranges and compress each separately, or (2) pass "summaryMaxChars" to allow a longer summary.', + 'Set to true to bypass the quality gate when you judge the summary acceptable despite some information loss (e.g. the range is low-value, or you cannot make it denser). Usable on the first attempt. For content that matters, prefer writing a dense summary or splitting an oversized range instead.', ), } } @@ -197,9 +196,6 @@ export function createCompressMessageTool(ctx: ToolContext): ReturnType50K tokens), one summary can rarely be dense enough to pass. Break it into 2-3 smaller contiguous ranges and compress each separately in the SAME batch \`compress\` call (each entry gets its own \`topic\` and \`summary\`). 2. WRITE a denser / longer summary. For a small range, keep every load-bearing detail (full file paths with line numbers, signatures, exact errors, decisions WITH their rationale). If the hard cap is too tight, pass \`summaryMaxChars\` (e.g. 12000) to allow a longer summary. -3. ACKNOWLEDGE RISK as a last resort. Only after (1) and (2) genuinely failed, add \`"acknowledgeRisk": true\` to accept information loss. This only works immediately after a rejection — it errors if set preemptively. +3. ACKNOWLEDGE RISK when you judge the information loss acceptable. Set \`"acknowledgeRisk": true\` to bypass the gate — usable on the first attempt, no need to trigger a rejection first. Use it for low-value ranges or when a dense summary is not feasible; for content that matters, prefer (1) or (2). Do NOT loop: if the same range is rejected twice, you MUST change strategy (split it). Never resubmit an identical or near-identical summary — it will be rejected again and waste context. diff --git a/tests/quality-gate-enforcement.test.ts b/tests/quality-gate-enforcement.test.ts index c684f60a..4da19d22 100644 --- a/tests/quality-gate-enforcement.test.ts +++ b/tests/quality-gate-enforcement.test.ts @@ -6,7 +6,6 @@ import { mkdirSync } from "node:fs" import { evaluatePreCommitQuality, buildQualityRejectionError, - buildPreemptiveAcknowledgeError, } from "../lib/compress/quality-gate" import { createCompressRangeTool } from "../lib/compress/range" import { createSessionState, resetSessionState, type WithParts } from "../lib/state" @@ -322,16 +321,6 @@ test("buildQualityRejectionError advises a denser summary for a small range", () assert.ok(msg.includes("acknowledgeRisk"), "should mention acknowledgeRisk as last resort") }) -test("buildPreemptiveAcknowledgeError explains the parameter is invalid without pending rejection", () => { - const error = buildPreemptiveAcknowledgeError() - assert.ok(error.message.includes("acknowledgeRisk"), "should mention the parameter name") - assert.ok( - error.message.includes("no quality gate rejection is pending"), - "should explain why it's invalid", - ) - assert.ok(error.message.includes("Remove it"), "should tell model to remove it") -}) - test("qualityGateRetryPending defaults to false", () => { const state = createSessionState() assert.equal(state.qualityGateRetryPending, false) @@ -458,29 +447,27 @@ test("integration: acknowledgeRisk bypasses quality after rejection", async () = assert.equal(state.prune.messages.blocksById.size, 1, "one block should be committed") }) -test("integration: preemptive acknowledgeRisk without prior rejection is rejected", async () => { +test("integration: preemptive acknowledgeRisk bypasses the gate without a prior rejection", async () => { const sessionID = `ses-int-preempt-${Date.now()}` const rawMessages = buildIntegrationMessages(sessionID) const state = createSessionState() const config = buildConfig(true) const tool = buildToolContext(state, config, rawMessages) - await assert.rejects( - tool.execute( - { - topic: "Auth analysis", - content: [{ startId: "m00001", endId: "m00004", summary: "Good enough summary with keywords." }], - acknowledgeRisk: true, - } as any, - { ...toolCtx, sessionID }, - ), - (err: Error) => { - assert.ok(err.message.includes("no quality gate rejection is pending"), `should be preemptive error, got: ${err.message}`) - return true - }, + const result = await tool.execute( + { + topic: "Auth analysis", + content: [{ startId: "m00001", endId: "m00004", summary: "Good enough summary with keywords." }], + acknowledgeRisk: true, + } as any, + { ...toolCtx, sessionID }, + ) + assert.ok( + result.includes("Compressed"), + "preemptive acknowledgeRisk should compress without a prior rejection", ) assert.equal(state.qualityGateRetryPending, false, "flag should stay false") - assert.equal(state.prune.messages.blocksById.size, 0, "no blocks committed") + assert.equal(state.prune.messages.blocksById.size, 1, "one block should be committed") }) test("integration: flag cleared on successful non-acknowledgeRisk compression", async () => { From 9e90a594c02bdac410cc231858c78e9cb7cf1d7b Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Thu, 23 Jul 2026 10:07:10 +0800 Subject: [PATCH 3/4] docs: align DESIGN/REQ with shipped token-only large-range threshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dual-agent review (issue #33) flagged that DESIGN §3.2 / REQ §4 specified the large-range branch as 'originalTokens > 50000 OR ratio > 50:1', but the code (rejection.ts buildRecoveryGuidance) only checks originalTokens. Token-only is the correct design: a small-but-terse range (high ratio) should get denser-summary advice, not split advice. Also fix DESIGN §3.1 which still described the old 'never preemptive' ACK semantics (Round 2 removed that guard). --- .../DESIGN.md | 30 ++++++++++++------- .../REQ.md | 6 ++-- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md b/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md index 50c471bb..700ea458 100644 --- a/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/DESIGN.md @@ -27,8 +27,12 @@ at three points: the tool schema, the thrown error text, and the system prompt. ### 3.1 Tool schema — discoverable escape hatch `lib/compress/range.ts` + `lib/compress/message.ts`: add `.describe(...)` to -`acknowledgeRisk`. The describe states (a) it overrides a rejection only, never -preemptive, (b) two better fixes to try first (split range / summaryMaxChars). +`acknowledgeRisk`. The describe states it bypasses the quality gate when the +model judges the information loss acceptable, is usable on the first attempt +(no prior rejection needed), and that for content which matters a dense +summary or splitting an oversized range is preferred. (Round 2 removed the +preemptive-ACK guard — see §6 of WORKLOG — so ACK is no longer gated behind a +prior rejection.) ### 3.2 Rejection message — recovery path sized to the failure @@ -36,20 +40,26 @@ preemptive, (b) two better fixes to try first (split range / summaryMaxChars). introduce a "large range" branch using `originalTokens` (already computed in `computeStats`). -- **Large range** (`originalTokens > 50000` OR `ratio > 50:1`): lead with - **SPLIT** guidance — "break into 2-3 smaller contiguous ranges, compress each - in the same batch call". A single dense summary is impractical at this size. +- **Large range** (`originalTokens > 50000`): lead with **SPLIT** guidance — + "break into 2-3 smaller contiguous ranges, compress each in the same batch + call". A single dense summary is impractical at this size. - **Small range**: keep the existing "write a denser summary" lead. - **Both paths** additionally mention `summaryMaxChars` (raise the cap) and `acknowledgeRisk: true` (last resort). Keep the existing metrics block (Range / Original / Summary / Ratio / Retention / Gate layer / rougeF1 / top20Recall) and the appended `HOW_TO_COMPRESS_RULES`. -Threshold rationale: L1 floor is `retentionPct = summaryChars / -(originalTokens*4) >= 1%`, i.e. `summaryChars >= originalTokens*0.04`. At -50K tokens the dense minimum is already 2000 chars and grows linearly; by -~250K it exceeds 9000 chars — impractical for one summary. 50K is a sensible -"crossover" where splitting becomes the better advice. +Threshold rationale (token-only, ratio intentionally excluded): the branch is +gated on **absolute token count only** (`originalTokens > 50000`), NOT on the +summary:tokens ratio. A small range with a terse summary (e.g. 1K tokens → 10 +chars, ratio 400:1) is better served by "write a denser summary" advice, not +"split the range" — splitting an already-small range makes no sense. L1 floor is +`retentionPct = summaryChars / (originalTokens*4) >= 1%`, i.e. +`summaryChars >= originalTokens*0.04`. At 50K tokens the dense minimum is +already 2000 chars and grows linearly; by ~250K it exceeds 9000 chars — +impractical for one summary. 50K is a sensible "crossover" where splitting +becomes the better advice. (The `ratioNum` field in `computeStats` is computed +only for the displayed metrics string; it does not influence the branch.) ### 3.3 System prompt — set expectations up front diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md b/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md index 9442f7e5..2e742b20 100644 --- a/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/REQ.md @@ -63,8 +63,10 @@ Three gaps make the correct recovery path undiscoverable: - [ ] `acknowledgeRisk` has a `.describe()` in both `range.ts` and `message.ts`. - [ ] `buildQualityRejectionError` gives **split-range** guidance when the range - is large (>50K tokens or ratio >50:1), and keeps denser-summary guidance - for small ranges. Both paths mention `summaryMaxChars` + `acknowledgeRisk`. + is large (>50K tokens, absolute count — ratio intentionally excluded so a + small-but-terse range still gets denser-summary advice), and keeps + denser-summary guidance for small ranges. Both paths mention + `summaryMaxChars` + `acknowledgeRisk`. - [ ] `system.ts` has a "COMPRESSION REJECTION HANDLING" section listing the three recovery paths in priority order, and an explicit "do not loop" rule. From 756529016703ee191a03694633e32ab9f541859a Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Thu, 23 Jul 2026 23:14:14 +0800 Subject: [PATCH 4/4] fix: interpolate real config values in rejection guidance, drop dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - rejection.ts: remove dead ratioNum field from computeStats (no caller read it) - rejection.ts: buildQualityRejectionError now takes maxSummaryLengthHard param; escapeHatches interpolates the real config value instead of hardcoded 12000 - range.ts, message.ts: pass ctx.config.compress.maxSummaryLengthHard - system.ts: drop hardcoded 12000 example (static prompt can't interpolate; schema already shows the real cap) - config.ts: layer1MinRetentionPct 5.0 → 1.0 to align with schema/README/cc-alg; makes LARGE_RANGE_TOKENS=50K threshold rationale (1% floor crossover) valid - tests: update buildQualityRejectionError call sites for new signature --- .../WORKLOG.md | 33 +++++++++++++++++++ lib/compress/message.ts | 1 + lib/compress/quality-gate/rejection.ts | 20 +++++------ lib/compress/range.ts | 1 + lib/config.ts | 2 +- lib/prompts/system.ts | 2 +- tests/quality-gate-enforcement.test.ts | 8 ++--- 7 files changed, 51 insertions(+), 16 deletions(-) diff --git a/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md b/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md index ed6448e2..76f6ac77 100644 --- a/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md +++ b/devlog/2026-07-23_quality-gate-rejection-prompt/WORKLOG.md @@ -86,3 +86,36 @@ rate-limit" — would worsen over-blocking; explicitly rejected by user. - **Still out of scope**: making the gate fully advisory (warn-only). This PR keeps it blocking for non-ACK calls; free-ACK is the middle ground. + +## Iteration 3 — hardcoded-values cleanup (review follow-up) + +- **Trigger**: review (dog/opencode-acp#34) flagged three hardcoded values that + drifted from config reality. +- **Fix 1 — `ratioNum` dead code** (`rejection.ts`): the PR added a + `ratioNum: number | null` field to `computeStats`'s return, but no caller + read it (`buildRecoveryGuidance` uses `originalTokens`; metrics use the + pre-formatted `ratio` string). Leftover from a dropped ratio-branching idea. + Removed the field + its computation; inlined `ratio` again. +- **Fix 2 — `12000` stale example** (`rejection.ts`, `system.ts`): the + escapeHatches string and system-prompt both hardcoded `12000` as a + `summaryMaxChars` example, but the real `maxSummaryLengthHard` default is + `20000` (`config.ts`), and the tool schema already interpolates the real + value. The example was both wrong and frozen (didn't follow config). + - `rejection.ts`: `buildQualityRejectionError` now takes + `maxSummaryLengthHard: number` as a 3rd param; `buildRecoveryGuidance` + receives it and interpolates `${maxSummaryLengthHard}`. + Callers `range.ts`/`message.ts` pass `ctx.config.compress.maxSummaryLengthHard`. + - `system.ts`: removed the number entirely (static prompt string can't + interpolate config; now says "see the tool schema for the current max"). + - `tests/quality-gate-enforcement.test.ts`: all 4 call sites updated to pass + `20000`. +- **Fix 3 — `layer1MinRetentionPct` 5.0 vs 1.0** (`config.ts`): the runtime + default was `5.0` but `dcp.schema.json`, `README.md`, and the cc-alg + `DEFAULT_ROUGE_RECALL_V1_CONFIG` all say `1.0`. The v1.13.0 changelog + documents the floor as 1%. The `LARGE_RANGE_TOKENS = 50_000` threshold + rationale (crossover where the 1% floor demands a multi-thousand-char + summary) assumes 1%; at 5% the real crossover is ~12K tokens. Aligned + `config.ts` to `1.0`. Updated the `LARGE_RANGE_TOKENS` comment to name the + config key and note the proportional coupling. +- **Verification**: `npm run typecheck` clean; `npm test` 836 pass / 0 fail; + `npm run build` ok. diff --git a/lib/compress/message.ts b/lib/compress/message.ts index 5b10764b..2ede37e5 100644 --- a/lib/compress/message.ts +++ b/lib/compress/message.ts @@ -220,6 +220,7 @@ export function createCompressMessageTool(ctx: ToolContext): ReturnType 0 ? originalTokens / Math.max(summaryChars / 4, 1) : null - const ratio = ratioNum !== null ? ratioNum.toFixed(1) : "?" + const ratio = originalTokens > 0 ? (originalTokens / Math.max(summaryChars / 4, 1)).toFixed(1) : "?" const retentionPct = originalTokens > 0 ? ((summaryChars / (originalTokens * 4)) * 100).toFixed(2) : "?" - return { originalTokens, summaryChars, ratio, ratioNum, retentionPct } + return { originalTokens, summaryChars, ratio, retentionPct } } // A range this large cannot be summarized densely enough in one pass: the L1 -// retention floor (1% of originalTokens*4 chars) would demand a multi-thousand -// char summary. At this size, splitting is the correct recovery, not rewriting. +// retention floor (qualityGate layer1MinRetentionPct, default 1% → originalTokens*4*0.01) +// would demand a multi-thousand char summary. At this size, splitting is the +// correct recovery, not rewriting. If layer1MinRetentionPct is raised, this +// crossover shrinks proportionally. const LARGE_RANGE_TOKENS = 50_000 -function buildRecoveryGuidance(stats: ReturnType): string { +function buildRecoveryGuidance(stats: ReturnType, maxSummaryLengthHard: number): string { const isLargeRange = stats.originalTokens > LARGE_RANGE_TOKENS - const escapeHatches = `If the hard length limit is too tight for important detail, pass "summaryMaxChars" (e.g. 12000) to raise the cap. + const escapeHatches = `If the hard length limit is too tight for important detail, pass "summaryMaxChars" (e.g. ${maxSummaryLengthHard}) to raise the cap. As a LAST RESORT only — after you have genuinely tried the above — add "acknowledgeRisk": true to accept the information loss and force the compression through. Without acknowledgeRisk: true on the retry, the compression will be rejected again.` if (isLargeRange) { @@ -82,6 +81,7 @@ ${escapeHatches}` export function buildQualityRejectionError( plan: RejectionPlanInfo, result: QualityGateResult, + maxSummaryLengthHard: number, ): Error { const stats = computeStats(plan) const metrics = [ @@ -105,7 +105,7 @@ Your summary becomes the SOLE record. If it fails, subsequent work is built on a memory loss → wrong assumptions → entire reasoning chain collapse. Treat every compression with maximum care. -${buildRecoveryGuidance(stats)} +${buildRecoveryGuidance(stats, maxSummaryLengthHard)} ${HOW_TO_COMPRESS_RULES}` diff --git a/lib/compress/range.ts b/lib/compress/range.ts index 2148a689..f288dae2 100644 --- a/lib/compress/range.ts +++ b/lib/compress/range.ts @@ -311,6 +311,7 @@ export function createCompressRangeTool(ctx: ToolContext): ReturnType50K tokens), one summary can rarely be dense enough to pass. Break it into 2-3 smaller contiguous ranges and compress each separately in the SAME batch \`compress\` call (each entry gets its own \`topic\` and \`summary\`). -2. WRITE a denser / longer summary. For a small range, keep every load-bearing detail (full file paths with line numbers, signatures, exact errors, decisions WITH their rationale). If the hard cap is too tight, pass \`summaryMaxChars\` (e.g. 12000) to allow a longer summary. +2. WRITE a denser / longer summary. For a small range, keep every load-bearing detail (full file paths with line numbers, signatures, exact errors, decisions WITH their rationale). If the hard cap is too tight, pass \`summaryMaxChars\` to allow a longer summary (see the tool schema for the current max). 3. ACKNOWLEDGE RISK when you judge the information loss acceptable. Set \`"acknowledgeRisk": true\` to bypass the gate — usable on the first attempt, no need to trigger a rejection first. Use it for low-value ranges or when a dense summary is not feasible; for content that matters, prefer (1) or (2). Do NOT loop: if the same range is rejected twice, you MUST change strategy (split it). Never resubmit an identical or near-identical summary — it will be rejected again and waste context. diff --git a/tests/quality-gate-enforcement.test.ts b/tests/quality-gate-enforcement.test.ts index 4da19d22..743e866f 100644 --- a/tests/quality-gate-enforcement.test.ts +++ b/tests/quality-gate-enforcement.test.ts @@ -233,7 +233,7 @@ test("buildQualityRejectionError includes range, stats, and acknowledgeRisk inst ], } - const error = buildQualityRejectionError(plan, result) + const error = buildQualityRejectionError(plan, result, 20000) const msg = error.message assert.ok(msg.includes("COMPRESSION REJECTED"), "should have rejection header") @@ -259,7 +259,7 @@ test("buildQualityRejectionError computes ratio and retention from plan data", ( metrics: [], } - const error = buildQualityRejectionError(plan, result) + const error = buildQualityRejectionError(plan, result, 20000) assert.ok(error.message.includes("1000 tokens"), "should show original tokens") assert.ok(error.message.includes("100 chars"), "should show summary chars") }) @@ -280,7 +280,7 @@ test("buildQualityRejectionError advises SPLITTING for a large range (>50K token metrics: [], } - const error = buildQualityRejectionError(plan, result) + const error = buildQualityRejectionError(plan, result, 20000) const msg = error.message assert.ok(msg.includes("SPLIT THE RANGE"), "large range should lead with split guidance") @@ -309,7 +309,7 @@ test("buildQualityRejectionError advises a denser summary for a small range", () metrics: [], } - const error = buildQualityRejectionError(plan, result) + const error = buildQualityRejectionError(plan, result, 20000) const msg = error.message assert.ok(