Skip to content

feat: expand message ref format from 4-digit to 5-digit (9999→99999) - #1

Merged
ranxianglei merged 4 commits into
masterfrom
feat/msgid-expansion
May 19, 2026
Merged

ranxianglei merged 4 commits into
masterfrom
feat/msgid-expansion

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Summary

  • Expand message ref format from m0001 (4-digit, max 9,999) to m00001 (5-digit, max 99,999)
  • Backward compatible: regex /^m(\d{4,5})$/ accepts both old 4-digit and new 5-digit refs
  • All 13 test files updated, 3 new boundary tests added

Changes

  • lib/message-ids.ts: MESSAGE_REF_WIDTH 4→5, MESSAGE_REF_MAX_INDEX 9999→99999, regex accepts 4-5 digits
  • 13 test files: all m0001→m00001 format references
  • AGENTS.md: added mandatory dual-agent review for code + tests, PR workflow

Test Results

346/346 tests pass, typecheck clean

Review Checklist

  • Backward compatible (old 4-digit refs still parse correctly)
  • No as any or type suppression
  • All tests pass
  • Typecheck passes

- MESSAGE_REF_WIDTH: 4 → 5
- MESSAGE_REF_MAX_INDEX: 9999 → 99999
- MESSAGE_REF_REGEX: /^m(\d{4})$/ → /^m(\d{4,5})$/ (backward compat)
- All test files updated to 5-digit format (m00001, m00002, etc.)
- Added boundary tests for 99999 and 100000
- Kept 4-digit backward compat parse tests (m0001 still resolves)
Critical fixes:
- state.ts: migrate 4-digit refs (m0001) to 5-digit (m00001) during state loading
  Fixes lookup failure when parseBoundaryId normalizes old refs
- compress-message.ts: restore XML tag template (accidentally destroyed in prior edit)
- prompts/tool descriptions: mNNNN → mNNNNN in all LLM-visible text
- Error messages: mNNNN → mNNNNN in search.ts and message-utils.ts
- message-ids.ts: error message range 0-99999 → 1-99999
- AGENTS.md: fix duplicate section 5.3 numbering, mNNNN → mNNNNN

Tests: +4 backward compat tests (350 total, 0 failures)
@ranxianglei
ranxianglei force-pushed the feat/msgid-expansion branch from 33bc110 to 5e54496 Compare May 19, 2026 00:52
@ranxianglei
ranxianglei merged commit 6cbd352 into master May 19, 2026
3 checks passed
ranxianglei added a commit that referenced this pull request Jul 5, 2026
Review #1 🔴: dcp.schema.json had nudgeGrowthTokens:6000 in both the
field default and the compress default object. If any config layer
applies schema defaults, it would override resolveAdaptiveNudgeGrowth.
Removed from both locations — field has no default, description documents
adaptive behavior.

Review #2 🔴: formatBlockAge was duplicated in status.ts and nudge.ts.
Extracted to lib/ui/utils.ts as formatAge, imported in both modules.
ranxianglei added a commit that referenced this pull request Jul 5, 2026
…tool (#51)

* v1.8.0: Principle-driven prompts, hybrid Tips, compress improvements

* feat: nudgeGrowthTokens — token-based Tips gating (default 6K)

* refactor(nudge): extract computeShouldNudge for testability + review fixes

- Extract pure computeShouldNudge() helper (no state/side effects)
- Add 11 unit tests covering gating logic (floor, growth step, overMaxLimit bypass)
- overMaxLimit now bypasses frequency gating (urgent for small models)
- Fix dcp.schema.json missing trailing newline

Addresses PR #51 code review.

* feat(notify): show context before→after level in compress header

Replace misleading header (cumulative removed vs active summary — apples
vs oranges) with a clear context transition:

  ▣ ACP | Context 90%→50% (180K→100K of 200K)

tokensBefore captured in pipeline; tokensAfter = before - removed + summary.
Per-batch Compression line unchanged (already correct).

* fix(notify): drop context limit/percentage from header to avoid model anchoring

Showing total + % lets the model learn its ceiling and revert to waiting
until 80-90% before compressing. Now only absolute before→after is shown:

  ▣ ACP | Context 180K→100K

The model can still see it saved ~80K, but cannot anchor to a ceiling.

* feat(nudge): adaptive nudgeGrowthTokens — 5% of context limit, clamped [6K, 50K]

Default nudgeGrowthTokens was 6000 (fixed), which for a 1M-context model
is only 0.6% — Tips fired far too frequently, making the model feel
over-pressured to compress even at low context levels (e.g. 26%).

New default is adaptive: 5% of modelContextLimit, clamped to [6000, 50000]:
  - 10K-100K context → 6K (floor)
  - 128K → 6.4K, 200K → 10K, 500K → 25K, 1M → 50K
  - 2M+ → 50K (cap, prevents explosion for future large models)

Uses absolute tokens (not percentage) so the model sees a concrete growth
threshold. Users can still override via explicit nudgeGrowthTokens config.

* fix(inject): gate injectContextUsage by shouldNudge — stop every-message spam

injectContextUsage (buildContextUsageGuidance) was called unconditionally
every turn, bypassing the graduated nudge mechanism (nudgeGrowthTokens).
This meant the urgent 'Context: XK tokens. compress waste promptly.' text
appeared on EVERY message regardless of context level or growth.

Now it only fires when shouldNudge is true (same gate as Tips), so the
context usage text respects the adaptive nudgeGrowthTokens interval.

* fix(inject): gate ALL suffix content behind shouldNudge — zero compression noise between nudges

blockGuidance (Compressed blocks list) and visibleIdRange were still
injected unconditionally every turn, bypassing the graduated nudge
mechanism. Even without explicit 'compress now' text, the model saw
'Compressed blocks: 9 (...)' every turn — a constant indirect reminder
that compression is active, creating low-level pressure to compress.

Now ALL suffix injections (context usage, block list, Tips, visible ID
range) are gated by shouldNudge. Between nudges, the suffix is empty —
the model sees zero compression-related metadata. When shouldNudge fires
(every nudgeGrowthTokens), the full info appears in one consolidated burst.

Updated test to reflect gated behavior (added token info to trigger
shouldNudge=true).

* feat: add acp_status tool + simplify suffix to compact summary

Add acp_status tool (lib/compress/status.ts) for on-demand block details:
block IDs, summary sizes, ages, and topics in a formatted table.

Simplify the per-message suffix from detailed block list
('b1 (750t), b59 (750t), ...') to a compact one-liner:
'Compressed blocks: 9 (4.2K summary, last 5m ago). Use acp_status.'

Aging warnings and consolidation suggestions remain in the suffix
(triggered only when usage > 50% or > 50 blocks respectively).

* feat: gate system prompt behind nudgeGrowthTokens frequency

System prompt (SYSTEM constant + decompress extension + protected tools)
was injected EVERY request via experimental.chat.system.transform hook,
adding ~50 lines of compression philosophy per turn regardless of context
level. At 26% context the model still saw 'BE FRUGAL — compress waste
promptly' every turn, creating false urgency.

Now gated: injectCompressNudges stores shouldInjectThisTurn in state.nudges
after computing computeShouldNudge. The system prompt handler reads it on
the NEXT request (1-turn lag, since system hook runs before messages hook).

- shouldInjectThisTurn === undefined (first turn) → inject (initial awareness)
- shouldInjectThisTurn === true → inject
- shouldInjectThisTurn === false → skip entirely (model sees nothing)
- manualMode always injects (needs manual trigger instructions)

* fix: nudgeGrowthTokens dead code + duplicate context usage injection

Code review #8 fixes:

1. resolveAdaptiveNudgeGrowth was dead code — config default (6000) made
   nudgeGrowthTokens always defined, so the ?? fallback never triggered.
   Fix: make nudgeGrowthTokens optional in config type, remove hardcoded
   default from DEFAULTS, simplify merge to pass-through. Now undefined
   when user doesn't configure it → resolveAdaptiveNudgeGrowth runs.

2. applyAnchoredNudges duplicated context usage info — it appended
   buildContextUsageGuidance output to contextLimitNudge, while
   injectContextUsage also injects the same info. Removed the duplicate
   from applyAnchoredNudges; injectContextUsage is now the single source.

* fix: compress resetting lastNudgeTokens to 0 bypassed growth gate

After compress, lastPerMessageNudgeTokens was reset to 0, which caused
computeShouldNudge's lastNudgeTokens===0 sentinel to fire every turn
(bypassing nudgeGrowthTokens growth check). This created a feedback loop:
compress → Tip fires immediately → model compresses again → repeat.

Fix: set lastPerMessageNudgeTokens to post-compress currentTokens so the
growth counter restarts from the correct level. Also hardened getModelInfo
to handle messages without model info (optional chaining).

* test: add integration tests for post-compress nudge state transitions

Unit tests of computeShouldNudge (pure function) passed but missed the
real bug: injectCompressNudges set lastPerMessageNudgeTokens=0 after
compress, which re-triggered the 'first turn' sentinel every turn.

New integration tests verify the full state machine:
- Post-compress: lastPerMessageNudgeTokens = currentTokens (not 0)
- Post-compress small growth (<threshold): shouldInjectThisTurn = false
- Post-compress large growth (>threshold): shouldInjectThisTurn = true

* feat: add scripts/dev-deploy.sh — one-command build + deploy

Generic, portable dev-deploy script for local ACP development:
- Auto-detects node/npm (nvm/fnm/volta/manual installs)
- Builds (tsup bundle + tsc types) + deploys to opencode plugin cache
- --check flag runs tests before building
- --no-build deploys existing dist/ only
- No hardcoded user paths — uses $HOME and script-relative paths

AGENTS.md section 3.4 updated with correct cache path, verification
commands, and common-mistake warning.

Fixes the stale-dist bug: deploy-acp.sh (~/scripts/) only copied dist/
without building, leading to 4 commits missing from deployed bundle.

* fix: remove schema default 6000 + dedup formatAge

Review #1 🔴: dcp.schema.json had nudgeGrowthTokens:6000 in both the
field default and the compress default object. If any config layer
applies schema defaults, it would override resolveAdaptiveNudgeGrowth.
Removed from both locations — field has no default, description documents
adaptive behavior.

Review #2 🔴: formatBlockAge was duplicated in status.ts and nudge.ts.
Extracted to lib/ui/utils.ts as formatAge, imported in both modules.

* fix: review suggestions — sentinel semantics, idWidth, nullglob, acp_status tests

1. lastNudgeTokens===0 sentinel → undefined (types.ts, state.ts, utils.ts,
   persistence.ts, inject.ts, inject/utils.ts). Uses undefined as 'never
   nudged' sentinel instead of 0. Old persisted state with 0 loads safely
   (growth = currentTokens - 0 = huge → nudges once, then corrects).

2. shouldInjectThisTurn 1-turn lag comment in hooks.ts.

3. dev-deploy.sh: shopt -s nullglob for node PATH glob detection.

4. status.ts: idWidth computed from displayed blocks (not activeIds).

5. tests/acp-status.test.ts: 7 dedicated tests for acp_status tool.

* docs: update changelog + devlog for v1.8.1 (adaptive nudgeGrowthTokens)
ranxianglei added a commit that referenced this pull request Jul 15, 2026
- notification.ts formatEntryRanges: use effectiveMessageIds.length instead of startId/endId (4th leak site)
- system.ts: update stale 'message-ID ranges' description to 'message counts'
- recap.ts + status.ts: singular grammar (1 msgs → 1 msg, 1 messages → 1 message)

Addresses Oracle #1 (FAIL→expected PASS) and Oracle #2 (PASS) consensus findings.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
ranxianglei added a commit that referenced this pull request Jul 16, 2026
- notification.ts formatEntryRanges: use effectiveMessageIds.length instead of startId/endId (4th leak site)
- system.ts: update stale 'message-ID ranges' description to 'message counts'
- recap.ts + status.ts: singular grammar (1 msgs → 1 msg, 1 messages → 1 message)

Addresses Oracle #1 (FAIL→expected PASS) and Oracle #2 (PASS) consensus findings.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
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.

1 participant