perf(test): route walk-tests through shared AST corpus

[iamwhatever]

Problem

~49-62 test files each independently walk the whole src/kiro_crew tree (Path.rglob("*.py") -> read_text -> ast.parse) on every scan, re-parsing ~1250 modules per gate. test/source_corpus.py already exists to fix this -- one cached rglob+read_text, with parsed_candidates(require_all=(literal,...)) yielding (path, text, tree) narrowed to files whose text holds the literals a gate can only match on -- but only ~5 files use it.

This is a conservative first migration: two clean, provably-lossless walkers. It deliberately does not attempt all ~60 files at once (each has different exclusions/patterns and per-file soundness risk).

Files migrated

test/test_knowledge_delete_off_loop.py

_on_loop_call_sites(name) now iterates parsed_candidates(require_all=(name,)). A call reaching name from an async body (directly, or via a same-module sync helper that calls it) can only exist in a file whose text contains the literal name, so the filter drops non-matches only. _SRC repointed to source_corpus.src_root() (same tree).

scan	before	after
delete_items_batch	14.97s	1.23s
_record_deduped_state	14.64s	1.18s
_resolve_old_item_ids	14.61s	1.12s

Whole-file suite: 38.11s -> 7.76s.

test/test_slack_render_pipeline.py

collect_repo_violations() now iterates parsed_candidates(require_all=("to_slack_mrkdwn",)). A direct to_slack_mrkdwn call -- bare-imported or reached as <module>.to_slack_mrkdwn -- can only exist in a file whose text holds that literal (the binding import, or the attribute call itself). find_violations is fed the corpus text (no second read).

test_no_module_converts_slack_markdown_directly: 14.47s -> 1.06s.

Lossless proof (per migrated scan, on the worktree tree the tests scan)

For each scan, the old full-rglob walk result-set == the new corpus-narrowed result-set, for the real target(s) AND a high-call-count probe symbol chosen so an unsound narrowing would drop offenders:

scan	target(s) old==new	probe(s) old==new
pilot	_record_deduped_state 0==0, delete_items_batch 0==0, _resolve_old_item_ids 0==0	append 1417==1417, get 6915==6915, info 850==850, close 308==308
slack	to_slack_mrkdwn 0==0	escape_mrkdwn 6==6, extract_options 7==7, render_for_slack 7==7

The get probe at 6915 call sites proves the text-narrowing loses nothing even at scale.

Red-before (per migrated file)

Injected a real violation the gate exists to catch and confirmed the migrated test still FAILS on it (proving the corpus-narrowed scan still sees offenders), then restored:

• pilot: a direct self.store.delete_items_batch(item_ids) in async _handle_deleted -> FAIL at folder_watcher.py:823.
• slack: a direct to_slack_mrkdwn("x") in subagent.py -> FAIL at subagent.py:2466.

Files deliberately SKIPPED (follow-up, not migrated here)

• test/test_lazy_data_home_paths.py -- _transitive_path_factories() builds a transitive closure over Path-returning functions across the whole tree; the forbidden set is derived dynamically and a factory can be named anything, so there is no single text literal that bounds the scan without risking a dropped offender.
• test/test_cron_store_unreadable_boundaries.py -- _read_decide_write_callers() flags a function calling both a _CRON_READS name and a _CRON_WRITES name: require_any over two separate name sets (needs >=1 read AND >=1 write literal). Expressible in principle but higher soundness risk; held for a dedicated review.
• test/test_jsondecodeerror_redundancy_ratchet.py -- scans three roots (src/kiro_crew, test, scripts); the shared corpus only covers src/kiro_crew, so routing it through the corpus would silently shrink its scope.

The rule applied throughout: correctness over coverage -- a silently-narrowed gate that stops catching real violations is worse than a slow test, so a file is migrated only when its full-walk result set is proven identical to the corpus-narrowed one.

Tests / gates

• Migrated files: 82 (pilot) + 13 (slack scan family) green; test/test_source_corpus.py guard suite green.
• isort clean, flake8 clean, black baseline gate passed (both files stay in the black baseline; formatting untouched).
• mypy shows 2 errors on run_to_completion(lambda: None) -- pre-existing, identical to base, not in the migrated code.

Note

The corpus cache is per-xdist-worker (each worker parses once) -- the accepted existing tradeoff, unchanged here.

Do not merge -- review-ready.

---

Update — GPT 5.6 BLOCKING finding addressed (commit 8b27e8f)

Finding (legitimate): parsed_candidates(require_all=(name,)) pre-filtered files by RAW TEXT, but CPython NFKC-folds identifiers at parse time. A src call written with a Unicode compatibility homoglyph of a guarded name — e.g. delete_items_bａtch (fullwidth ａ, U+FF41) or to_slａck_mrkdwn — is the ASCII name in the AST (a real offender) yet the raw literal is absent from the bytes, so the file was skipped and the gate passed green while the unsafe call shipped. Reproduced and confirmed.

Fix (keeps the speedup, closes the hole, general to every corpus consumer): source_corpus.candidate_sources now matches require_all/require_any against an NFKC-normalized view of each file's text with NFKC-normalized needles. The normalized view is computed once over the tree (~0.3s) and cached like the read. source_texts() still returns raw text — gates that scan comments/strings (# render-ok, import aliases) depend on that. NFKC is a fixpoint on ASCII and never removes/merges ASCII letters, so every raw ASCII match is preserved and only homoglyph spellings are newly caught. Chose this over GPT's suggested "drop the text-filter" because it closes the hole and keeps the narrowing.

Loses nothing (re-proved on the worktree tree): old raw full-walk result-set == new normalized corpus-narrowed set, for targets AND high-call probes: append 1417==1417, get 6915==6915, info 850==850, close 308==308; slack escape_mrkdwn 6==6, extract_options 7==7, render_for_slack 7==7.

Unicode red-before (hole closed): injected homoglyph offenders and confirmed the migrated tests now FAIL:

• delete_items_bａtch in async _handle_deleted → FAIL at folder_watcher.py:823
• _fmt.to_slａck_mrkdwn(...) in subagent.py → FAIL at subagent.py:2467

then restored. The ASCII red-befores still fail as before.

Speedup preserved: pilot scan ~1.0s, slack scan ~0.6s (the one-time NFKC is amortized across scans; baseline was 14–15s).

Verification note: all runs used PYTHONPATH=<worktree>/src — the .venv is an editable install whose .pth points at the MAIN checkout, but under pytest kiro_crew resolves to the worktree src both with and without that prefix (confirmed via find_spec); the prefix makes it explicit. Gates: isort/flake8/black clean; mypy clean on source_corpus.py; 96 tests green (test_source_corpus.py + both migrated suites).

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 8b27e8f8573e8bcc9e403c2a9f2624297f7bef8b and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 8b27e8f

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Sound, conservatively-scoped migration with genuine per-gate soundness arguments; the NFKC fix closes the narrowing hole for every corpus consumer, not just these two.

Suggestions

• The homoglyph closure is proven only in the PR body — test_source_corpus.py has no NFKC test, so a later "simplify back to raw in text" refactor silently reopens the bypass across every corpus-narrowed gate (~60 planned). Pin it with a cheap fixture test (a homoglyph needle/haystack pair through candidate_sources) before migrating more gates onto this property.

[DESIGN-REVIEWED] 8b27e8f

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 8b27e8f8573e8bcc9e403c2a9f2624297f7bef8b — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 8b27e8f

Verdict parsed from the review's SHA-scoped output markers for commit 8b27e8f8573e8bcc9e403c2a9f2624297f7bef8b.

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

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

All user-visible changes reduce friction: the chat auto-follow no longer drops mid-stream, and theme loader icons fail closed to the default poses, never blank.

Suggestions

• theme_validate.py "contains unknown symbol: {name!r}" — append the allowed names (already enumerated in _THEME_LOADER_ICONS) so a pack author can fix the manifest without hunting the docs.

[UX-REVIEWED] 8b27e8f

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 8b27e8f8573e8bcc9e403c2a9f2624297f7bef8b — 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 verification is done. The declared change checks out against the repository (61 test files still carry their own rglob("*.py") walk, matching the author's "~49-62 remain" claim; 7 now import source_corpus; the corpus module's own docstring pins the measured rejection of the parse-everything alternative). Three of the patch's five undeclared clusters are byte-for-byte the subjects of already-merged main commits (#7982, #7954, #7957), which proves the diff was cut against a stale base.

First-Principles-Verdict: CONCERNS

The declared corpus migration is exemplary; the patch carries five unrelated clusters, three provably already-merged main commits — so the evidence doesn't isolate this PR.

What this change ships

Intent: make ~50 whole-tree AST gate tests fast by routing two of them through the existing shared corpus — a FIX (test perf + a reported soundness hole).

1. Two AST-gate suites scan via the shared corpus (38.11s→7.76s, 14.47s→1.06s) — justified
2. Corpus literal-narrowing is NFKC-normalized, closing the homoglyph bypass for every corpus consumer — justified, cause-level
3. ~59 sibling walk-tests left unmigrated (my count: 61 files grep rglob("*.py") in test/) — declared, accepted-and-deferred
4. Five review workflows rename their job on fork PRs — duplicate of main tip 48c393d (fix(ci): give the same-repo review lanes a distinct check name on fork PRs #7982); stale-base drift
5. Widgets skill doc CSP/canvas corrections — duplicate of 7afe34e (docs(skills): fix widget canvas sizing hazard and stale CSP claim #7954); drift
6. UNC path-token boundary derived from whitespace — duplicate of 96ce973 (fix(security): derive the UNC path-token boundary from whitespace #7957); drift
7. Theme manifests gain a loaderIcons key (validator, frontend map, spec) — undeclared
8. Chat scroll-follow gains a viewportShrink allowance — undeclared

Watch

• The diff was cut against a stale merge-base: items 4–6 are verbatim the subjects of commits already on main (the checkout is pull/8024/merge onto 48c393d, whose own subject IS item 4). Items 7–8 have the same signature — complete, unrelated, self-tested — and are almost certainly further main-side commits below my 5-commit visibility window, but I cannot prove it from this checkout. If either is actually on this branch, a perf(test) PR is shipping an undeclared one-way-door manifest key (loaderIcons must be honored forever) plus a UX behavior change, and each belongs in its own PR. A human should confirm the PR's real file list before merging.
• Lens-3/4 on the declared work, for the record: it uses the existing mechanism (test/source_corpus.py) rather than adding one, and the delete-alternative (drop narrowing, cache parsed trees) was measured and rejected in the module docstring (~900 MB RSS, 4× slower parse) — the narrowing earns its NFKC subtlety.

[FIRST-PRINCIPLES-REVIEWED] 8b27e8f