Skip to content

fix(nudge): gate count-triggered tier nudges on the min usage band - #239

Open
ranxianglei wants to merge 1 commit into
masterfrom
2026-09-09_tier-nudge-usage-gate
Open

fix(nudge): gate count-triggered tier nudges on the min usage band#239
ranxianglei wants to merge 1 commit into
masterfrom
2026-09-09_tier-nudge-usage-gate

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem (#237)

The growthReady branch in decideNudge (src/compress.ts) has two paths for T2/T3 override:

  • token masst2Pen >= 1.5x nudgeGrowthTokens (requires a real mass)
  • block countt2Count >= tier2Trigger / t3Count >= tier3Trigger (fires at ANY usage)

In production (session 01a07b3c, 2026-09-09 14:41:34), a T2-distill nudge injected at 43% usage with only 5872 pending tokens — right after a successful T2→T3 distillation had just brought usage down. The count path has no usage gate, so any session that accumulates tier2Trigger T1 blocks gets a distill nudge on tiny summary mass, burning a model turn + cache for negligible reclaim. With a repetition-prone model this amplified into the perceived injection storm (consumed-span rejections, below-min ranges).

Fix

  • Gate the COUNT paths on usage >= config.nudge.minContextLimitPct — the same band the first-sight mass bypass (growth floor 挡住新会话/重启继承的大量就绪 pendingT1 —— 首次 nudge 被无谓推迟 #194) already uses. Below the band there is no pressure, so distillation is not needed yet; blocks will distill once usage enters the band (or immediately via the pressure band at ≥75%).
  • The token-mass paths stay ungated: they already require a real 1.5x mass, so a below-band mass nudge reclaims something meaningful.
  • The idle reason now labels count-ready-but-gated tiers as T2 N blocks (count, usage-gated) so acp_status explains why nothing injected.
  • The injected-reason label now reflects the path that actually passed: below the band, a mass-ready tier is labeled (1.5x), not >= tier2Trigger (previously mislabeled as count even when only the mass path could fire).

Tests (4 new, tests/nudge.test.ts)

  • count-triggered T2 stays silent below the band (43%), reason shows (count, usage-gated)
  • count-triggered T3 stays silent below the band (43%)
  • count trigger fires AT the band boundary (exactly 45%) — pins >= semantics
  • T2 token-mass path still fires below the band (scope check: only the count path is gated)

Existing count-trigger tests (60% usage) are unaffected. Full suite: 584 pass, 0 fail. Typecheck + build clean.

The growthReady branch's COUNT paths (t2Count >= tier2Trigger /
t3Count >= tier3Trigger) fired at ANY usage once the block count was
reached — in production a T2-distill nudge injected at 43% usage with
only 5872 pending tokens (#237), burning a model turn + cache for
negligible reclaim.

Gate the count paths on usage >= nudge.minContextLimitPct, the same
band the first-sight mass bypass (#194) already uses. The token-mass
paths stay ungated: they already require a real 1.5x mass. The idle
reason now labels count-ready-but-gated tiers as
'T2 N blocks (count, usage-gated)' so acp_status explains why
nothing injected.

Fixes #237
@ranxianglei
ranxianglei force-pushed the 2026-09-09_tier-nudge-usage-gate branch from 04d7101 to 44c96a4 Compare September 9, 2026 15:13
@ranxianglei ranxianglei added the duplicate This issue or pull request already exists label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-09-09_tier-nudge-usage-gate (44c96a4)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-239

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr239.tgz
npm install ./package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷

Duplicate screening verdict: the fix in this PR is already on master via the merged #238 — but the PR carries 4 regression tests that are a genuine, mergeable delta. Labeled duplicate, kept open, and rebased so it's now a clean test-only PR.

Triage (what I verified)

1. The bug (#237) is real and the fix is already shipped. The count path (t2Count >= tier2Trigger / t3Count >= tier3Trigger) had no usage gate, so it fired at any usage once enough blocks accumulated — the 43%-usage / 5.8K-token production repro. Your #238 already fixed exactly this and is merged (master 71f31ef). I confirmed master's decideNudge carries t2CountReady = t2Count >= tier2Trigger && usage >= nudge.minContextLimitPct (and the T3 analog), with the mass paths left ungated.

2. Layer check — the fix hits the root cause, not the symptom. The "injection storm" (consumed-span rejections, below-min ranges) was a symptom amplified by a repetition-prone model flailing against the tier3-rewrite guard; the root cause is the count path firing below the usage band. Gating the count path on minContextLimitPct (while leaving the 1.5x mass paths ungated) addresses the root cause directly. That approach is sound — and it's the same one #238 already landed.

3. So this PR's fix is redundant with #238. I proved it by rebasing the PR head onto current master: after resolving the 3 conflicts, src/compress.ts is byte-identical to master. The conflicts were (a) two pure line-wrap formatting diffs on the injectedReason ternary, and (b) one real nuance in the idle-reason readyCount builder — where #238's (master's) version is actually more correct: it filters with countReadyUngated so a below-band count-ready tier is still listed as (count, usage-gated), which is precisely what this PR's own tests assert. This PR's version would have referenced the now-gated countReady and dropped the tier from the hint, contradicting its own test. I resolved all three in master's favor.

4. The real value here is 4 regression tests (2 beyond #238's two):

  • count trigger fires AT the min usage band boundary (exactly 45%) — pins the >= semantics at the band edge.
  • T2 token-mass path still fires below the min usage band (43% + 1.5x mass) — scope check that only the count path is gated, mass stays ungated.
  • plus two "stays silent below the band" cases at 43% (the production-repro usage), alongside fix: usage-gate count-triggered tier nudges #238's 40% versions.

What I did

  • Rebased the PR onto current master (71f31ef) and force-pushed the branch. It's now a clean, mergeable, test-only PR: 1 commit, 1 changed file (tests/nudge.test.ts), mergeable_state: clean.
  • Verified on the rebased head: full suite 635 pass / 0 fail, tsc --noEmit clean. (The PR body's "584 pass" was correct for its then-head; master has simply accumulated more tests since.)
  • Added the duplicate label (fix overlaps fix: usage-gate count-triggered tier nudges #238) and left it open, since the tests are a worthwhile delta.

Recommendation

The fix needs nothing — it's already live from #238. For the test coverage, my recommendation is yes, keep it: the boundary >= pin and the mass-scope check are cheap insurance against exactly the #237 class of regression. As it stands the PR only adds tests, so you can merge it as-is. If you'd rather not, close it and I'll fold just the two new tests (boundary + mass-scope) into a standalone test PR.

(Merges are human-only, so I'll leave the merge to you either way.)

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

Labels

duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant