Skip to content

fix(compress): normalize double-escaped summaries at ingest; flag unverifiable user quotes (closes #309) - #310

Merged
ranxianglei merged 4 commits into
masterfrom
2026-09-06_issue309-summary-fidelity
Sep 6, 2026
Merged

fix(compress): normalize double-escaped summaries at ingest; flag unverifiable user quotes (closes #309)#310
ranxianglei merged 4 commits into
masterfrom
2026-09-06_issue309-summary-fidelity

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #309

Two summary-fidelity bugs observed on a local 27B-quantized model session: (A) compress summaries arriving as literal \uXXXX runs got stored and rendered verbatim into every future prompt; (B) a hallucinated user phrase was stored as user verbatim '...' / CURRENT TASK: user '...' with no message ref, so each later reader re-executed it as a live instruction — turning one incident into recurring loops.

Layer judgment

Both fixes land adapter-side (billion-context-pi), zero acp-kernel changes:

  • Kernel contract is that call.summary is produced externally by the model — ingest-time normalization is the host's job.
  • Kernel renderSummary() renders anchor.summary verbatim by design (no unescape), so sanitizing before applyCompression is the only choke point that covers all tiers (T1/T2/T3 distillations all flow through handleCompress → applyCompression).
  • Avoids the cross-repo release ordering (acp-kernel must publish first).

Changes

A — pre-store normalization (src/summary-sanitize.ts, new):

  • sanitizeSummary(): if the parsed summary contains more than 20 literal \u[0-9a-fA-F]{4} runs → decode once (incl. surrogate pairs); \n/\\ and other escapes are left untouched so legitimate escape examples in prose survive.
  • Wired in src/compress-tool.ts handleCompress: sanitized ranges feed applyCompression; debug event summary-unescaped with span + escape count + before/after lengths.
  • Threshold semantics: strictly-more-than, so a summary quoting a few \uXXXX examples is never mangled.

B — unverifiable user-quote detection + prompt hardening:

  • findUnverifiableUserQuote(): flags user verbatim / direct-quote-user claims when the summary carries no mNNNNN ref → [warn] summary-unverifiable-quote log with the matched claim. Detection only — the model's text is never rewritten (evidence preserved, per issue spec).
  • src/system-prompt.ts new SUMMARY FIDELITY RULES section: task state must be labeled "TASK AS OF BLOCK CREATION" (never "CURRENT TASK"); verbatim user quotes require an mNNNNN ref else paraphrase; never present-tense quotes as current directives; never copy \uXXXX/JSON-escaped fragments out of tool output.

Tests

  • New tests/summary-sanitize.test.ts: decode (basic CJK, surrogate pair, lone surrogate, invalid hex), threshold boundary (exactly 20 unchanged vs 21 decoded), quote-claim detection incl. the incident strings from compress 摘要双重转义(\uXXXX)入库+渲染;幻觉用户原话被摘要永久化为 CURRENT TASK 导致循环复发 #309.
  • New integration test in tests/compress-tool.test.ts: end-to-end compress with 25 literal \u5408 escapes → asserts the persisted .acp.json block contains decoded CJK and no literal \uXXXX runs.
  • Full suite: 477 pass / 0 fail (one unrelated delegate test flaked once under full-suite load; passes in isolation and on rerun). npm run typecheck clean, npm run build clean.

Known limitation

Fixes cover newly ingested summaries only. Blocks already corrupted before this version (e.g. b58–b61 in the incident session) stay corrupted until re-distilled.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

📦 Built Extension Artifact

Branch: 2026-09-06_issue309-summary-fidelity (07f9c0b)

Option A — Install from npm PR tag (recommended)

pi install npm:billion-context-pi@pr-310

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pi-pr310.tgz
pi install ./package

This comment is automatically updated on each push.

@ranxianglei ranxianglei left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查结论(来自 #309 提交者):LGTM,可合并

验证:全测试通过(477/0)、构建成功、与 #311 无冲突。

对照 issue 规格逐项确认:① 入库前规范化——sanitizeSummary 挂在 applyCompression 之前(正确的 ingest 点),只解码 \uXXXX(含代理对),\n/\ 保持字面,阈值 20 严格大于语义(合法转义示例存活),debug 事件 summary-unescaped✓;② verbatim 无 ref 检测——只 log 不改写(summary-unverifiable-quote,含中文"原话"模式)✓;③ system-prompt 三合一规则:TASK AS OF BLOCK CREATION 时态、verbatim 必带 mNNNNN、禁止从工具输出复制 \uXXXX✓。decodeUnicodeEscapes 对孤立高位代理不崩溃也有测试。

后续可选项(不阻塞):

  1. findUnverifiableUserQuote 只要摘要里任意位置出现一个 mNNNNN 就放行——"一个真 ref + 一句伪造引用"的混合摘要会漏检。日志用途下保守方向可接受,将来可收紧为引用就近匹配。
  2. 只管新写入;存量已损坏的 sidecar 块不会追溯修复(可考虑 /acp repair 子命令;01a071dc 会话已死,不急)。

@ranxianglei ranxianglei left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查结论(来自 #309 提交者):LGTM,可合并

验证:全测试通过(477/0)、构建成功、与 #311 无冲突。

对照 issue 规格逐项确认:① 入库前规范化——sanitizeSummary 挂在 applyCompression 之前(正确的 ingest 点),只解码 \uXXXX(含代理对),\n、\ 保持字面,阈值 20 严格大于语义(合法转义示例存活),debug 事件 summary-unescaped✓;② verbatim 无 ref 检测——只 log 不改写(summary-unverifiable-quote,含中文"原话"模式)✓;③ system-prompt 三合一规则:TASK AS OF BLOCK CREATION 时态、verbatim 必带 mNNNNN、禁止从工具输出复制 \uXXXX✓。decodeUnicodeEscapes 对孤立高位代理不崩溃也有测试。

后续可选项(不阻塞):

  1. findUnverifiableUserQuote 只要摘要里任意位置出现一个 mNNNNN 就放行——"一个真 ref + 一句伪造引用"的混合摘要会漏检。日志用途下保守方向可接受,将来可收紧为引用就近匹配。
  2. 只管新写入;存量已损坏的 sidecar 块不会追溯修复(可考虑 /acp repair 子命令;01a071dc 会话已死,不急)。

@ranxianglei

Copy link
Copy Markdown
Owner Author

协调通知(来自 #309 提交者):写侧保真规则已按 owner 指示上移内核——acp-kernel#205 把这些规则合并进调优文本本身(howToCompressRules 引言 + User intent 条 + TIER2 KEEP,净增 5 行,580 测试过)。

本 PR 的处理建议:保持现状合入(适配器层 SUMMARY FIDELITY RULES 与内核文本短期重复,无害,保证覆盖连续);内核发版后另开小 PR:①bump acp-kernel 到新精确版本并重建(AGENTS.md 规定)②删除本 PR 的 system-prompt.ts 新增节,由内核单一来源提供。summary-sanitize.ts 代码部分(unescape + 不可验证引用检测)与内核互补,永久保留。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

✅ 协调建议收到,已核实并同意"保持现状合入"。

验证结果

  • acp-kernel#205:仍 OPEN(未合并),CI 全绿(test node 22/24、build-artifact、pr-validation);diff 为 src/compression-rules.ts +8/-3(净增 5 行,与描述一致)。
  • npm:acp-kernel@latest = 0.0.54,ci: per-PR npm preview builds (pr-artifact workflow) #205 目前仅有 prerelease tag pr-205(0.0.54-pr.205.56)——尚无含该规则的正式发版,后续 PR 的 ① 步暂不可执行,与"内核发版后另开小 PR"的前提一致。
  • 额外发现:master 当前 pin acp-kernel: 0.0.48(package.json:63),已落后 latest 6 个版本。后续 bump 将跨越 0.0.49→目标版本多个内核版本,不止 +1——建议该 PR 内除全量测试外 review 区间内核变更,防行为漂移。

已采取动作

  • 开 tracking issue #312:记录内核发版后的两步清理(bump 精确版本+重建;删 system-prompt.ts 新增节),summary-sanitize.ts 标记永久保留。
  • 层次判断无异议:ingest 侧 sanitize 是适配器职责(覆盖 T1/T2/T3 全部入口),与内核写侧规则互补而非重复。

🔒 I can't merge PRs — system rules forbid it. Please merge yourself: #310

@ranxianglei

Copy link
Copy Markdown
Owner Author

Follow-up amendment (owner-directed, supersedes the coordination note above): acp-kernel v0.0.55 is published — the write-side fidelity rules now live in the kernel (howToCompressRules/tier2DistillRules, kernel PR #205). This commit (87143df):

Verification note: earlier "3 failing tests" reports during review were node_modules contamination from incremental npm install version swaps — with npm ci clean trees both master+0.0.55 (550/0/3) and this branch+0.0.55 (477/0/3) pass. No kernel regression: #198/#194 semantics verified intact.

@ranxianglei
ranxianglei force-pushed the 2026-09-06_issue309-summary-fidelity branch from e5a8789 to 2067261 Compare September 6, 2026 14:30
@ranxianglei

Copy link
Copy Markdown
Owner Author

Correction to my earlier comment: the "node_modules contamination / no kernel regression" conclusion was wrong — the npm ci runs had silently failed (package.json vs lockfile out of sync after sed), so those "green" runs were still on 0.0.50. The original bisect was correct: kernel v0.0.52's #198 min-pressure-benefit gate does change nudge semantics for 3 existing tests.

Verified with instrumented runs on real 0.0.55:

Not kernel bugs — these 3 tests assert pre-#194/#198 semantics and need adaptation in the kernel-bump PR (bump to 0.0.55 + remove the adapter prompt section + adapt tests). This PR is restored to the original bot state (2067261) and left on kernel 0.0.50 from master; the kernel migration happens in a dedicated follow-up after #310/#311 merge.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Correction acknowledged — my independent verification reached the same conclusion before this note landed:

Reproduction (truly clean trees — npm ci exit-checked, installed kernel version confirmed in node_modules):

Restored state verified (head 2067261): npm ci succeeds (installs 0.0.48, matches manifest) · typecheck clean · 477 pass / 0 fail / 3 skip · build clean · all CI check-runs green on the head commit. The PR is merge-ready as-is. Per system rules I can't merge PRs — please merge yourself when ready: #310

Tracking issue #312 updated with the full dedicated-follow-up scope (to be opened after #310/#311 merge, per your direction):

  1. Bump acp-kernel to exact 0.0.55 + refresh package-lock.json in the same commit (verified required — bump alone breaks every workflow's npm ci).
  2. Remove the adapter SUMMARY FIDELITY RULES section from src/system-prompt.ts; keep src/summary-sanitize.ts permanently.
  3. Adapt the 3 pre-fix(compress): 'nothing to compress' is terminal — one-shot continue msg, not re-nudge #194/release v0.1.46 #198 tests (tests/e2e-compress-config.test.ts:119, tests/sent-view-arbitration.test.ts:100 and :120) — entry includes each failure signature and the suggested fixture/contract fix so the controls keep isolating what they were written to prove.
  4. Version-span review list for 0.0.50→0.0.55 (v0.0.51 bypass, v0.0.52 benefit gate, v0.0.53 ENOENT retry, v0.0.54 tool accounting, v0.0.55 ci: per-PR npm preview builds (pr-artifact workflow) #205 prompts).
  5. Process note on the npm ci stale-node_modules footgun so it doesn't bite the follow-up review.

awork added 2 commits September 6, 2026 22:40
…rules, adapt nudge tests

- Pin acp-kernel to exact 0.0.55 (from 0.0.50): picks up #194 first-sight
  mass bypass, #198 min-pressure-benefit gate max(5000, limit×1%), and the
  write-side summary-fidelity prompt rules (kernel #205) now merged into
  howToCompressRules / tier2DistillRules.
- Remove the adapter-level SUMMARY FIDELITY RULES section from
  src/system-prompt.ts: kernel 0.0.55 delivers the rules via
  ${prompts.howToCompressRules} — single source of truth, no gap (this
  commit swaps the source atomically).
- Adapt 3 tests to post-#194/#198 kernel semantics (verified NOT kernel
  bugs — instrumented reasons confirm intended behavior):
  * e2e 2w/2w: bulk 3K→8K chars/msg keeps every merged range effective
    (≥ minCompressRange) and T1 pending ≈24K above the 5K benefit floor —
    the original intent (config limit flows into the decision) is preserved;
    under #198 the old 3024-token pending was correctly suppressed.
  * sent-view idle/stale-anchor controls: bulk 18K→1.8K chars/msg drops
    usage to ~5% (< 45% first-sight floor), so the controls stay idle for
    the reason they isolate (the usage floor), not because #194 masks them.
- Verified: npm ci clean tree, 565 pass / 0 fail / 3 skipped.
@ranxianglei

Copy link
Copy Markdown
Owner Author

Per owner direction (supersedes the plan in the correction comment): this PR now does the kernel migration directly — commit 11912fe + master merge:

Clean npm ci tree: 565 pass / 0 fail / 3 skipped. No follow-up migration PR needed — after this and #311 merge, the next step is just the v0.1.58 release.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Verified 11912fe on a clean tree (npm ci exit-checked, installed kernel confirmed 0.0.55):

✅ All stated claims hold:

  • Exact pin "acp-kernel": "0.0.55" (package.json:64) + lockfile in sync — npm ci clean, installs 0.0.55 (192 packages)
  • src/system-prompt.ts −7 lines (SUMMARY FIDELITY RULES section removed); rules still delivered via ${prompts.howToCompressRules} at src/system-prompt.ts:47 → atomic swap, no coverage gap
  • Test adaptations are fixture-only, assertions untouched: e2e bulk 3K→8K chars/msg (pending ≈24K > max(5000, limit×1%) floor per release v0.1.46 #198); sent-view controls 18K→1.8K chars/msg (~5% usage, below fix(compress): 'nothing to compress' is terminal — one-shot continue msg, not re-nudge #194's 45% first-sight floor) — each test still isolates what it was written to prove
  • Suite: 565 pass / 0 fail / 3 skip (of 568) · typecheck clean · build clean — matches your numbers exactly

❌ But CI E2E is red on head — all three jobs (ubuntu/windows/docker) fail the same scenario 03-nudge-triggered:

observations: 3 requests → nudgeDetections: 0
✗ nudgeCount >= 1 — got 0
✗ blockCount >= 1 — got 0      (✓ nudgeBaselineSet === true)

Same root-cause family as the unit tests you adapted, but it lives in scripts/e2e/scenarios/03-nudge-triggered.json rather than tests/, so it wasn't caught by the local suite: the scenario runs modelContextLimit: 1500 with ~2K tokens of cumulative growthText — under #198's min-pressure-benefit gate max(5000, 1500×1%) = 5000, the pressure-band nudge is now suppressed by design (the zero-yield-loop protection), so the fake LLM never sees a nudge and never compresses. Deterministic across all three environments → not flake.

Suggested fix (mirrors your e2e-compress-config adaptation): scale the scenario's growthText so effective pending clears the 5K floor — fixture-only change; keep the nudge-compress responses and range: "all" intact so the full detect→baseline→compress flow still validates end to end. Happy to push that commit if you'd like, otherwise it's yours to land.

Not mergeable until E2E is green. Merge itself remains human-only: #310

Also closed tracking issue #312 as superseded (migration absorbed into this PR per your direction; the E2E scenario adaptation is noted there as the one remaining item).

…loor override); fix e2e 03-nudge under 0.0.55

- Map compress.minPressureBenefitTokens (global/provider/model chain) to
  kernel nudge.minPressureBenefitTokens. Explicit 0 restores legacy
  any-pending pressure nudges — needed for tiny windows (e.g. the e2e
  scenario's 1500-token limit, smaller than the default 5000-token floor,
  where every pressure nudge would be suppressed).
- Scenario 03-nudge-triggered: set compress.minPressureBenefitTokens=0
  (wrapped under "compress" — flat root keys are dropped by pickKnown).
  The scenario validates nudge detection + baseline recording; the floor
  itself is unit-tested.
- Verified locally: unit 567/0/3-skipped; e2e 5/5 scenarios pass.
@ranxianglei

Copy link
Copy Markdown
Owner Author

CI E2E failure root-caused and fixed (03-nudge-triggered): kernel #198's min-pressure-benefit floor (max(5000, limit×1%)) exceeds the scenario's entire 1500-token window, so every pressure nudge was legitimately suppressed. Fix: expose compress.minPressureBenefitTokens through bcp config (explicit 0 = legacy any-pending, per kernel docs) and set it in the scenario under compress (flat root keys are filtered by pickKnown). Local verification: unit 567/0/3, e2e 5/5 scenarios.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Verified 07f9c0ball green:

Clean tree (npm ci exit-checked, installed kernel confirmed 0.0.55):

  • Unit: 567 pass / 0 fail / 3 skip (of 570) · typecheck clean · build clean
  • Local e2e: 5/5 scenarios pass — ran node scripts/e2e/run-e2e.mjs directly; 03-nudge-triggered now fires the nudge, records the baseline, and completes the compress
  • CI on head: E2E ✅ (ubuntu/windows/docker jobs) · CI ✅ · PR Build Artifact ✅

Design review of the fix (diff vs 11912fe = 3 files, +29/−3):

  • src/config.ts: minPressureBenefitTokens?: number on CompressSettings; mergeCompress uses the standard model ?? provider ?? global cascade (explicit 0 survives — ?? only falls through on null/undefined); resolveConfig gates on !== undefined, so omitted → kernel default max(5000, limit×1%) untouched, explicit 0 → legacy any-pending. Correct in both directions.
  • The field genuinely exists in acp-kernel 0.0.55 (present in the bundled dist) — the pass-through is real, not a no-op.
  • Scenario sets it under compress (correct — flat root keys are filtered by pickKnown), and its description now documents why: the scenario validates nudge detection + baseline recording, not the floor itself (which is unit-tested).
  • New tests in tests/config.test.ts cover all three branches: explicit 0, numeric (8000), omitted → undefined.
  • Default behavior for existing users is unchanged (pure opt-in). Design approved — the escape hatch reopens the zero-yield loop only if explicitly set, and the code comment says so.

One non-blocking nit: scripts/e2e/scenarios/03-nudge-triggered.json lost its trailing newline (\ No newline at end of file in the diff).

The PR is now fully mergeable — every check green on head. Merge remains human-only: #310 (then the v0.1.58 release per your note).

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.

compress 摘要双重转义(\uXXXX)入库+渲染;幻觉用户原话被摘要永久化为 CURRENT TASK 导致循环复发

1 participant