Skip to content

fix: concise quality-gate rejection message (issue #396) - #397

Merged
ranxianglei merged 4 commits into
masterfrom
2026-09-15_concise-quality-rejection
Sep 15, 2026
Merged

ranxianglei merged 4 commits into
masterfrom
2026-09-15_concise-quality-rejection

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem (issue #396)

When the pre-commit quality gate rejects a compression, the error returned to the model embedded the complete HOW_TO_COMPRESS_RULES (~4,936 chars ≈ ~1,234 tokens) plus a multi-line "⚠️ CRITICAL" warning paragraph — on every rejection. For a failed compression whose purpose is to save context, this re-injects ~5.4K chars of new text into the model's context. The content is also 100% redundant: the same constant is already in the ACP system prompt (lib/prompts/system.ts:58), sent with every request.

Secondary defects found while fixing:

Fix (lib/compress/quality-gate/rejection.ts)

  • Removed the HOW_TO_COMPRESS_RULES import/embedding and the CRITICAL warning paragraph.
  • Added a Reason: line from result.reason.
  • Rewrote retry guidance into three sentences: rewrite a more complete summary and retry (gate re-evaluates automatically); full rules already live in the system prompt; acknowledgeRisk: true bypasses this rejection once if you're confident the summary is correct despite the metrics.

Result: standard rejection payload drops from ~5.4K chars (~1.4K tokens) to ~600 chars (~150 tokens), ~85% smaller.

Not changed: the blocking gate itself, its config/metrics, the throw/bypass mechanics in lib/compress/range.ts, and the E2E fake-LLM marker strings (COMPRESSION REJECTED / QUALITY GATE FAILURE remain in the header — scripts/e2e/fake-llm-server.ts:222,503). Advisory mode / auto-bypass after N rejections stays out of scope (open design discussion in #339).

Tests (tests/quality-gate-enforcement.test.ts)

  • Extracted shared fixture buildRejectionFixture().
  • Old assertion "should include compress rules" → inverted to absence assertions (HOW TO COMPRESS / KEEP VERBATIM / CRITICAL must NOT appear) + reason inclusion asserted.
  • New size-budget regression guard: message < 1,000 chars.
  • Integration tests (real-tool rejection, acknowledgeRisk bypass, compress acknowledgeRisk failed #301 preemptive no-op, flag cleared on success) unchanged and green.

Full suite: 1,133 pass / 0 fail; typecheck + build green. New/changed lines kept prettier-conformant (repo baseline has 430 pre-existing flagged files — no unrelated reformatting mixed in).

Devlog: devlog/2026-09-15_concise-quality-rejection/ (REQ + WORKLOG).

Fixes #396


中文摘要:质量门拒绝压缩时返回给模型的信息不再重复注入完整的 HOW TO COMPRESS 规则(该内容已在系统提示中),改为「失败原因 + 关键指标 + 简短重试指引」,单次拒绝的注入量从约 5.4K 字符降到约 600 字符;同时修正了 acknowledgeRisk 说明与实际行为不符的问题。可以合并。

The pre-commit quality gate rejection error embedded the full
HOW_TO_COMPRESS_RULES (~1,234 tokens) plus a multi-line CRITICAL warning on
every rejection — content already present in the system prompt
(lib/prompts/system.ts:58) — so a failed compression injected ~5.4K chars of
mostly redundant text into the model's context (issue #396).

- drop HOW_TO_COMPRESS_RULES embedding + CRITICAL paragraph
- add the gate reason line (result.reason was previously unused)
- rewrite retry guidance to 3 sentences; correct the acknowledgeRisk
  description to match #301/#303 semantics (flag bypasses only after a prior
  rejection set qualityGateRetryPending; a passing rewrite needs no flag)
- tests: invert rules-presence assertion to absence, assert reason, add
  <1,000-char size-budget regression guard; E2E marker strings preserved

Fixes #396
@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Plugin Artifact

Branch: 2026-09-15_concise-quality-rejection (6f1492e)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-397 --global

Each push to this PR publishes a new version under the pr-397 npm tag.

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-15_concise-quality-rejection" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr397.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Verification complete for PR #397 (2026-09-15_concise-quality-rejection @ 3d8688f). I pulled the branch and checked every claim in the description against the code — all hold up:

Diff review — clean, exactly as described. 4 files: lib/compress/quality-gate/rejection.ts (+6/-12), tests/quality-gate-enforcement.test.ts (+29/-5), devlog REQ/WORKLOG. No unrelated changes, no version bump, diff contains nothing beyond the stated purpose.

Verified against code:

  • HOW_TO_COMPRESS_RULES import and the CRITICAL warning paragraph are gone from rejection.ts; retry guidance is now three sentences (rewrite+retry / rules already in system prompt / one-shot acknowledgeRisk bypass).
  • The new Reason: line is always meaningful: context-compress-algorithms@1.3.0 populates result.reason on both failure paths (L1 "Summary too short: …", L2 "Content coverage too low: …"). The || "unknown" fallback only triggers for a hypothetical custom gate that omits the optional field (types.ts:29).
  • The corrected acknowledgeRisk wording matches real behavior: lib/compress/range.ts:333 — bypass requires acknowledgeRisk && state.qualityGateRetryPending, and the flag is set pending at range.ts:350 on rejection. So "rewrite and retry without the flag → gate re-evaluates automatically" and "flag bypasses this rejection once" are both accurate; the old "will be rejected again" text was indeed wrong under compress acknowledgeRisk failed #301/fix(compress): make preemptive acknowledgeRisk a no-op instead of an error [stable] #303 semantics.
  • E2E marker strings preserved: header still reads ⚠️ COMPRESSION REJECTED — QUALITY GATE FAILURE, and scripts/e2e/fake-llm-server.ts:222,503 are untouched.

Local runs on the PR head:

  • npm run typecheck ✅
  • Full suite: 1,133 pass / 0 fail (matches the claimed number)
  • tests/quality-gate-enforcement.test.ts: 15/15 (new absence assertions + <1000 char size guard included)
  • Measured an actual rejection payload end-to-end through buildQualityRejectionError: 672 chars with a real gate reason — consistent with the "~600 chars, ~85% smaller" claim.

CI status (head commit): pr-validation, build, build-artifact, test (22), test (24) all ✅; e2e still in progress (Docker suite). Given the marker strings are byte-identical where the fake LLM scans for them, e2e failure would be surprising, but I'd wait for it.

One note: the branch base (5135dfd) is 69 commits behind current master (the v1.17.x/v1.18.0 releases landed since). Neither of the two touched source/test files was modified on master in that window, so rebasing or merging will be conflict-free — but consider a quick rebase before merging for a tidy history.

Conclusion: verified, no defects found. Ready to merge once e2e goes green. Per project rules I won't merge it myself — please click merge when e2e finishes.


中文摘要:已拉取分支逐条核对——拒绝消息不再重注入压缩规则(实测 672 字符)、Reason 行在两条失败路径下均有内容、acknowledgeRisk 说明与 range.ts 实际语义一致、E2E 标记字符串保留、本地全量测试 1133/0 通过;CI 除 e2e(运行中)外全绿,改动文件与 master 无冲突,e2e 通过后可合并。

Round-1 code review NIT: qualityGateRetryPending is cleared at the start of
every compress call (lib/compress/range.ts:338), so the bypass window is
exactly the next compress call — not a strict one-shot bound to 'this
rejection'.
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Update: the last outstanding check e2e just finished green. All 6 CI checks on head 3d8688f are now passing (pr-validation, build, build-artifact, test (22), test (24), e2e).

PR #397 is fully verified — code, tests, and CI. Ready for you to merge whenever.

中文摘要:e2e 检查已通过,全部 6 项 CI 全绿,PR 验证完毕,可以合并。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

Review & CI status for this PR:

  • Dual-agent review (AGENTS.md §5.3 code + §5.6 tests): both APPROVE. Test reviewer ran a mutation check (swapped in master's rejection.ts) — all 3 new/changed tests fail against old code as expected, pass after restore. Both NITs fixed in follow-up commits (6dd1860, a242578).
  • CI: 6/6 green on 6f1492e — pr-validation, build, build-artifact, test (22), test (24), e2e. The e2e job exercises the rejection→retry path via the fake-LLM server, confirming the preserved marker strings keep detection working.
  • Final payload size: 620 chars vs ~5,811 pre-fix (~89% smaller); full suite 1,133/1,133.

Ready for human merge per §5.1.1.2.

中文摘要:质量门拒绝信息去掉了与系统提示重复的全量压缩规则注入(~1.2K tokens),改为失败原因+关键指标+三句重试指引,单次拒绝注入量从 ~5.8K 降到 ~620 字符;双 agent 评审通过(含变异验证)、CI 全绿(含 e2e),可以合并。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 压缩质量检测失败时返回的错误信息过长,导致额外的上下文消耗

1 participant