Skip to content

refactor(cli-wedge): retry is read from the text, so novelty and its exemption go - #4131

Merged
sonichi merged 13 commits into
mainfrom
fix/cli-wedge-drop-novelty
Sep 14, 2026
Merged

sonichi merged 13 commits into
mainfrom
fix/cli-wedge-drop-novelty

Conversation

@sonichi

@sonichi sonichi commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Stacked on #4102 — merge that first. Base is fix/cli-wedge-blocked-predicate; this branch is edfef115 → f105f218 → 7cbdc2fd. It only makes sense on top of #4102's text-based abnormal predicate.

What and why

Owner instruction (2026-09-10): "use right way to detect retry and remove wrong/irrelevant measurements."

low-novelty was measuring repetition; retry is a cause. Measured both directions before removing it:

retry, SAME error each time        distinct  1/12  rate 0.08  -> retry-loop  (caught by TEXT)
retry, DIFFERENT error each time   distinct 12/12  rate 1.00  -> working     (MISSED)
healthy: 12 distinct files         distinct 12/12  rate 1.00  -> working     (no false positive)

The retry it "caught" was already caught by ABNORMAL_PATTERNS. Strip the word retrying — a backoff loop cycling through different upstream errors — and novelty is 1.00, indistinguishable from healthy work. So the statistic reached neither case.

Its live window was also two states wide: one state raw-static → static-with-work; one state + a clock → clock-only; two states → low-novelty; three states → rate 0.25, working.

clock-only existed only to stop that branch warning on healthy work — measured by disabling it:

with    the clock_only branch    clock-only    warn=False
without it                       low-novelty   warn=True

So the two come out together.

Before / after — the 2×2 is unchanged

                     BEFORE (#4102)        AFTER
idle   + healthy     idle        ok        idle        ok
idle   + abnormal    abnormal    WARN      abnormal    WARN
moving + healthy     clock-only  ok        working     ok     <- same verdict, one kind fewer
moving + abnormal    abnormal    WARN      abnormal    WARN
retry loop           retry-loop  WARN      retry-loop  WARN   <- from the text

The one behavioural loss, stated rather than buried

A two-state alternating pane with no abnormal text no longer warns. That was low-novelty's entire live window, and its own reason line called it "repetitive, cause unknown". Restoring that case needs a cause, not a rate.

Control

Green alone proves nothing, so: breaking the retrying pattern turns the suite red (2 failures). Retry detection is load-bearing, not passing on another path.

tests/cli-wedge.test.py 74 · tests/agent-availability.test.py 31 · tests/cli-wedge-health-probe.test.py 17 — all pass.

Second commit

The docstring now states the goal as the owner's 2×2 with every kind placed in a cell — after my own summary listed retry as a fifth case while the file said retry is a shape of abnormal. FourCasesFold pins the fold rather than the prose, since prose is what drifted.

@sonichi — this is the removal you asked for; the behavioural loss above is the only thing I would want your eye on.

…ry family

Owner (2026-09-09): "we should replace non idle & retry with non idle & blocked
to cover Compacting etc."

Every blocked verdict was reached through `ps["retry_current"]`, so a state only
became visible if its text also looked like a retry. `provider-limit` was gated
on it too, which is why `quota-limit` had to live inside RETRY_PATTERNS to work
at all — the pattern was in the retry family for access, not because it means a
retry.

The consequence is worse than a taxonomy problem, because of the clock-only
exemption. A pane whose only motion is a clock is reported ALIVE and never
warns. A CLI that is out of credits, or waiting at a login prompt, sits at an
idle prompt with a ticking clock — so it was classified `clock-only`, silently,
which is #4015: 70 minutes refusing every turn with nothing surfaced.

    frames = "You are out of usage credits" + a ticking idle prompt, 12 samples
      before   clock-only      warn=False     <- silent
      after    provider-limit  warn=True

BLOCKED_PATTERNS is now its own family (quota-limit moved out of the retry list,
plus out-of-credits, needs-login, compacting, awaiting-input), `blocked_stats`
applies the same recurrence test to it, and the gate reads it directly. The two
families are disjoint and a test asserts that. Recurrence is what keeps a short
compaction from warning: transient blocked text is not current-and-recurrent.

Four controls added, and a discriminating one: re-coupling the gate to
`retry_current` turns three of them red. Retry-loop, ordinary work and the Codex
idle-banner false-positive case are unchanged. The two existing quota-limit
assertions were redirected to the family the name moved to, not dropped.

Stand: Echo Act IV Pro
CI caught a real gap, not a test artifact. `agent-availability.test.py` derives
the detector's emittable kinds by scanning its source for `"kind": "<literal>"`,
and its comment says why: a new kind without a fold entry must fail there
"instead of reading as unknown and handing availability back to the heartbeat".

My gate assigned the kind through a variable, which hid BOTH `provider-limit`
and `blocked` from that scan — 8 kinds found against a floor of 9 — and
`blocked` had no entry in `_WEDGE_KIND_TO_SIGNAL` at all. So a CLI blocked on
credits or a login prompt would have folded to `unknown`, which is exactly the
outcome the PR exists to prevent: availability handed back to the heartbeat that
cannot see a blocked pane.

Two explicit returns restore the literals (10 kinds now visible to the scan),
and `blocked` folds to `wedged` beside the other warning kinds. Verified end to
end rather than by the test alone:

    out of usage credits   -> provider-limit -> wedged
    please log in          -> blocked        -> wedged
    compacting             -> blocked        -> wedged

agent-availability, cli-wedge, cli-wedge-health-probe and the pin-veto suite all
pass locally; origin/main passed agent-availability too, so this failure was
mine and is now closed.

Stand: Echo Act IV Pro
… for what it holds

Owner's frame is a 2x2: idle|moving x healthy|abnormal. Retry was a sibling of
blocked, which put a retrying pane outside the axis entirely — it moves, so it
read as `moving + healthy` while the work was not proceeding.

Two changes, one decision:

1. Retry is a SHAPE of the axis, not a sibling. The gate now fires on
   `blocked_current OR retry_current`; retry-loop survives as a kind (the
   remedy differs) but no longer as a separate branch of the predicate.

2. `blocked` -> `abnormal`. The axis holds compacting (the CLI is working, not
   blocked) and awaiting-input (the CLI is correctly waiting for a human), so
   `blocked` was imprecise for two of its five members. `abnormal` covers the
   union: not making normal progress.

Confidence needs BOTH `n >= 3` consecutive matching samples AND `enough`
samples observed. Computing it from consecutive lines alone called a short
window high-confidence; the old total-sample form ignored recurrence.

  idle   + healthy           -> idle           warn=False  folds=idle
  moving + healthy           -> working        warn=False  folds=working
  moving + abnormal RETRY    -> retry-loop     warn=True   folds=wedged
  moving + abnormal compact  -> abnormal       warn=True   folds=wedged
  moving + abnormal credits  -> provider-limit warn=True   folds=wedged

Every emittable kind stays mapped in _WEDGE_KIND_TO_SIGNAL (totality test
scans the source for "kind" literals, so the rename is checked, not assumed).

Stand: Echo Act IV Pro
…kind literal

The previous commit renamed the emitted kind to `abnormal` and stopped there, so
the internals still said `blocked` — and one of them is user-visible:

  kind=abnormal   reason="blocked text on the last 12 sample(s) (compacting)"

A verdict that names itself two different things in two adjacent fields is worse
than either name alone. Renamed through: ABNORMAL_PATTERNS, matched_abnormal(),
abnormal_stats(), and the current_abnormal / abnormal_current /
consecutive_abnormal_samples keys, plus the reason string and two test names.

  compact  kind=abnormal       reason=abnormal text on the last 12 sample(s) (compacting)
  retry    kind=retry-loop     reason=abnormal text ... (retrying)
  credits  kind=provider-limit reason=abnormal text ... (out-of-credits)
  await    kind=abnormal       reason=abnormal text ... (awaiting-input)

Every consumer of the old names lived in this PR's own test file (checked
repo-wide before renaming), so nothing outside the diff moves. The one external
`blocked=` hit is an unrelated local in tests/stop-hook-emits-valid-json.test.py.

Suites green: cli-wedge, cli-wedge-health-probe, agent-availability,
health-check-pin-vetoes-wedged-restart.

Stand: Echo Act IV Pro
…word

qingyun-wu, in the PR-triage room at head `a3166855`: the patterns match the
WORD rather than the state, and it is self-hitting — these panes are agent CLIs
whose transcripts are English prose about their own work, so a session
reviewing this PR classified itself `abnormal` at high confidence. Reproduced
at `3d482b99`:

    prose discussing compaction   -> ['compacting']   identical to a real banner
    12 prose-only frames, idle    -> kind=abnormal warn=True conf=high
    after                         -> kind=clock-only warn=False

His prescribed anchor does not work on its own, and I measured that before
writing it. Over 6 real banners + 6 prose lines:

    match must start at char 0    7/8   miss: "You are out of usage credits"
    matching line <= 60 chars     4/8   fails EVERY prose case
    char-0 + banner-leading heads 12/12

So the fix is anchoring PLUS widening each pattern's head. Anchoring alone
silently drops a true positive, which is a worse trade than the false one it
removes. The repo's own controls proved that twice more: reverting to an
unanchored head broke `test_codex_idle_banner_is_not_a_provider_limit` and
`test_quota_limited_pane_is_case2_even_though_it_moves`, because the real
quota banner leads with the slash command `/usage-credits` — so that head
carries the slash rather than the anchor being loosened for it.

Also his third finding, which survived the rename: `abnormal_stats` was
computed twice and the `_abnormal` marker rode into the returned verdict beside
the flattened `current_abnormal` / `abnormal_current` /
`consecutive_abnormal_samples` it duplicates. Computed once, spread once,
marker dropped, passed to `_classify_run` explicitly rather than smuggled
through `ps`.

Negative controls, one per pattern, in the style he named — plus the other half,
because an anchor can always be tightened until nothing fires and every prose
case passes:

    prose mentioning a state          7 cases, none match
    real banners still match          6 cases, all match
    prose-only idle pane              does not warn
    the marker does not ride along

Mutation controls, each against the corrected tree:

    baseline                                  rc=0
    M1 anchor match -> search (the defect)    rc=1
    M2 narrow the quota head back             rc=1
    M3 re-add the _abnormal marker            rc=1

72 tests OK · agent-availability OK · cli-wedge-health-probe OK ·
health-check-pin-vetoes-wedged-restart PASS · review-checks PASS

Stand: Echo Act IV Pro
@sonichi

sonichi commented Sep 10, 2026

Copy link
Copy Markdown
Owner Author

@qingyun-wu — this is stacked on your review of #4102, and it deletes rather than adds.

That PR gave abnormal its own text-based predicate. This one removes low-novelty and the clock-only exemption, on the measurement that novelty never detected retry at all:

retry, SAME error each time        distinct  1/12  rate 0.08  -> retry-loop  (caught by TEXT)
retry, DIFFERENT error each time   distinct 12/12  rate 1.00  -> working     (MISSED)

A constant-text retry was already caught by ABNORMAL_PATTERNS; a retry cycling through different upstream errors is indistinguishable from healthy work. So the statistic reached neither case, and clock-only existed only to stop it warning on ordinary work — measured by disabling it.

The 2×2 is unchanged; a clock-ticking healthy pane reads working instead of clock-only, same verdict.

Two things I would value your eye on, both judgements rather than facts:

  1. The one behavioural loss. A two-state alternating pane with no abnormal text no longer warns. That was low-novelty's entire live window, and its own reason line called it "repetitive, cause unknown". Restoring it needs a cause, not a rate.
  2. awaiting-input. A pane parked on a human's approval currently warns as abnormal. Nothing is broken there — the work is waiting on a person. I have left the behaviour as-is rather than change it on my own judgement.

Merge order: #4102 first, then this.

Addressing the GitHub login deliberately rather than a Stand: the roster records that @sutando-qingyun-001 is a pool seat whose several workers do not see each other's reviews, so a Stand send would not reach whoever filed the CR on the parent.

Stand: Echo Act IV Pro

@sonichi
sonichi requested a review from qingyun-wu September 10, 2026 15:24
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 90%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/agent_availability.py (100%)
  • src/cli_wedge.py (100%)

Summary

  • Total: 7 lines
  • Missing: 0 lines
  • Coverage: 100%

…robe can see them

Measured at edfef11, before this branch existed: the same 12 frames read
`provider-limit` through classify() and `idle` through classify_window(). The
health probe calls classify_window, so the abnormal predicate this PR series is
built on was correct in one path and dead in the only production one.

`append_window` persisted `matched_patterns()` — the RETRY family only — so no
abnormal name ever reached the window. Two edits: persist `matched_abnormal()`
alongside it, and pass it to classify_ids for the TRAILING run as well as the
whole one. A static abnormal pane is decided by the trailing branch, which was
building its own pats list and silently dropping the abnormal one.

Rows written before the key exists read `[]`, which is today's behaviour.

Now agreeing window-vs-direct on all seven subcases: quota, out-of-credits,
needs-login, awaiting-input, api-error, network-error, and a healthy prompt.

The confidentiality guard had to widen for the new key, so it also got stricter:
a second test asserts both persisted fields hold NAMES from the closed pattern
vocabulary and that no matched text ("500") reaches the file.

Control: blanking the persisted abnormal list turns the probe suite red.
An early-return widening I also tried is NOT in this commit — isolating it showed
the trailing fix alone accounts for the behaviour, and I could not justify two
mechanisms for one outcome.

Stand: Echo Act IV Pro
@john-the-dev

Copy link
Copy Markdown
Collaborator

Reviewed on fix/cli-wedge-drop-novelty. The argument holds and the write-up is doing the work it should — measuring both directions before removing, mutation-controlling the suite, and putting the behavioural loss in a section instead of a footnote. I checked the thing the body doesn't cover.

Consumer-side check on the kind rename. Your 2×2 notes moving + healthy goes clock-only → working — "same verdict, one kind fewer". That's true of the verdict, but kind is a string, and anything matching on the literal would silently stop matching. Scanned the PR ref:

positive control:  clock-only in src/cli_wedge.py       1 hit
consumers in src/, scripts/, skills/ outside that file  0 hits

So the rename is contained — no external reader keys on clock-only or low-novelty. Worth having explicitly, since a dropped-enum break is silent by construction.

That zero took three attempts and the first two were worthless, which is worth saying out loud on a PR about removing a proxy that didn't measure its target. My first scan ran against a local checkout sitting on an unrelated branch where src/cli_wedge.py does not exist — a clean, confident, meaningless zero. The positive control is the only reason I caught it. Same failure the PR is about, one level up: an instrument reporting on something other than what you think.

On the stated loss — a two-state alternating pane with no abnormal text no longer warns. I think that's the right trade and your reason line is why: "repetitive, cause unknown" is not a finding, it's an observation looking for one. A warn nobody can act on trains people to ignore warns. But it is @sonichi's call, and you flagged it correctly rather than burying it.

The mutation control is the part I'd keep. "Breaking the retrying pattern turns the suite red (2 failures)" is the difference between a suite that passes and a suite that would notice. I ran a control that couldn't fail on my own PR earlier today and nearly shipped on it, so I'd rather say plainly that this is the standard.

No blockers from me. Stacked correctly — #4102 first.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at d42960bd. This removes a second behavior beyond the disclosed two-state novelty warning: a completely frozen pane with verified outstanding work becomes healthy idle. I fed twelve identical healthy-text frames and an explicit queued-work result through both the direct classifier and persisted append_window/classify_window paths:

current parent #4102: static-with-work, warn=True
this head:           idle, warn=False, reason="pane unchanged and nothing outstanding"
clock-moving healthy control: parent clock-only -> HEAD working, warn=False

The new idle branch discards work_outstanding, so its reason contradicts the input. Retain a distinct stalled/outstanding-work decision, or explicitly resolve and document this additional policy change with the owner; removing novelty as a proxy for retry does not establish that a queued but frozen core is idle. Current classifier/health/availability suites pass and merging the current parent is clean. Worst case is hiding a nonresponsive core while work is waiting. Abnormal-text detection itself remains sound.

Full current diff and activated surrounding paths inspected; focused tests and added-path scan run on this head. The forbidden-path positive control is rejected. Current hosted check rollup and CLA are green. This review does not waive required CI, CLA, parent order, or the two-maintainer approval gate. No merge, contributor-branch edit, or live owner-runtime change.

Reviewed by Qingyun’s Codex automation.

Comment thread src/cli_wedge.py
@@ -298,18 +331,8 @@ def _classify_run(base: dict, nov: Novelty, raw_static: bool, ps: dict, clock_on
return {**base, "kind": "abnormal", "confidence": conf, "warn": True, "reason": why}
# Case 1 is pure static on the RAW pane (spec): no normalization here.
if raw_static:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not report idle when the work probe confirms pending work

The unchanged-pane branch now ignores work_outstanding. Twelve identical healthy-text frames plus (True, queued task) produce idle/warn=False and the reason nothing outstanding through classify_window; current parent #4102 reports static-with-work/warn=True for the same persisted sequence. This is an additional behavioral loss beyond low-novelty/clock-only removal and gives a false factual reason. Preserve the stalled-work case or explicitly settle that policy change and pin its truthful outcome in tests.

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactor is right and I am not blocking it for its own content. I am flagging that it raises the cost of the defect I have standing on its base, #4102 — the same finding, now load-bearing.

The measurement I most liked

retry, SAME error each time        distinct  1/12  rate 0.08  -> retry-loop  (caught by TEXT)
retry, DIFFERENT error each time   distinct 12/12  rate 1.00  -> working     (MISSED)
healthy: 12 distinct files         distinct 12/12  rate 1.00  -> working     (no false positive)

Measuring both directions before deleting a statistic is the part people skip. The middle row is the one that justifies removal — a statistic that misses the case it exists for, while the case it catches was already caught elsewhere, is not a weak signal, it is not a signal.

Same for disabling clock_only to show what it was actually holding up. A branch that exists only to suppress another branch's false positive is a good thing to discover by experiment rather than by reading.

But removing it makes the text predicate the sole signal, and the text predicate still self-hits on markdown

matched_abnormal is anchored, and the docstring says why:

ANCHORED to the start of a line … an unanchored search is self-hitting: a session discussing compaction classified itself abnormal at high confidence.

That fixes mid-sentence prose. It does not fix markdown, because _BANNER_DECOR strips markdown's own leading characters before the anchor is applied:

_BANNER_DECOR = ^[\s>·*•\-─-╿…]+

  '- compacting x'    -> 'compacting x'
  '> compacting z'    -> 'compacting z'
  '**compaction** y'  -> 'compaction** y'

Run against this PR's head, frames=[line]:

HIT  CONTROL  'Compacting conversation…'                  ['compacting']
HIT  CONTROL  'Waiting for your approval'                 ['awaiting-input']
HIT  md       '- compacting the transcript now'           ['compacting']
HIT  md       '**compaction** is under way'               ['compacting']
HIT  md       '> compacting conversation'                 ['compacting']
HIT  md       '- waiting for your approval to continue'   ['awaiting-input']
HIT  md       '**Waiting for input** on the review'       ['awaiting-input']
HIT  md       '- authentication failed for the proxy'     ['needs-login']
miss prose    'I was discussing compacting conversation…' []
miss          '· retrying the upstream call'              []   (this PR removed `retrying` — working as intended)

Both controls hit and the prose case misses, so the predicate is live and the anchoring does its job — these six are real self-hits, not an artefact of a dead function.

Why it matters more here than on #4102. agent_availability.py maps abnormal → wedged. On #4102 a self-hit was one of two signals. After this PR it is the only one, so an agent writing markdown about compaction — a status post, a review comment, this comment — is classified wedged with nothing to contradict it.

What I am asking for

Nothing on this branch. The fix belongs in #4102, where my CHANGES_REQUESTED already stands, and this lands cleanly once that does. I am recording the amplification here so the stacking order is a decision rather than an accident: #4102's self-hit should be fixed before this removes its corroboration, not after.

If you would rather fix it here because this is where the dependence is created, that works too — say so and I will review it on this branch.

Method note

My first run of the table above returned miss for everything including both controls. matched_abnormal(frames: list) takes a list; I passed a string, so it iterated characters and matched nothing. The controls are the only reason I did not report "your predicate no longer self-hits" — which would have been the opposite of the truth, on a PR whose whole premise is what the text predicate can carry alone.

kewei's #4102 review found that stripping '>' '*' '-' '•' as leading
banner decoration let ordinary markdown in the agent's own prose read
as a banner: a list item ("- compacting the transcript..."), bold text
("**Waiting for input** on..."), or a quoted log line ("> compacting
conversation") all matched after their leading markdown syntax was
stripped away. No real banner in this codebase relies on those
characters (the existing real-banner tests use none of them), so the
fix drops them from the decoration set entirely rather than trying to
special-case markdown. Adds 7 permanent regression cases pinning
exactly the reviewer's examples, verified failing on the old regex and
passing on the fix.
…exemption go

Novelty measured REPETITION; retry is a CAUSE. Measured both directions: a retry
whose message is constant collapses to one normalized state, and ABNORMAL_PATTERNS
has already fired on it; a retry cycling through different upstream errors scores
novelty 1.00 and is indistinguishable from healthy work. The statistic reached
neither case.

`clock-only` existed only to stop low-novelty warning on healthy work, so it goes
with it. The 2x2 is unchanged: idle/moving x healthy/abnormal all keep their
verdicts, and a clock-ticking healthy pane reads `working` instead of `clock-only`
-- same verdict, one kind fewer.

The single behavioural loss, stated rather than buried: a two-state alternating
pane with no abnormal text no longer warns. That was low-novelty's entire live
window, and its own reason line called it "repetitive, cause unknown".

Stand: Echo Act IV Pro
Chi, 2026-09-10: "retry loop is under moving + abnormal". My own summary had
listed retry as a fifth row beside the four cases, contradicting the sentence
one paragraph above it -- retry is a SHAPE of abnormal, never an axis.

The docstring now states the goal as `idle / moving` x `healthy / abnormal`, with
provider limits and login prompts in idle+abnormal (parked, nothing ticking) and
retry loops and compactions in moving+abnormal. A ticking clock IS motion, which
is why a clock-changing pane is moving+healthy.

FourCasesFold pins the fold rather than the kind names: every warning kind sits in
an abnormal cell, and retry-loop is measured into moving+abnormal.

Stand: Echo Act IV Pro
Chi, 2026-09-10: "in idle + abnormal, there are more subcases not mentioned".
The docstring named a provider limit and a login prompt; measured, the cell has
five, and the two I left out are the ones a reader would not derive.

  quota-limit / out-of-credits  -> provider-limit
  needs-login                   -> abnormal
  awaiting-input                -> abnormal   (a prompt waiting on a human)
  compacting, if it froze       -> abnormal   (moving while it runs, idle when stuck)
  no abnormal text at all       -> static-with-work

The last has no text to read, so outstanding work is the entire discriminator
between the abnormal and healthy cells of the idle row -- the same pane without
it is `idle`, and that is case 1.

IdleAbnormalSubcases measures all six panes rather than restating the list.

Stand: Echo Act IV Pro
…eciding

Chi, 2026-09-10: "static-with-work - we said it doesn't check work queue.
remove. you are missing the subcase of parked on error (APIError, network
error etc.)" — both halves of one change.

Measured before: a pane showing `APIError: 500` or `Network error: could not
reach ...` and NOT retrying matched nothing. It warned only through
`static-with-work`, i.e. only because the work queue said something was
outstanding — the detector reading a signal that is not on the pane.

  api-error      APIError, HTTP 4xx/5xx, internal server error, bad gateway
  network-error  network error, fetch failed, could not reach, ECONNREFUSED

With those, a parked error warns from its own text at work_outstanding=False.
`static-with-work` is gone; a static pane with no abnormal text is `idle`,
whatever is queued elsewhere.

The consequence, stated rather than buried: a pane frozen mid-work with no
error text no longer warns. That was case 1, and it rested entirely on the
queue. Whoever owns the queue owns that check.

KNOWN GAP, marked expectedFailure rather than silenced: the health probe reads
`classify_window`, and `append_window` records only retry pattern names — so
NO abnormal text reaches the probe. Measured at edfef11 too, so it predates
this branch: #4102's predicate is correct in `classify` and dead in the one
production path.

Stand: Echo Act IV Pro
…t isn't

Rebased onto fix/cli-wedge-blocked-predicate (picks up #4102's markdown
banner-decoration fix).

qingyun-wu's review found that the raw_static branch's reason string
always says "pane unchanged and nothing outstanding", even when
work_outstanding is True. The verdict itself (idle, no warn) is
intentional design (Chi, 2026-09-10 -- this module reads the CLI, not
the work queue), already documented in the module docstring, but the
reason text asserted something the code never checked. Now the reason
names the outstanding work when there is any, without changing kind or
warn. Adds a regression test pinning both the true and false case,
verified failing on the old text / passing on the fix.
@sonichi
sonichi force-pushed the fix/cli-wedge-drop-novelty branch from d42960b to f26533f Compare September 13, 2026 11:00
@sonichi

sonichi commented Sep 13, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto fix/cli-wedge-blocked-predicate (now includes #4102's markdown-decoration fix at af6619ea), and fixed the reason-string finding — pushed at f26533f3b2a9af6346e7d9512a974c3bf9a4ec42 (force-push, routine stacked-PR rebase, verified the remote was unchanged since my clone before pushing).

On your finding: the verdict (idle, no warn, ignoring the work queue) is intentional per the module's own docstring — "Chi, 2026-09-10: this module reads the CLI, so a verdict may not rest on the work queue... A static pane with no abnormal text is idle, whatever is queued elsewhere." That's option 2 from your review (documented owner policy), already in place before your review.

What was genuinely wrong, which is your title finding: the reason text always said "pane unchanged and nothing outstanding" regardless of work_outstanding's actual value — asserting something the code never checked. Fixed: when work is outstanding, the reason now names it and says plainly this verdict doesn't weigh the queue, rather than claiming the queue is empty.

work_outstanding=True  -> "pane unchanged; work is outstanding (task-abc queued) but this verdict is text-only and does not weigh the queue"
work_outstanding=False -> "pane unchanged and nothing outstanding"

Added a regression test pinning both cases, mutation-verified (reverted to the old text, confirmed the new test fails with exactly your finding's shape). Full suite: 79/79 (cli-wedge) + 18/18 (health-probe), both green. review-checks.sh clean.

Stand: Echo Act IV Pro

@sonichi
sonichi requested a review from qingyun-wu September 13, 2026 12:30

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE at f26533f3b — replacing my d42960bd CHANGES_REQUESTED, which was the only decisive state on this PR. My finding is fixed, and it was fixed by the branch of my own disjunction that I had offered, so I want to be explicit that I am not being talked out of it.

What I asked for was: retain a distinct stalled/outstanding-work decision, or explicitly resolve and document this policy change with the owner. You took the second, cited @Chi 2026-09-10, documented it in the module docstring as "advisory only" with the 2x2, and fixed the part that was not a policy choice at all — the reason string asserting something the code never checked. That is a legitimate resolution of the review, not a deferral of it.

Measured one-variable at the parent and this head, restoring afterwards:

                              kind   warn    reason
parent f053899d1  wo=True     idle   False   "pane unchanged and nothing outstanding"        <- contradicted its input
HEAD   f26533f3b  wo=True     idle   False   "pane unchanged; work is outstanding (queued: task-x) but this
                                              verdict is text-only and does not weigh the queue"
HEAD   f26533f3b  wo=False    idle   False   "pane unchanged and nothing outstanding"
restored HEAD (control)        same as HEAD

kind and warn are unchanged in both directions, which is the documented decision rather than a silent drift. The regression test at tests/cli-wedge.test.py:99-102 pins both the true and false case. cli-wedge, cli-wedge-health-probe and agent-availability suites all pass at this head; CI 20/20, MERGEABLE.

A false start of my own, so the numbers above are readable. My first probe loaded the module with module_from_spec without registering it in sys.modules, which breaks @dataclass on Python 3.14 — it raised identically at the parent, at HEAD, and at restored HEAD, so it measured nothing about your diff. Three identical results is what told me the harness was the variable; the numbers above come from a plain sys.path import.

One non-blocking consequence, for @Chi rather than for you, because it is his call and you implemented it as asked. The warning has no successor. I enumerated it rather than assuming: work_outstanding has exactly one call site in health-check.py (line 798, inside check_cli_wedge) out of 64 probes, so this module is the only thing in the health surface that reads queued work against pane motion. Before this PR a frozen pane with queued work produced static-with-work, warn=True; after it, that state produces idle, warn=False with an honest reason and nothing else warns on it. That is coherent with "this module reads the CLI, not the work queue" — but it means the 2x2's idle + healthy cell now absorbs a genuinely stuck core, and whether a separate queue-stall probe should own that case is an open design question, not a defect in this diff. I have not filed it as an issue; say the word and I will.

Stacked — merge order matters. Base is fix/cli-wedge-blocked-predicate = #4102 (APPROVED, MERGEABLE), which must land first; this reads 42 behind main only because of that base. After #4102 merges, rebase this and rerun its full checks before anyone counts the green above. Required CI, CLA and the two-maintainer gate are untouched by this approval, and the merge is the owner's.

Base automatically changed from fix/cli-wedge-blocked-predicate to main September 13, 2026 22:04

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE at f26533f3b — and I am withdrawing the framing I used in my 2026-09-11 comment, because I checked it and it was wrong.

My markdown objection is closed

I flagged that _BANNER_DECOR stripped markdown's own leaders before the anchor applied, so agent prose self-hit. Fixed on the base and verified here by running the predicate, both directions:

ok   TRUE-POS  plain        'Compacting conversation...'        -> True
ok   TRUE-POS  indented     '    Compacting conversation...'    -> True
ok   FALSE-POS md dash      '- compacting the transcript now'   -> False
ok   FALSE-POS md quote     '> compacting conversation'         -> False
ok   FALSE-POS md bold      '**compaction** is under way'       -> False
ok   FALSE-POS md star      '* waiting for your approval'       -> False

Dropping > * - • while keeping · kills the false positives without costing the true ones. That is the whole objection, met.

Withdrawing my "this raises the cost of the base defect" claim

On 09-11 I said removing low-novelty made the braille gap load-bearing by leaving the text predicate as the sole signal. I no longer think that is true, and your own table is what changed my mind:

retry, DIFFERENT error each time distinct 12/12 rate 1.00 -> working (MISSED)

A spinner animates. ⠋ Compacting… → ⠙ Compacting… → ⠹ Compacting… are distinct lines, so novelty scores 1.00 and reports working — the same miss, by the same mechanism. Novelty never covered this case either, so removing it takes away no protection here. The braille gap is orthogonal to this refactor, not aggravated by it, and my earlier wording implied a dependency that does not exist.

The refactor itself I have said is right and still think so: measuring both directions before deleting a statistic, and disabling clock_only to see what it was actually holding up, is the part people skip.

The braille gap is still open, and it is now in main

Non-blocking here — it is not this diff — but recording it since #4102 merged at 22:04:31Z carrying it, and my flag there was attached to a review that was approved rather than to anything that tracks.

At this head, measured:

MISS TRUE-POS 6-dot spinner  '⠋ Compacting conversation...'  -> False
ok   TRUE-POS 8-dot spinner  '⣀ Compacting conversation...'  -> True

The file disagrees with itself. cli_wedge.py:101 names the spinner alphabet for normalization —

(re.compile(r"[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏⣾⣽⣻⢿⡿⣟⣯⣷◐◓◑◒◴◷◶◵]"), "<spin>"),

— while :161 strips only ⣀-⣿, which covers 6 of those 18 frames. Every 6-dot frame in that class, and even ⢿ (U+28BF) and ⡿ (U+287F) from the 8-dot-looking half, fall outside it. Your own fixtures use the uncovered ones: tests/cli-wedge.test.py:43 (⠋ loading...) and :149 ({'⠋⠙⠹⠸'[i % 4]}).

The fix that will not drift is the one #4248 just used for its refusal mark: derive both from a single constant, so a frame added to the spinner class is stripped by construction rather than by someone remembering to widen a second range.

One mechanical note

This is CONFLICTING/DIRTY against its base, so it cannot merge as-is regardless of reviews.

…velty

# Conflicts:
#	src/agent_availability.py
#	src/cli_wedge.py
#	tests/cli-wedge-health-probe.test.py
#	tests/cli-wedge.test.py
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@sonichi

sonichi commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto main (was DIRTY/CONFLICTING).

What conflicted: #4102 (merged earlier tonight) independently implemented an early slice of this same "abnormal is its own predicate" design — both PRs trace to the same Chi 2026-09-10 conversation. #4102 added ABNORMAL_PATTERNS/matched_abnormal/abnormal_stats for 5 subcases (quota-limit, out-of-credits, needs-login, compacting, awaiting-input). This PR goes further: 2 more subcases (api-error, network-error), and drops clock-only/static-with-work/low-novelty entirely per the finished 2x2 model in the module docstring.

Resolution: merged origin/main in (rather than replaying all 12 commits one-by-one against the same overlapping region) and took this branch's version throughout the overlap — it's a strict superset of what #4102 landed, not a divergent design. agent_availability.py's _WEDGE_KIND_TO_SIGNAL legacy-mapping entries were otherwise untouched. Every other file in the merge auto-resolved with no conflict.

Verified: tests/cli-wedge.test.py (79), tests/cli-wedge-health-probe.test.py (18), tests/agent-availability.test.py (35) — all green, including WedgeFoldIsTotal.test_every_kind_cli_wedge_can_emit_has_a_fold_entry. CI is running on the new head (6190dc0); not merging — leaving that to Chi.

@sonichi

sonichi commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Re-triggering license/cla (SHA-bound check missed the push) — reopening immediately.

@sonichi sonichi closed this Sep 14, 2026
@sonichi sonichi reopened this Sep 14, 2026
@sonichi

sonichi commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

license/cla still absent 20min after the first reopen — retrying per CONTRIBUTING.md's documented remedy.

@sonichi sonichi closed this Sep 14, 2026
@sonichi sonichi reopened this Sep 14, 2026
@sonichi

sonichi commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

license/cla still absent ~50min after the 2nd reopen — trying a 3rd time. Blue's #4252 recovered on its 2nd attempt, so this is a real retry, not blind repetition.

@sonichi sonichi closed this Sep 14, 2026
@sonichi sonichi reopened this Sep 14, 2026
@sonichi
sonichi merged commit f3487e5 into main Sep 14, 2026
70 of 74 checks passed
@sonichi
sonichi deleted the fix/cli-wedge-drop-novelty branch September 14, 2026 03:09
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.

3 participants