Non-orch surface: thrash pause, resume, feedback, telemetry names - #443
Merged
Conversation
…progress signal A Grok session hard-paused at 10 turns while making real progress through Linear lookups and code reads, because the pause fired on any tool-only turn count rather than actual thrash. Forensics over ~/.corbits/projects session traces (54 sessions with tool-only runs) found healthy streaks topping out at 13 turns and zero sessions repeating an identical tool-call fingerprint 3+ times in a row. The soft wrap-up nudge now fires at a shared 25-turn threshold for every model family (still just a check-in, never a stop). The hard pause now requires the tool calls to actually repeat identically 4 turns in a row (fingerprintToolCalls, the same helper SubAgentDirector already uses), independent of overall streak length. Grok drops its miscalibrated 6/10 tool-only pair and shares the default; its shorter sub-agent stall timeout and finish-bias residual are untouched.
…shared period-detection helper Lifted the shortest-period-that-repeats-enough search out of detectRepetition into src/util/period-detection.ts so tool-call fingerprints can reuse the same detection shape instead of a hand-rolled consecutive-identical check. stall-watchdog's detectRepetition now delegates to it; behavior is unchanged, covered by its existing test suite.
The old identicalToolFingerprintStreak only compared each turn to the one immediately before it, so an alternating A,B tool-call pattern never triggered the hard pause at any length (critique proved this over 200 turns), while 4 truly identical calls in a row still false-positived on legitimate polling (rerunning a flaky test, checking a build). detectToolFingerprintThrash runs exact-period detection over a rolling fingerprint history instead, catching A,A,A..., A,B,A,B..., and A,B,C,A,B,C... uniformly. Identical-consecutive (period 1) needs 5 repeats to tolerate legitimate short polling; any longer cycle needs only 3, since there's no legitimate reason to repeat a fixed rotation of different tool calls. Added scripts/tool-fingerprint-forensics.ts to re-derive these thresholds against real local session traces: 328 sessions / 559 tool-only runs show zero repeating cycles of any period 1-8 at all, so both floors sit well above the measured healthy ceiling. The period-1 floor of 5 is inferred headroom for the polling case (not measured — the dataset has no repeats to calibrate against), chosen only to clear the previously false-positived value of 4.
ChatDirector now keeps a capped rolling history of tool-only-turn fingerprints and pauses on detectToolFingerprintThrash instead of a hand-rolled last-fingerprint comparison, so it catches alternating and rotating tool-call cycles the old check missed entirely, without false-positiving on a handful of identical polling calls. Removed toolOnlyNoProgressRepeatLimit from ModelFamilyPolicy — the thrash check is no longer a single tunable number, and isn't family-specific. Updated the stale applyToolOnlyLoopProtection JSDoc, which claimed the pause only fires after the nudge — no longer true, since the thrash check can (and often does) fire well before the nudge threshold. Updated docs/ARCHITECTURE.md's director-policy section to describe period detection accurately, with file:line references. Tests: alternating A,B for 200 turns now pauses (critique's exact repro), a 3-cycle A,B,C pauses, 4 identical polls followed by varied work does not pause, a long varied productive streak never pauses, and the nudge path still does not reply-pause.
Period detection has a hard ceiling (max scanned period 8) and only fires on
an exact repeating tail, so a rotation longer than the ceiling, or a
"phase-broken" cycle that inserts a varying element between repeats (e.g.
A,B,A,B,UNIQUE,...), escapes it forever regardless of streak length.
detectRawToolOnlyBackstop is a secondary, pattern-free check on the raw
tool-only streak length, wired into the ChatDirector so it only fires once
period detection has not already caught the turn. It uses its own pause
message ("ran N tool-only turns without narrating progress") rather than the
pattern-detection wording, since no pattern was found. Threshold is 60,
derived from the current forensic scan (328 sessions with a tool-only run,
559 tool-only runs): run-length p50 3, p90 8, p99 16, max 28 — 60 is more
than double the longest healthy streak ever observed and stays well clear of
the old hard-pause-at-10 that originally motivated this rework.
Also documents on TOOL_FINGERPRINT_MAX_PERIOD that it is a ceiling with no
forensic backing above period 6 (the scan's actual range), and that the
backstop is what catches anything above it.
An earlier commit on this branch claimed the forensic scan (scripts/tool-fingerprint-forensics.ts) covered periods 1-8 across 328 sessions; the script only ever scanned periods 1-6 (MAX_PERIOD_SCANNED). That inaccurate claim was repeated in docs/ARCHITECTURE.md and model-family-policy.ts (stop-policy.ts's copy was fixed in the previous commit alongside the ceiling comment it lives next to). All three now say periods 1-6, and model-family-policy.ts's healthy-streak figures are updated to the run this scan currently produces (p50 3, p90 8, p99 16, max 28) rather than the older "13-28" summary. Also drops product-name attribution from a comment that no longer needs it. docs/ARCHITECTURE.md's director-policy section now also describes the raw-count backstop added in the previous commit: period detection as the fast path, the backstop as the final net for cycles above the period ceiling or phase-broken patterns, with file references for both.
Critique found the round-3 backstop's own escape: narrated text reset both
the period-detection history AND the raw backstop counter, so a model that
narrated one word every ~55 turns kept resetting the backstop before it
could fire. Period detection ("is the model cycling?") still clears on
narration. The backstop is now a separate counter, turnsSinceUserMessage,
that only clears on a genuine fresh user message.
Since narration no longer buys back backstop budget, a legitimately long
autonomous run will now reach it. Reaching the backstop no longer pauses
outright — it nudges for a progress summary. Only if that nudge goes
unanswered for a further full backstop interval, with still no user message
and no thrash detected, does the session hard-pause. A genuine cycle (period
detection) still pauses immediately regardless.
Re-derived the threshold from a fresh local scan of
turns-since-last-genuine-user-message (filtering tool-result echoes, which
are also role "user" in the transcript format): p50 5, p90 14, p99 29, max
32 across 428 runs. Set to 100, roughly 3x the measured max.
… apart from synthetic sends Round 4 reset the turns-since-user-message backstop on any message.received event, which synthetic system sends (compaction continuations from tui/runner.ts, exec/runner.ts, subagent/run.ts) also fire without being operator input — and compaction fires more often during long tool-only loops, exactly when the backstop should be counting. Adds OPERATOR_ORIGINATED_FLAG, set only where a human actually submits a prompt (TUI prompt-submit and the "send" command result, exec's initial task). The backstop now resets only when that flag is present, so a future synthetic sender has to explicitly opt in rather than silently qualifying by omission.
The commit message, stop-policy.ts, and ARCHITECTURE.md cited a 358-session/428-run scan of turns-since-last-genuine-operator-message with a stated methodology; no corresponding script or output exists anywhere in the tree, and the two numbers already disagreed with each other. That measurement was never taken. Rewrites all three to state plainly that 100 is a judgment call, not a measured value, informed only by the streak-length data we do have (tool-fingerprint-forensics.ts: p50 3, p90 8, p99 16, max 28 across 328 sessions) even though that measures a different quantity than this counter. Also fixes the stale backstopNudgeFiredAtTurn comment, which claimed a reset on thrash/escalation that does not happen in code.
Bring the closed PR #401 work onto current main: soft nudge ~25, hard pause only on fingerprint thrash / delayed backstop, not raw tool-only count. Grok no longer kills productive multi-step tool work at 10. # Conflicts: # docs/ARCHITECTURE.md # src/agent/director.test.ts # tests/unit/director.test.ts
Operators can send free-text product feedback with /feedback even when ambient telemetry is off. Bare /feedback arms the next non-command line; inline text sends immediately. Env kill switches still block send, and captureIntentional only accepts survey responses so ambient events cannot ride the bypass.
Operators can reopen the latest session, a specific UUID session, or open the interactive picker. Invalid ids and id+--pick combinations error instead of silently falling through; legacy session trees migrate on resume by id.
Heading-only frames could pass the prior no-marker check while the bold body line had not painted yet, causing intermittent CL-5715 flakes.
Ship blockers from the OSS panel: --help exits 0 via CliHelpError, feedback says queued not sent with truncation notice, hide /feedback from the slash menu until survey env ids are set, and drop a bare feedback arm when another slash command runs.
Operators never configure survey routing; same public-id class as the baked PostHog project key. Env overrides remain for tests and forks.
Classify slash commands and multi-turn /feedback before the session bridge marks busy or enqueues, so the prompt never shows Working… for a PostHog survey capture. Flush feedback on capture, keep settings ambient-only, and sync CLI help docs with the honest wording.
The busy-path fix was dropped at mountRunnerHost, so typecheck failed on classifySubmit. Empty Enter also never reached exclusive hooks, so armed /feedback cancel was dead code until the shell lets blank lines through.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wave of non-orchestration surface work on Corbits Code:
corbits resume/continuefor project-keyed sessions (last/<id>/--pick)/feedbackas headless PostHog survey capture (works when ambient telemetry is off; env kill switches still win)CliHelpErrorexits 0, feedback honesty (“queued”), hide/feedbackuntil survey env ids set, drop bare feedback arm on other slash commandsTest plan
bun run test— 4443 pass / 0 fail locallycorbits --helpprints to stdout, exit 0corbits resume/resume --pick/resume <id>against a known project/feedbackwithout survey env → hidden / “not configured”CORBITS_FEEDBACK_SURVEY_ID+CORBITS_FEEDBACK_QUESTION_IDset,/feedback helloqueues survey response; bare/feedbackthen Enter cancels; other slash drops armOps note (not a merge blocker)
/feedbackneeds a PostHog free-text survey created once, then:Without those, the client fails closed and hides the command from the slash menu. No survey id is baked into the binary.
Tickets
CL-5611, CL-5749, CL-5744, CL-5532, CL-5722, CL-5715