refactor(probes): flatten the watch collapse and correct its comments

[bolichen97]

Problem / Motivation

src/kiro_crew/probes/ had never been through the module-simplification sweep, and it
had accumulated the two things that sweep exists to remove.

Comments written as a review log. gh_pr.py and targets.py explained their own
rules with a PR number (#7665), a review-round marker (Round 23 deleted this pattern outright), a scanner attribution (this is the one CodeQL flagged), a finding count
(produced three separate review findings), and past-tense narration about how each
rule reached its current shape. AGENTS.md § Code style forbids all of these: a comment
states current behavior and its reason, and that history lives in git.

Two comments that were simply false — found by checking every rewritten claim against
the adjacent code rather than trusting the prose, which is the part of this work that is
not cosmetic:

1. _collapse's docstring ended "a missing startedAt sorts oldest". A row with no
   timestamp is not sorted at all: when either row is undated the recency compare is
   skipped and the more conservative bucket wins, so an undated failing row beats a
   dated passing one — verified against a 2099 timestamp. The docstring pointed the
   wrong way on the one axis that matters for a watch probe (whether a possibly-broken
   row can be swallowed by a green one) and contradicted _CONSERVATIVE's own note 50
   lines above. _CONSERVATIVE's "must not lose to an older all-good row" is
   corrected the same way: that branch reads no dates, so older was aspirational.
2. _PR_SHORTHAND's quantifier bound was justified because the pattern "reads the whole
   instruction rather than stopping at a match". It is the URL loop that runs to
   completion (it cannot know whether a second subject exists without seeing them all);
   the shorthand loop return Nones on the first differing match.

Why it matters

A stale comment flipped to confident present tense misleads the next reader worse than
the stale one did, so a comment sweep that does not verify its claims makes things worse.
Both false comments are about how a red check can be suppressed — exactly the reasoning a
maintainer would lean on when changing this arbitration, and exactly where being wrong is
expensive. AUTOSDE.yaml's recurring-defect-patterns names "a docstring or comment that
CONTRADICTS the code below it" as a class that has shipped as a bug here before.

What changed (motivation → approach → change)

Backend-only, two files, no behavior change anywhere.

Comment hygiene — six task-log citations dropped, the surviving rationale restated in
present tense, and the two false claims above corrected. For each removal I checked
whether a constraint a future editor needs went with the provenance; none did (e.g. the
#7665 paragraph's load-bearing part — do not coerce wake_on_green from a string,
because bool("false") is True — survives verbatim beside the validator).

Three structural simplifications:

• _collapse unpacks the stored (started, bucket) pair into prev_started /
  prev_bucket instead of reading prev[0] / prev[1], and returns early on the
  first-row case, so the recency-versus-conservativeness arbitration reads as one flat
  chain. parsed.path.strip("/") is computed once instead of twice.
• _bucket's pending vocabulary becomes the module constant _PENDING, matching
  _FAILING / _PASSING / _NOISE rather than an inline tuple literal.
• infer parses a bare chain number the way its two sibling int parses already do —
  assign inside the try, compare after it — dropping one nesting level. int() is the
  only ValueError source there, so the guarded region is unchanged.

Plus two accuracy fixes: PrWatchProbe now declares host, which identity() assigns
and _fetch() reads but the attribute block omitted; and infer builds its probe config
as one dict literal instead of a literal followed by config["host"] = … (which also
removes the window where the config exists without its host pin).

Deliberately not done, so a reviewer can see they were considered:

• The three chained ternaries the detector still reports repo-wide are all in
  acp/client.py, a different module. One module per PR.
• Documenting host in the module docstring's "Message format" block. It is a real
  validated message key, but the code comment at its parse site says it is expressly not
  a configuration point — only targets.infer sets it, to one constant. Listing it in an
  operator-facing format block would contradict that.
• Target.host_key's "default" value is currently unreachable (infer is the only
  constructor and always passes the public host). Removing the default changes a
  constructor contract, which is a design call for the second probe kind, not a comment
  sweep.
• test/test_probe_targets.py's own docstring still carries the CodeQL attribution
  and first-person narration this PR removed from the code. Real, but it is a different
  file this PR does not otherwise touch — a "while I'm here" fix belongs in its own PR.
• _PR_URL's ReDoS rationale (targets.py:32-36) claims an unbounded + there is a
  polynomial shape; measurement did not reproduce that as quadratic, because the literal
  http prefix search rejects most start positions first. Unchanged by this PR and left
  alone: correcting it means re-deciding why those bounds exist, which is wider than this
  diff.

Tests

No test changes — this PR adds no behavior to lock in, and a test asserting a comment's
text would be the wrong instrument. Existing coverage already pins what the corrected
docstring now describes: test_babysit_pr_watch.py::test_queued_rerun_without_timestamp_blocks_false_ready
asserts that an undated rerun row must not lose to an older green row and produce a false
all-green wake — i.e. the behavior the old docstring described backwards.

167 tests across test_babysit_pr_watch.py, test_probe_targets.py,
test_babysit_guidance_gates.py and test_irq.py pass, plus the 824 in the wider
test_irq / test_autonudge* / test_monitor_* / test_github_pull_request_monitor set.

Manual verification

N/A — unit coverage sufficient, and equivalence was established mechanically rather than
by inspection:

• _collapse: exhaustive differential over all 1,884 possible 1–3-row input sequences
  (3 timestamp values × 4 buckets), pre-diff vs post-diff — 0 divergences. Independently
  reproduced by a reviewer at 40,000 and 60,000 randomized rollups (mixed workflow-less
  rows, 15 detailsUrl shapes, non-dict rows, missing startedAt), compared as ordered
  lists to catch dict-insertion-order drift — 0 diffs.
• _bucket: exhaustive over 1,080 and 3,174 conclusion × state × status combinations
  including None / "" / 0 / False / lowercase / unknown vocabulary — 0 diffs, and
  nothing moved between pending and failing in either direction.
• infer: 200,000 and 60,000 randomized instruction texts built from adversarial
  fragments (bare chains, &/and/comma separators, path fragments, enterprise hosts, a
  4,400-digit number that trips CPython's int-conversion limit, #0, #007), comparing
  (kind, subject, host_key, message) with message byte-wise — 0 diffs. "host" present
  on every one of 6,685 non-None results, and the set of host values ever written is
  exactly {'github.com'}.
• All four regexes: .pattern and .flags compared at runtime — byte-identical,
  including _PR_BARE, whose two adjacent string literals were already concatenated at
  parse time.
• host: str: Probe is a plain class (no @dataclass, no __slots__, no metaclass,
  no __init_subclass__), so a bare annotation creates no class attribute —
  hasattr(PrWatchProbe, "host") is False both before and after, and nothing in the repo
  reflects over these annotations.

Gates, all exit 0 on the rebased head: flake8, isort --check-only,
mypy --platform linux, black --check (both touched files), check_brand_name.py,
check_harness_parity.py, check_black_formatting.py, check_changelog_history.py,
check_per_file_coverage.py --test, docs_lint.py --test, docs-lint.sh,
verify_vendor_manifest.py, scrub-lint.sh --no-history, and the deterministic
code-review.yml grep rules (sensitive-path reads, hardcoded model literals, inclusive
language on added lines) — no matches.

Blocked by a red main, not by this diff

Backend Tests (3.12, 3), Backend Tests (Windows) (3) and Coverage Gate are red on
this PR and cannot be made green from here. They are inherited from main:

• The 9 failures are all in test/test_push_branch_gate.py and test/test_security.py
  (TestGitPublishSubshellGluing, TestUnrecognisedOptionsReadProtectively) — the
  git-publish shell-word gate. This PR touches only two modules under
  src/kiro_crew/probes/ and no security file.
• Attributed, not assumed. A pristine detached worktree at origin/main
  (6d1b51704) fails the identical 9 tests — the failure sets diff clean against this
  branch's. Bisected to eaa8a45bb "fix(security): model publish option arity so the
  floor tag holds (fix(security): model publish option arity so the floor tag holds #7808)": its parent passes 1936 tests, the commit itself fails 9.
• Coverage Gate is the same single failure, not a second one — shard 3 aborted, so it
  produced no coverage data to gate on.
• Already reported twice, so nothing is added by a third: main is red: #7808 broke test_push_branch_gate.py (6 failures), cascading to every PR #8695 and main is red: publish-gate arity change breaks 9 push-gate tests (semantic conflict) #8706, both
  naming #7808 as cascading to every PR.

The two shard-3 jobs on this PR's head report the same nine test ids that pristine
origin/main does — the sets diff clean, so zero failures are attributable to this PR.

I did not touch it. security.py is a keystone security file that the simplification
campaign forbids reshaping, and folding an unrelated security-gate repair into a
comment-and-readability PR is precisely the "undocumented bug fix riding in a cleanup PR"
this work exists to avoid. This PR is otherwise green — 52 of the 55 non-skipped checks
pass, including all five review lanes with zero findings between them — and it will go
green on its own once #7808 is fixed or reverted on main and this branch is rebased.

Screenshots / video

Why no screenshot: backend-only diff — two Python modules under src/kiro_crew/probes/,
nothing under website/, so no rendered surface changes and CI reports only_backend.

Related Issues

no linked issue: routine module-simplification sweep, not a tracked defect.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality (none needed — no new behavior)
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — the corrected comments are the documentation; no spec covers this module's internals
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 5d0d96010fd49c94ed65a037b6e75f5de332310f — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the code: self.host is assigned in identity() (line 383) and read at the _fetch call site (line 605), so the new annotation documents a real attribute; the _collapse restructure and infer reshaping are mechanically equivalent as described; the corrected docstring now matches the actual elif started and prev_started arbitration. Scope is tight (two files, one module), non-goals are explicitly reasoned, and equivalence was established by exhaustive/randomized differential testing rather than inspection.

Design-Verdict: PASS

A correctly-scoped, behavior-preserving hygiene sweep whose corrected docstrings now match the code they sit above; no design-level concerns.

[DESIGN-REVIEWED] 5d0d960

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 5d0d96010fd49c94ed65a037b6e75f5de332310f and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5d0d960

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5d0d96010fd49c94ed65a037b6e75f5de332310f: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5d0d96010fd49c94ed65a037b6e75f5de332310f — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5d0d960

Verdict parsed from the review's SHA-scoped output markers for commit 5d0d96010fd49c94ed65a037b6e75f5de332310f.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 5d0d96010fd49c94ed65a037b6e75f5de332310f: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 5d0d96010fd49c94ed65a037b6e75f5de332310f — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the code. Writing the review.

First-Principles-Verdict: PASS

A comment sweep that checked its own claims: both corrected docstrings now match the arbitration code they sit on, and every structural item is declared and behavior-free.

What this change ships

Intent: make the probes module's comments truthful and its code flat, with no behavior change — a FIX (two false comments) plus a declared hygiene/simplification sweep.

1. _collapse docstring: undated rows now described as winning by conservativeness, not "sorting oldest" — justified (contradicted the code at gh_pr.py:227-231; the class AUTOSDE names).
2. _CONSERVATIVE note drops the false "older" claim — justified (that branch reads no dates).
3. _PR_SHORTHAND bound rationale drops the false "reads the whole instruction" claim — justified.
4. Six task-log citations (fix(babysit): refuse a non-boolean wake_on_green instead of coercing it #7665, Round 23, CodeQL, finding counts) deleted, rationale kept in present tense — justified (AGENTS.md Code style mandates it; grep Round \d|CodeQL|#7665 in src/kiro_crew/probes/ now returns 0).
5. PrWatchProbe attribute block gains host: str — justified (identity() assigns it at gh_pr.py:383, _fetch reads it at :605; Probe in irq.py:255 is a plain class, so the annotation creates nothing).
6. infer builds its config as one dict literal with the host pin inline — justified.
7. _collapse flattened: tuple unpack, first-row early continue, path computed once — justified, readably equivalent.
8. _bucket's pending vocabulary becomes _PENDING beside _FAILING/_PASSING/_NOISE — declared; single consumer, module-private, carries the empty-string explanation.
9. Bare-chain parse assigns inside try, compares after — justified; int() is the only ValueError source, guarded region unchanged.
10. _PR_BARE adjacent literals joined, pattern byte-identical — justified.

No undeclared items, no new public surface, no config keys, no behavior. The corrected docstring's behavior is already pinned by test/test_babysit_pr_watch.py:630. The deferred siblings (test-file docstring, acp/client.py ternaries) are named in the description with counts and a plan — accepted deferrals, not gaps.

[FIRST-PRINCIPLES-REVIEWED] 5d0d960

[bolichen97]

• src/kiro_crew/probes/gh_pr.py:193 — StatusContext failures lack startedAt, so "an undated row is a just-queued rerun" contradicts the supported failing path — fixed in 9c6098c3a4a0fa4d3415e89b0092fdbe0ba46d89 (span=src/kiro_crew/probes/gh_pr.py:validation).

FINDING -- src/kiro_crew/probes/gh_pr.py:193 -- StatusContext failures lack startedAt, so "an undated row is a just-queued rerun" contradicts the supported failing path -> Fix: say an undated row may be queued or otherwise unorderable. (origin: validation)

Confirmed, and it is the same class of error this PR exists to remove — I replaced one overclaiming comment with another. Verified against GitHub's own schema as this repo spells it in src/kiro_crew/apps/builtins/issue_radar/backend/github_queries.py:40: ... on StatusContext{context state createdAt}. A StatusContext carries createdAt and no startedAt, while _collapse reads only startedAt — so every StatusContext row is permanently undated, not transiently so. "A just-queued rerun" named one cause as if it were the only one.

The docstring now reads:

A dateless failing row therefore beats a dated passing one, which is the point: a row may be undated because it is a just-queued rerun or because it is a StatusContext, which carries createdAt and no start time at all -- so silence about when it began is never evidence that it is stale.

I took the fix one step wider than the finding asked, because the same overclaim had a sibling: _CONSERVATIVE's own note (gh_pr.py:132-136) justified the tiebreaker with the bare parenthetical (a queued rerun has no startedAt yet). Leaving that would have left the corrected docstring pointing at a comment that still named one cause. It now reads (a queued rerun has no startedAt yet, and a StatusContext never has one). Both are comment-only; no behavior changed, and the gates plus the 167 probe/irq tests are green on the new head.

[bolichen97]

The three red checks are inherited from main, not from this diff

Backend Tests (3.12, 3), Backend Tests (Windows) (3) and Coverage Gate are
red on this PR because they are red on origin/main at the identical base
commit this branch sits on.

Evidence

• This branch's base is 6d1b51704, which is the current origin/main.

• main's own CI run for 6d1b51704 —
  run 33953620339
  — fails exactly the same three jobs: Backend Tests (Windows) (3),
  Backend Tests (3.12, 3), Coverage Gate.

• The failing-test sets are byte-identical between main's shard-3 job
  (101274249845) and this PR's shard-3 job (101278334614) — 9 tests, no
  additions and no removals:

  test/test_push_branch_gate.py::TestUnrecognisedOptionsReadProtectively
      ::test_a_background_operator_reads_protectively
      ::test_a_glued_all_branches_flag_keeps_its_identity
      ::test_a_glued_redirection_keeps_the_precise_positional_identity
      ::test_a_heredoc_strip_operator_consumes_its_delimiter
      ::test_every_bash_metacharacter_is_accounted_for
      ::test_extglob_patterns_are_wildcards_too
  test/test_security.py::TestGitPublishSubshellGluing
      ::test_a_redirect_is_skipped_not_treated_as_the_end_of_the_args
      ::test_path_qualified_feature_pushes_are_not_over_blocked
      ::test_quoted_operator_ref_names_stay_pushable
  

• Reproduced locally on this worktree (Python 3.12 CI-parity venv): the same 9
  fail deterministically, 42 passed.

• They exercise security._git_publish_floor_tags and
  _is_push_to_protected_branch. This PR touches only
  src/kiro_crew/probes/gh_pr.py and src/kiro_crew/probes/targets.py — no
  security module, no test file, nothing on the git-publish gate's call path.

• Culprit located: eaa8a45bb (fix(security): model publish option arity so the floor tag holds #7808, merged 07:03 UTC today) introduced
  TestUnrecognisedOptionsReadProtectively and its 6 now-failing tests, and its
  landing also flipped 3 pre-existing TestGitPublishSubshellGluing tests. At
  eaa8a45bb^ the class does not exist and none of the 9 run.

Already tracked, so nothing is being added here: #8695 reports the same
breakage, and three repairs are in flight — #8719, #8721, #8727.

Consequence for this PR: no code change here can clear those checks, and no
push is warranted — the reviewed head is unchanged and every reviewer lane is
fresh and PASS on it. Once one of the repairs lands on main, this branch
rebases onto it and the rollup goes green with no edit to the diff. Per the
repo's rules the tests are left exactly as they are: not skipped, not weakened,
not re-run as a "flake" — the failure is real, it is just not this PR's.

Everything else on this PR is settled: GPT 5.6 PASS, Opus 4.8 PASS, Design
Review PASS, First Principles PASS, 0 unresolved threads, and the one GPT
finding raised on the previous head is dispositioned as fixed.

[dwu96]

Tier 1 auto-approve: refactor (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: behaviour-preserving refactor of the check-rollup collapse — pending-conclusion set extracted to a named constant, the duplicate-row arbitration rewritten as an early-continue with unpacked locals (identical branch outcomes), a repeated path.strip() hoisted, the int() parse in the bare-PR scan lifted out of the comparison (same semantics), an adjacent-literal regex joined, and one dict entry moved into its literal; the only other addition is the missing host attribute annotation on PrWatchProbe, which is annotation-only on a plain class (not a dataclass) whose self.host assignment already exists on main. No control-flow or output change.