Skip to content

feat(output-steering): turn classification + verbosity L0-L4 + clamp-only effort routing (#355) - #356

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-23_output-steering
Sep 23, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-23_output-steering

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Implements the kernel-owned output-side steering decision logic specified in #355 (owner-decided scope from billion-context#1093). The kernel decides; adapters land decisions onto wire fields — no protocol knowledge enters the kernel.

What's in it

New root module src/output-steering.ts (+ barrel exports, README/DESIGN docs):

  1. Turn classifier — classifyTurn(messages): TurnKind
    Pure structural read of the last user message's block composition (new_user_ask / mechanical_continuation / error_continuation / unknown); no content regex. The wire→structure normalization contract is documented in the module JSDoc; each adapter maps its protocol's tool-result shapes into StructuralBlock (bili keeps its per-wire classifiers as thin normalizers).
  2. Verbosity levels L0–L4 — VERBOSITY_LEVELS, verbosityDirective(level), renderSteeringBlock, applySteeringToPrompt
    Byte-stable directive texts (frozen baseline; tests assert the exact literals). L0 = no injection, default L2. Placement is replace-in-place or append-at-tail — never prepend (prefix-cache invariant); same-level re-application is a no-op (idempotence tested, incl. the compress-retry double-pass path).
  3. Clamp-only effort routing — EFFORT_LADDER, clampEffortToFloor(current, floor = "low"): EffortValue | null
    Never raises; returns null when the value is absent / unrecognized / at-or-below the floor, so adapters inject nothing (client-absent fields are never injected; the kernel does not know protocol field names by design).
  4. Entry point — decideOutputSteering(messages, config?): { turnKind, verbosityLevel, lowerEffort }
    Config { enabled, verbosityLevel, effortRouting }, default OFF; resolveOutputSteeringConfig tolerates malformed input with warnings. Per-provider route overrides remain an adapter concern.

Behavior deltas vs the bili reference (ranxianglei/billion-context#1096)

  1. Unrecognized effort strings are left untouched instead of being forced to "low". The reference forced any non-low string down to low, which can raise unknown sub-floor values — a latent clamp-only violation. Known-ladder behavior is identical; xhigh was added to the ladder (the reference hit it via the force path — same outcome).
  2. Numeric budget floors stay adapter-side (Anthropic thinking.budget_tokens → 1024, Google thinkingBudget → 128, dynamic -1 untouched): they are wire-field properties, not decision properties.
  3. Default sentinel <acp_output_steering> differs from bili's <bili_output_steering>. When migrating, bili must pass its own sentinel to renderSteeringBlock / applySteeringToPrompt, otherwise sessions pinned under bili's block would accumulate two steering blocks (cache bust). Documented in the module JSDoc.

Verification (rebased onto current origin/master, v0.0.82 base b30723f)

  • npm run typecheck — clean
  • npm test — 904 pass / 0 fail, including the new 429-line regression suite that ports all applicable cases from the bili reference (byte-exact L1–L4 text anchors, idempotence/retry paths, clamp matrix, config resolution)
  • npm run build — green
  • New source files are prettier-clean (pre-existing repo-wide drift left untouched; CI has no format gate)

Follow-up after merge + release

Per #355: confirm npm view acp-kernel version shows the new release → billion-context bumps the exact pin → release → replace the duplicated decision logic in PR#1096 with kernel calls (wire-side landing + its own sentinel stay in bili).

Closes #355

…lassification, verbosity L0-L4, clamp-only effort routing (#355)
@github-actions

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-23_output-steering (7d9487e)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-356

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr356.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] 🏷 Reviewing PR #356 now: diff cleanliness check, full pre-flight (typecheck / test / build), and a regression review of existing behaviors touched by the diff. Findings will follow as a comment on this thread; small nits will be fixed directly on the branch.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review of PR #356 complete — findings below.

Diff cleanliness (first gate): PASS

  • Single commit 7d9487e on current origin/master (b30723f, v0.0.82) — no stale base.
  • 5 files, +778/−0, purely additive: src/output-steering.ts (new, 320 ln), tests/output-steering.test.ts (new, 429 ln), barrel exports in src/index.ts, one doc line each in README/DESIGN. No unrelated changes, no whitespace churn, no lockfile/generated churn, no version bump. New files are prettier-clean. Zero-runtime-deps preserved (the module has zero imports).

Pre-flight at PR head: all green

  • Local: npm run typecheck clean · npm test 904/904 pass · npm run build green.
  • CI: test (22), test (24), pr-validation, build-artifact — all success.

Spec conformance (#355) — verified against the spec AND the bili reference (PR#1096)

  • classifyTurn: pure structural, no content regex, conservative unknown; rules match the spec and bili's four per-wire classifiers, incl. the Responses sets (outputs: function/custom_tool/local_shell/apply_patch *_call_output; neutrals: message/function_call/custom_tool_call/local_shell_call/apply_patch_call/reasoning; user-signal definition).
  • Verbosity L1–L4 texts: byte-identical to the bili reference (lengths 123/262/696/392) — frozen-baseline requirement met; sessions pinned under these texts stay cache-stable through migration. L0 = no injection, default L2.
  • Placement: tail-only, never prepends; same-level re-apply is a byte-stable no-op (incl. compress-retry double-pass); level change replaces in place without duplication; malformed unclosed sentinel consumed, not duplicated — all tested.
  • Clamp-only routing: never raises; absent/unrecognized → null (adapters inject nothing); ladder minimal<low<medium<high<xhigh; fires iff mechanical_continuation.
  • Config: default OFF; strict enabled === true; effortRouting on-by-default-when-enabled; malformed verbosityLevel falls back with warning while omitted takes the default silently; non-object → pure defaults, no warnings.

Disclosed deltas vs the bili reference — all three verified accurate

  1. Unrecognized effort strings left untouched: confirmed bili forces any non-low/non-minimal string (OpenAI/Responses) or any non-low string (Anthropic) to "low" — coercing an unknown sub-floor value would be a raise, violating clamp-only. Kernel's leave-alone is the stricter reading; known-ladder outcomes identical (incl. xhigh→low).
  2. Numeric budget floors stay adapter-side: confirmed present in bili's lowerEffort (Anthropic 1024, Google 128, dynamic -1 untouched) and correctly absent from the kernel.
  3. Default sentinel differs: kernel default is `` (verified byte-wise) vs bili's own sentinel; the "pass your own sentinel on migration" note is in the module JSDoc.

Regression review — existing behaviors touched by this diff

Touched path Carried behavior Verdict
Barrel src/index.ts existing public API surface Preserved — purely additive exports, no name collisions (typecheck clean), no signature changes
processTurn pipeline, nudges, wire codec, prompts, block model, ref system §7.3 kernel contracts Untouched — new module is standalone, zero imports; no node ordering, config-schema, or state-shape changes
Existing defaults / output format shipped behavior Preserved — feature is default-OFF; nothing changes until a host opts in

Verdict: no regressions; this is a purely additive feature.

Minor find (non-blocking, reported in-thread per policy)

README's Status line still says "23 source modules" while master already carried 38 root-level src/*.ts files before this PR (39 after) — pre-existing doc drift, not introduced here; left untouched to keep the diff additive. Say the word if you want it fixed separately.

Merge gating

This is a feat: adding new public API surface → human-gated per AGENTS.md §7.4 (no auto-merge). And agents can't merge regardless — please merge yourself: #356


中文摘要:审查了 PR #356(输出侧 steering 决策逻辑入内核)——diff 纯增量、本地与 CI 全绿、对 bili 参考实现的三处行为差异均属实且更严格(clamp-only 无升档风险)、文案字节级一致、不触碰任何既有行为与内核契约,可以合并(feat 类按规则需人工合并)。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Addendum (second minor find, non-blocking): running npm install on any branch locally modifies package-lock.json by 2 lines — its "version" field still says 0.0.77 while package.json is at 0.0.82. This is pre-existing on master (release commits touch only package.json per the release workflow), so every local install shows that 2-line diff until reverted; it's expected churn, safe to discard, and I left it out of the review scope accordingly. Worth a one-line lockfile sync at some future release if you want to stop the recurring dirt.

@ranxianglei
ranxianglei merged commit 52fe4f5 into master Sep 23, 2026
4 checks passed
@ranxianglei ranxianglei mentioned this pull request Sep 23, 2026
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.

[提案] 输出侧决策逻辑入内核:轮次结构分类 + verbosity 档位(L0-L4) + effort 路由(billion-context 做 wire 落位)

1 participant