perf: arm compression plans from nudges

[frozenranger]

Fixes #436

Summary

• arm one canonical raw-message compression plan directly from ACP nudges
• keep acp_status as optional diagnostics/refresh and enforce stale/visibility guards
• merge adjacent safe segments for useful compression payloads while preserving tier block compression
• resolve provider/model overrides consistently and clean up per-session snapshots

Verification

• npm run typecheck
• npm test (1237/1237)
• npm run build
• changed-file Prettier check
• two independent read-only reviews: APPROVE

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 PR review — arm compression plans from nudges

I checked out frozenranger:2026-09-17_nudge-armed-compression (4bb363d7) and reviewed the full diff (17 files, +1842/-298) against master. Summary below.

Verification (local)

• npm run typecheck: pass
• npm test: 1230 tests, 1228 pass, 2 fail — both environmental, not regressions. This sandbox mounts /tmp read-only, so tests/soft-block.test.ts:14 (mkdirSync('/tmp/opencode-dcp-dangerous-…') crashes file load) and one E2E case writing /tmp/test-inactive-block-decompress.txt fail. Neither file is touched by this PR, and both fail identically on master. The claimed 1237/1237 vs local 1230 total looks like an environment/Node-version difference — CI is authoritative.
• Design-level checks I ran against the SDK types: session.idle / session.deleted event names and property extraction (properties.sessionID ?? properties.info.id) match EventSessionIdle/EventSessionDeleted in @opencode-ai/sdk exactly; the event-handler early-return is safe (the rest of the handler only processes message.part.updated); assignMessageRefs is idempotent for known raw IDs, so calling it from acp_status cannot shift model-visible refs; when no plan can be armed, nothing is injected and lastNudgeShownTokens is not updated (it lives inside if (shouldInject)), so there's no baseline corruption — the next turn retries, and ≥98% still fires the emergency notice via emergencyOverride.
• Plan lifecycle is sound: armed only after resolving candidates through the real executor path (resolveRanges + protected/last-user/recent filters + visibility snapshot), strict validation (10-min TTL, structureVersion, exact start/end IDs, exact ordered message set, char count within ±300 or 2%), consumed only on success, cleared on idle/deleted. Tier block compression (b-refs) correctly bypasses the smart plan, so T2/T3 distillation is unaffected. Zero overhead when the flag is off. No persisted-state format change (in-memory module maps only).
• Tests look solid: 6 dedicated smart-plan.test.ts cases (selection, exclusion, altered-bounds/stale/expiry rejection, adjacent-split merging under an 80K floor, visibility-snapshot refusal) and new inject.test.ts cases that assert lastPerMessageNudgeTokens side effects across turns with production-like preserveRecentMessages.

Blocker: merge conflict with master

The PR is currently not mergeable (mergeable_state: dirty). Master has moved since the branch was cut (v1.18.1 release, #403 Windows path normalization). Content conflicts in lib/compress/range.ts and lib/compress/status.ts, plus both-sided changes in dcp.schema.json, lib/config.ts, lib/config-validation.ts, lib/hooks.ts, lib/messages/inject/inject.ts. Please rebase onto master before merge. (CI was still pending at the time of this review.)

Findings

F1 (medium, backward compat) — system prompt change is ungated. lib/prompts/system.ts now tells every model to "Use the exact SMART PLAN in the latest ACP nudge", but with the default smartPlanRequired: false no SMART PLAN line ever appears in nudges or acp_status. Default users get instructions pointing at a feature they don't have. The repo already has a mechanism for conditional prompt content (renderSystemPrompt() base + extensions, cf. lib/prompts/extensions/nudge.ts). Recommendation: keep the base wording neutral ("verify via acp_status({scope:"uncompressed"})") and emit the SMART PLAN sentences as a flag-gated extension.

F2 (medium, backward compat) — range segmentation change is ungated. In lib/messages/inject/utils.ts (buildCompressibleRanges, ~line 900) the split rule changed from "split at user message after 3 msgs, or gap" to shouldSplitUser = isUser && (tokens >= 10000 || count >= 25) / shouldSplitSize = tokens >= 16000 || count >= 40. This function takes no config, so it applies to all users: small sessions get fewer/larger recommended ranges, large tool runs get split at 16K tokens/40 msgs. The thresholds look tuned for your local 80K-char minCompressRange policy but ship globally. Recommendation: either thread the flag (or thresholds) through so default behavior is unchanged, or explicitly document this as an intentional global improvement in the devlog/changelog. Your call — happy to patch either way.

F3 (low) — CONFIGURATION.md doesn't document the new compress.smartPlanRequired key (schema description exists, but the parameter reference should too).

F4 (low, confirm intent) — with the flag on, if the maxLimit variant triggers but no plan can be armed, the strong "Context limit reached" alert is also suppressed (condition changed from !emergencyNoTargets to && shouldInjectNudge), so between maxLimit and the 98% emergency threshold the model gets silence. Rare with the default minCompressRange: 0 (only visibility-mismatch/empty-selection cases reach it), but please confirm that's the intended safety trade-off.

F5 (nit) — DESIGN.md says behavior "remains gated by existing smartPlanRequired"; the flag is introduced by this PR (verified absent from master during the #436 triage).

F6 (minor) — heavy Prettier reflow noise in the touched files inflates the diff. Acceptable (repo standard) but noted for future diffs.

Verdict

Executor logic, validation guards, and tests check out — no correctness blockers found. Before merge: (a) rebase onto master, (b) resolve F1/F2 (gate or justify), ideally (c) add the CONFIGURATION.md entry. If you want, I can prepare patches for F1–F3+F5 once you decide gate-vs-global for F2. Merge remains a human operation per repo rules.

中文摘要：审查了 nudge 武装压缩计划的实现，执行器校验、可见性快照与测试均无正确性问题；但 PR 与 master 存在合并冲突需先 rebase，且系统提示词和范围切分阈值两处改动未受 smartPlanRequired 开关保护会影响默认用户（建议改为按开关启用或明确说明），解决后可合并。

[frozenranger]

Patched after live native-compaction failure and reviewer feedback. Added stable message-ref high-water preservation, full-history mixed-epoch repair via acp_status, non-contiguous range boundaries, nudge-armed canonical plans sized toward the native watermark, execution-side exact-plan validation, provider/model consistency, docs/tests, and native template hardening for serialized tool markers/plain headings. Verification: typecheck PASS; focused 115/115; full 1311/1311; build PASS; native compaction regression PASS; two independent final reviews APPROVE. Commit: 2f824e5.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Re-review — 2f824e5b (post-compaction hardening)

Pulled the rebased head (87b19972 + 2f824e5b, base = master tip 026dbe73, clean rebase — the earlier conflict is gone) and re-reviewed everything, including the new commit.

Previous findings — all addressed ✅

• F1 (ungated system prompt): fixed. Base prompt now reads "Use the current nudge target when one is provided, or verify the range via acp_status" — neutral wording, no SMART PLAN references for default users.
• F2 (ungated segmentation thresholds): resolved better than I suggested. The size-based split thresholds are gone; instead hasGap is now info.refNum !== prevRefNum + 1 (lib/messages/inject/utils.ts), so backwards alias jumps become hard boundaries. Healthy monotonic sessions behave identically to master; mixed-epoch sessions get split at the epoch seam instead of displaying impossible ranges like m01621–m00008.
• F3 (docs): CONFIGURATION.md now documents compress.smartPlanRequired (type/default/status/description).
• F5 (DESIGN.md): now says "newly introduced smartPlanRequired". Correct.
• Rebase: done onto current master; mergeable_state is only "blocked" because the required e2e check was still in progress at review time (pr-validation, build, test(22), test(24), build-artifact all green).

New commit reviewed — root cause and fix are sound

The native-compaction failure root cause checks out: the old [FIX] in resetOnCompaction (lib/state/utils.ts) reset state.messageIds to nextRef: 1 after native compaction, starting a second alias epoch while older messages kept high refs — hence reversed/impossible ranges when full history was fetched later. The fix preserves aliases + high-water mark across compaction (raw OpenCode IDs are stable in history, so this is correct), and adds repairNonMonotonicMessageRefs() (lib/message-ids.ts) for already-corrupted persisted sessions: detects non-monotonic refs, renumbers chronologically from a freshly fetched full history, appends legacy aliases after the rebuilt epoch, rebuilds each block's startId/endId from its effective/direct IDs, and bumps structureVersion (which invalidates any armed plan — consistent). It runs unconditionally in acp_status and persists state only when a repair happened. Healthy sessions return early with zero mutation.

Also verified: plan sizing toward the watermark (targetChars = max(minCompressRange, (total − maxLimit + 8192) × 4) chars/token; prefers oldest candidate ≥ target for prefix-cache locality, else largest candidate); filteredPlans[0]! in range.ts is safe because prepareExecutableRangePlans throws on empty plans; tier block compression still bypasses the mandatory single-range policy. New tests are faithful: high-water-mark preservation regression + mixed-epoch repair with block-boundary rebuild, plus a provider/model-override status test.

Local verification

• npm run typecheck: PASS
• npm test: 1304 tests, 1302 pass, 2 fail — both known sandbox-environmental (/tmp read-only here: soft-block.test.ts mkdir, and one E2E case whose toFile path /tmp/... violates this environment's workspace-only rule); identical on master, not regressions. The claimed 1311/1311 vs local 1304 total is again an environment difference — CI's test jobs are green and authoritative.

Remaining nits (non-blocking)

1. WORKLOG verification numbers are stale: the Verification block still lists 1237/1237 and 112/112 from the first commit, but the comment reports 1311/1311 and 115/115 after the hardening commit. I didn't patch it directly because I can't independently reproduce your exact counts in this sandbox — please refresh that block (and the "Updated:" date) to the final state.
2. Minor: repairNonMonotonicMessageRefs's eligibility filter doesn't apply assignMessageRefs' subagent first-user-message skip. Only reachable on corrupted legacy sessions, and the effect is just one extra alias — noting for completeness, no action required unless you want strict parity.

Verdict

All prior blockers and findings resolved; the compaction-coordination logic is correct and well-tested. Ready to merge once e2e goes green — merge itself stays a human operation per repo rules.

中文摘要：复审了 rebase 后的新提交，此前全部问题（系统提示词未加开关、切分阈值全局生效、缺文档、devlog 措辞、合并冲突）均已修复，新增的压缩纪元保留与混合纪元自动修复逻辑经核查正确且有回归测试；仅剩 WORKLOG 验证数字未更新等两处小瑕疵，e2e 转绿后可合并。