fix: break the consolidation-package headless_authoring import cycle (#237) by cdeust · Pull Request #277 · cdeust/Cortex

cdeust · 2026-07-30T00:58:41Z

Summary

Fixes #237. candidate_scan, claude_cli, cycle_orchestration, and
drain_operations each did from . import headless_authoring as _root at
module top, while headless_authoring imported names back from each of
them at its own module top. Importing headless_authoring first resolved
the cycle (why the suite stayed green); importing any of the four directly
in a fresh interpreter raised the partial-module ImportError quoted in
the issue.

Root cause (coding-standards.md §6): local-reasoning defeat at the
module-import boundary — a load-order-sensitive circular import, the same
defect class as #233 (different package, not touched here).

Fix (Fowler: Slide Statements): each of the four modules now resolves
_root with a deferred, function-scoped import instead of a module-top-
level one (# noqa: PLC0415 — import cycle, the exemption
pyproject.toml already names for "the pre-existing #233 family"). The
load-time back-reference is gone; the call-time attribute lookup that
makes monkeypatch.setattr(headless_authoring, ...) observable across
~30 existing test sites is unchanged — same live module object, same
patchability, just resolved when the function runs instead of when the
module loads.

Three DI-seam parameters (invoke, max_drains, max_anchor_drains)
baked _root.X into their parameter-list defaults, which only works when
_root is bound at module scope. Converted to None sentinels resolved
inside the function body — same live-module read, now at call time.

Root cause

Classified per coding-standards.md §6: local-reasoning defeat (§7.3) at
the module-import boundary, not a missing contract or layer violation —
the four submodules and their hub read each other's names correctly, just
at the wrong TIME (module load vs. function call). Fixed at that exact
source (the import's binding site), not by reordering/pinning callers.

Follow-up work folded into this PR (issue #276, closed by this PR)

A prior round of this fix filed #276 as a deferral for pre-existing
method-length/file-size debt in this same touched-file family, plus a
CLAUDE.md/CONTRIBUTING.md size-limit doc drift. Per a later directive
against splitting discovered work into follow-up issues, that debt is
now fixed here instead of deferred:

Doc drift resolved: 300 lines/file, 40 lines/method is now
authoritative (CLAUDE.md and CONTRIBUTING.md agree); CLAUDE.md's
stale "enforced by the craftsmanship-checker pre-commit hook" claim
corrected to "enforced by code review today; no automated hook
exists" (verified: no .pre-commit-config.yaml, no hook script).
Every file this PR touches is now ≤300 lines, every method ≤40
lines (Fowler Extract Function / Move Function, one catalog entry
per commit):
- candidate_scan.py: _scan_pages_with_gaps/
  _collect_anchor_candidates split into smaller named helpers.
- drain_operations.py: drain_one/drain_all_gaps_on_page split;
  drain_missing_anchors moved to a new anchor_authoring.py
  (distinct concern — whole missing anchor pages vs. per-page gap
  fills). Shared _drain_result/_elapsed_ms helpers factored out
  of the repeated DrainResult(...) construction (§3.3, far more
  than 3 concrete uses).
- cycle_orchestration.py: run_headless_authoring_cycle's two
  nested coroutines became module-level functions taking
  sem/budget/invoke/_root as explicit parameters — exactly
  headless-authoring worker family: methods exceed 50-line cap (§4.2), CLAUDE.md size-limit text drifted from CONTRIBUTING.md #276's own prescribed fix shape.
- headless_authoring.py: _claude_invoke split and moved to a new
  claude_invoke.py; CycleBudget/DrainResult/CycleSummary
  moved to a new cycle_types.py (plain dataclasses, no
  circular-import risk). File: 469 → 255 lines.
Pyright gotcha worth recording: the if X is None: X = _root.Y
default-parameter resolution pattern must stay inline in the
function where _root is bound via the local deferred import
(concrete module type there) — moving it into an extracted helper
that receives _root: Any as a parameter breaks pyright's narrowing
(assigning Any to a T | None-declared parameter doesn't clear the
None branch), failing a strict non-Optional return type even
though the runtime value is never None. Verified via a minimal
repro before settling on the inline placement.
Mutation testing (§12): scoped mutmut on the touched family
surfaced ~570 survivors. A controlled A/B comparison — mutating the
pre-refactor cycle_orchestration.py against the identical test
selection — produced the exact same 121-survivor count as the
post-refactor file, proving the bulk of these are inherited test
weakness (subprocess/invoke test doubles that accept-and-ignore most
kwargs), not something this refactor introduces. Closed the
tractable subset: 7 new tests for candidate_scan.py (no subprocess/
PostgreSQL, cheap to test directly — and every existing test in the
suite monkeypatches its functions out entirely), cutting that file's
survivor count from 38 (pre-refactor baseline) to 25. The remaining
survivors (subprocess/invoke fakes across claude_invoke.py,
drain_operations.py, cycle_orchestration.py, anchor_authoring.py)
are a test-infrastructure investment out of this PR's blast radius —
reported to the coordinator with the evidence above, not filed as a
new issue per this session's directive.

Completion Ledger (§13.2)

Path / branch introduced	Asserted by
`candidate_scan` imports standalone in a fresh interpreter	`test_submodule_imports_standalone_in_fresh_interpreter[...candidate_scan]`
`claude_cli` imports standalone in a fresh interpreter	`test_submodule_imports_standalone_in_fresh_interpreter[...claude_cli]`
`cycle_orchestration` imports standalone in a fresh interpreter	`test_submodule_imports_standalone_in_fresh_interpreter[...cycle_orchestration]`
`drain_operations` imports standalone in a fresh interpreter	`test_submodule_imports_standalone_in_fresh_interpreter[...drain_operations]`
`anchor_authoring` imports standalone in a fresh interpreter (new module, issue #276)	`test_submodule_imports_standalone_in_fresh_interpreter[...anchor_authoring]`
`claude_invoke` imports standalone in a fresh interpreter (new module, issue #276)	`test_submodule_imports_standalone_in_fresh_interpreter[...claude_invoke]`
hub-first import path + re-export surface unaffected	`test_headless_authoring_still_imports_first_and_re_exports`
`drain_one`'s omitted-`invoke` sentinel resolves to the live `_claude_invoke`	`test_drain_one_default_invoke_resolves_to_live_claude_invoke`
`drain_all_gaps_on_page`'s omitted-`invoke` sentinel resolves to the live `_claude_invoke`	`test_drain_all_gaps_on_page_default_invoke_resolves_to_live_claude_invoke`
`run_headless_authoring_cycle`'s omitted `invoke`/`max_drains`/`max_anchor_drains` resolve to the live module attributes (cap actually applied, not silently unbounded)	`test_run_headless_authoring_cycle_defaults_resolve_from_live_module`
`_scan_pages_with_gaps` finds frozen-gap pages, skips no-gap/dotfile/underscore-dir pages, handles a missing wiki_root, and live-audits kind=reference file-docs (issue #276 mutation follow-up)	`tests_py/handlers/consolidation/test_candidate_scan.py` (7 tests)
`drain_missing_anchors` still resolves off `anchor_authoring` after the Move Function (groundable-only filter unaffected)	`test_drain_missing_anchors_skips_ungroundable` (monkeypatch targets updated to the new module)
Existing suite behavior unchanged (0 modified assertions across all refactor commits)	full pre-existing suite; targeted subset 65 → 67 passed (+2 new fresh-interpreter cases only); full suite 6558 → 6567 passed (+9 new tests only, all additive)

Test plan

🤖 Generated with Claude Code

…time (#237) candidate_scan, claude_cli, cycle_orchestration, and drain_operations each did `from . import headless_authoring as _root` at module top, while headless_authoring imported names back from each of them at ITS module top. Importing headless_authoring first resolved the cycle (why the suite stayed green); importing any of the four directly in a fresh interpreter raised "cannot import name '<x>' from partially initialized module ... (most likely due to a circular import)" — reproduced pre-fix for all four. Fix: each of the four modules now resolves `_root` with a deferred, function-scoped import (`# noqa: PLC0415 — import cycle`, the exemption pyproject.toml already names for this defect family, "the pre-existing #233 family") instead of a module-top-level one. The load-time back- reference is gone; the call-time attribute lookup that makes `monkeypatch.setattr(headless_authoring, ...)` observable is unchanged. Three DI-seam parameters (`invoke`, `max_drains`, `max_anchor_drains`) previously baked `_root.X` into their parameter-list defaults, which is only valid when `_root` is bound at module scope — converted to `None` sentinels resolved inside the function body instead (same live-module read, now at call time rather than def time). Mutation testing (scoped to drain_operations.py + cycle_orchestration.py) found 2 real surviving mutants in the new sentinel logic: flipping `max_drains`/`max_anchor_drains`'s `is None` to `is not None` left the cap silently unresolved (`list[:None]` == no cap) and slipped past the existing single-candidate test. Strengthened test_run_headless_authoring_cycle_defaults_resolve_from_live_module to offer 3 candidates against a cap of 1 on each side; both mutants now kill. Before: 4/4 fresh-interpreter imports fail (reproduced). After: 4/4 succeed; hub-first import path unaffected (verified). Tests: 1480 passed before, 1488 passed after (+8 new, 0 modified assertions in existing tests — the load-order workaround imports/comments in test_s110_sweep_handlers.py and test_ble001_sweep_handlers.py were removed as they document a cycle that no longer exists, not a behavior change). Refs #233 (same defect class, different package — not touched here). Follow-up filed as #276 (pre-existing method-length debt in this file family, discovered but outside this fix's blast radius). Fixes #237 Co-Authored-By: Claude <noreply@anthropic.com>

CLAUDE.md stated "300 lines max per file; 40 lines max per method. Enforced by the craftsmanship-checker pre-commit hook" while CONTRIBUTING.md cited coding-standards.md's actual §4 numbers ("File <=500 lines, function <=50 lines"). Neither number was wrong on its own, but no craftsmanship-checker hook exists (.git/hooks/ has only *.sample files, no .pre-commit-config.yaml) — the claim was stale. Resolution: 300/40 is this repo's local tightening of the global 500/50 standard (coding-standards.md SS10 permits tightening a hard rule, never weakening it). CONTRIBUTING.md now cites 300/40 and points at CLAUDE.md as authoritative; CLAUDE.md's enforcement claim is corrected to "code review today, no automated hook yet" instead of naming a hook that was never wired. Refs #276 (found during #237's boy-scout pass over the same touched file family).

_scan_pages_with_gaps (52 lines) and _collect_anchor_candidates (55 lines) both exceeded the local 40-line/method cap (CLAUDE.md, resolved against CONTRIBUTING.md in the prior commit). Extracted the per-page gap-detection body into _gap_entry_for_page (+ the lazy missing_sections probe into _load_missing_sections_probe), and the per-domain candidate loop into _extend_anchor_candidates_for_domain (mutates the same candidates list in place, same early-exit-at-cap semantics as the original inline return). Pure Extract Function — no behavior change. Longest function now 36 lines; file 166 lines (was 134, growth is docstrings/signatures on the new small functions). Before: pytest tests_py/handlers/{test_headless_authoring_throttle,test_s110_sweep_handlers}.py tests_py/handlers/consolidation/{test_import_cycle_237,test_headless_authoring_governance,test_distill_drain}.py tests_py/hooks/test_headless_guard.py -> 65 passed After: same command -> 65 passed, 0 modified assertions. Refs #276.

…sue #276) drain_one (81 lines), drain_all_gaps_on_page (122 lines), and drain_missing_anchors (94 lines) all exceeded the local 40-line/method cap, and the file itself (397 lines) exceeded the local 300-line/file cap (both from CLAUDE.md, resolved against CONTRIBUTING.md two commits ago). Extract Function: split drain_one into _invoke_section_fill + _finish_drain_one; split drain_all_gaps_on_page into _invoke_page_fill, _no_response_results, _fill_gaps_from_response, _persist_filled_gaps. Factored the repeated DrainResult-construction + elapsed-ms computation (previously inlined 3-8x per function) into two small shared helpers, _elapsed_ms and _drain_result — a legitimate extraction per coding-standards.md SS3.3 (far more than 3 concrete uses). Move Function: drain_missing_anchors (a distinct concern — authoring whole missing anchor pages, vs. per-page gap-filling) moved to a new sibling module anchor_authoring.py, matching this package's existing split-by-concern convention (candidate_scan/drain_operations/ cycle_orchestration were already split out of headless_authoring the same way). anchor_authoring.py reuses _drain_result/_elapsed_ms from drain_operations (no import cycle — anchor_authoring -> drain_operations is one-directional). headless_authoring.py's re-export now pulls drain_missing_anchors from .anchor_authoring instead of .drain_operations. Both new/changed files keep the issue #237 deferred-import pattern (_root resolved at call time, not module top) so monkeypatch.setattr(headless_authoring, ...) stays observed — verified by extending tests_py/handlers/consolidation/test_import_cycle_237.py's fresh-interpreter parametrize list to include anchor_authoring (a new module carrying the same load-order-sensitive pattern needs the same regression coverage). tests_py/handlers/test_headless_authoring_throttle.py's test_drain_missing_anchors_skips_ungroundable patched drain_operations._build_registry/_project_source_root/audit_domain by dotted-path string; updated the three patch targets to anchor_authoring (the function's new home) — mechanically required by the Move, not a behavior-expectation change. Before: 65 passed (prior commit's baseline + the candidate_scan split). After: 66 passed (+1 for the new anchor_authoring parametrize entry), 0 modified assertions. Post-refactor sizes: drain_operations.py 397->300 lines (all methods <=38); anchor_authoring.py new file, 164 lines (all methods <=37). Refs #276.

run_headless_authoring_cycle was 184 lines with two nested coroutines (drain_anchor_bounded, drain_page_bounded + an inner charging_invoke closure) closing over sem/budget/invoke/wiki_root/today/_root — both the local 40-line/method cap and the file's readability suffered. Extract Function, following #276's own prescribed fix shape ("become module-level functions taking sem/budget as explicit parameters"): split into _build_cycle_candidates (phase 1), _invoke_anchor_call + _drain_anchor_bounded + _make_charging_invoke + _drain_page_bounded (phase 2, now module-level, no closures), _gather_cycle_results (phase 3), _build_cycle_budget_and_sem, _execute_cycle, and a thin run_headless_authoring_cycle that only resolves omitted parameters then delegates. Reused drain_operations' _drain_result/_elapsed_ms helpers here too (imported, no cycle — cycle_orchestration -> drain_operations is one-directional and already existed). Pyright gotcha (worth recording): the four "if X is None: X = _root.Y" default-parameter resolutions MUST stay inline in run_headless_authoring_cycle, not move into an extracted helper — _root there is the concrete live-module type (a local deferred import), which is what lets pyright narrow away the parameter's `| None` on reassignment. An extracted helper receiving `_root: Any` as a parameter loses that narrowing (`Any` doesn't clear a declared `X | None`), so a strict `-> tuple[Path, int, int, Callable[...]]` return type fails pyright even though the runtime value is never None. Verified by a minimal repro before settling on the inline placement. Every `_root.X` access still resolves at call time (issue #237's patchability contract for monkeypatch.setattr(headless_authoring, ...) is unchanged — verified by the unmodified throttle test suite passing). Before: 66 passed (prior commit's baseline). After: 66 passed, 0 modified assertions. pyright: 0 diagnostics. Post-refactor: cycle_orchestration.py 237->299 lines (file itself was already under the 300 cap; growth is from the extra function signatures), all methods <=38 lines (was 184 for the worst offender). Refs #276.

headless_authoring.py was 469 lines (over the local 300-line/file cap) and its _claude_invoke was 113 lines (over the 40-line/method cap) -- the last remaining size-cap violation in the consolidation-package touched-file family from #237's fix. Move Function: _claude_invoke split into _spawn_claude_process, _communicate_with_timeout, and _parse_invoke_response (Extract Function), then all four moved to a new sibling module claude_invoke.py, following this package's established split-by-concern pattern. claude_invoke.py carries its own deferred `_root` import (same issue #237 pattern -- headless_authoring imports _claude_invoke back at load time) and returns `Any` (not InvokeResult) matching every other sibling's established convention for the same circular-import reason. Move Function (second split, same commit -- both needed to clear the file-size cap): CycleBudget, DrainResult, and CycleSummary moved to a new cycle_types.py (plain dataclasses, zero dependency on headless_authoring or any sibling -- no import-cycle risk). InvokeResult and _AnchorCandidate stay in headless_authoring.py (distinct concern: worker identity/config, not cycle telemetry). headless_authoring.py re-exports all three names from cycle_types -- every existing `_root.CycleBudget(...)` / `_root.DrainResult(...)` / `_root.CycleSummary(...)` call site in the other sibling files is unaffected (same class objects, same module attribute, different file). Removed now-unused imports from headless_authoring.py (asyncio, json, field) after the move; verified with grep before removing each one. Regression coverage: extended test_import_cycle_237.py's _AFFECTED_MODULES to include claude_invoke (same load-order-sensitive pattern as the other five). cycle_types.py needs no entry -- it has no dependency on headless_authoring at all. Before: 67 passed (prior commit's baseline). After: 67 passed, 0 modified assertions (the +1 from claude_invoke's new parametrize entry was already counted in "before" via this same commit's test change). pyright: 0 diagnostics across headless_authoring.py, claude_invoke.py, cycle_types.py, and every other touched consolidation file. Post-refactor: headless_authoring.py 469->255 lines; claude_invoke.py new, 160 lines (all methods <=40); cycle_types.py new, 90 lines (no functions over 7 lines). Every file this PR touches in the consolidation package is now <=300 lines; every method is <=40 lines. Refs #276.

…§12) Scoped mutation testing (mutmut) on the consolidation-package files touched by #237/#276 surfaced 570-576 survivors across the family. A controlled A/B comparison (mutating the pre-refactor cycle_orchestration.py against the identical test selection) produced the EXACT SAME 121 survivor count as the post-refactor file, proving the bulk of these are inherited test weakness (subprocess/invoke fakes that accept-and-ignore most kwargs), not introduced by this PR's Extract/Move Function work -- see the refactorer's session notes for the full evidence and the surviving-mutant inventory. candidate_scan.py was the one file in this family cheap to close real gaps in: no subprocess, no PostgreSQL, pure filesystem + string logic, and every EXISTING test in the suite monkeypatches _scan_pages_with_gaps out entirely (test_headless_authoring_throttle.py, test_import_cycle_237.py), so its real walking/filtering/tuple-building logic had zero direct coverage before this commit. Added 7 tests exercising _scan_pages_with_gaps against a real tmp_path wiki tree: frozen curation_gaps detection, no-gaps exclusion, dotfile/ underscore directory exclusion, missing wiki_root, mixed-page filtering, and the live-audit axis (kind=reference + source_file_path, both the missing-sections-found and non-reference-kind-skip cases). Effect (scoped mutmut, candidate_scan.py only, same test list plus this file): survivors 38 (pre-refactor baseline) -> 35 (post-refactor, before these tests) -> 25 (post-refactor, with these tests) covering _gap_entry_for_page and _scan_pages_with_gaps fully. The remaining ~25 survivors are in _collect_anchor_candidates/_extend_anchor_candidates_ for_domain (registry/audit_domain/_AnchorCandidate construction), requiring the same class of test-infrastructure investment as the subprocess/invoke fakes elsewhere in this family -- out of this PR's blast radius per the same evidence. Test-only commit: no production code changed; existing suite passes unchanged (0 modified assertions), 7 new tests added.

…defaults test test_run_headless_authoring_cycle_defaults_resolve_from_live_module was 91 lines (this repo's local 40-line/method cap, CLAUDE.md § Code Style) -- new code introduced by this PR's own #237 fix, caught by this PR's pre-push size-cap gate before pushing (the same class of avoidable refusal as PR #284's 54-line method, per this session's directive). Extract Function: the anchor-candidate stub, the page-scan stub, and the five-line monkeypatch setup block each moved to a module-level helper (_make_fake_collect_anchor_candidates, _make_fake_scan_pages_with_gaps, _patch_cycle_defaults_fixtures). Pure extraction -- same assertions, same fakes, same 10/10 passing tests before and after; no behavior change. Before: 91 lines. After: 37 lines (file 267 -> 296, both under the 300/40 caps). Boy-scout note (coding-standards.md §14): three sibling test files in this same package (test_headless_authoring_throttle.py 788 lines, test_ble001_sweep_handlers.py 470, test_s110_sweep_handlers.py 326) also exceed the caps, but predate this PR entirely and this PR's own diff to them is a mechanical 8-10 line patch-target update from the #276 Move Function work, not new growth -- splitting them is a substantial refactor out of a rebase PR's blast radius. Filed as issue #300 with acceptance criteria rather than silently deferred. Refs #237, #276. Filed: #300 (pre-existing sibling test-file size debt).

cdeust mentioned this pull request Jul 30, 2026

headless-authoring worker family: methods exceed 50-line cap (§4.2), CLAUDE.md size-limit text drifted from CONTRIBUTING.md #276

Closed

cdeust force-pushed the fix-consolidation-import-cycle-237 branch 2 times, most recently from cc66bd1 to bc1902c Compare July 30, 2026 11:56

cdeust and others added 7 commits July 30, 2026 14:23

cdeust force-pushed the fix-consolidation-import-cycle-237 branch from bc1902c to 43e1d98 Compare July 30, 2026 12:34

cdeust mentioned this pull request Jul 30, 2026

consolidation-package test files exceed 300-line/40-line caps (pre-#237, untouched by #276's own cap sweep) #300

Closed

5 tasks

cdeust force-pushed the fix-consolidation-import-cycle-237 branch from 43e1d98 to 9b7df49 Compare July 30, 2026 12:40

cdeust merged commit 3a5e6d4 into main Jul 30, 2026
19 checks passed

cdeust deleted the fix-consolidation-import-cycle-237 branch July 30, 2026 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: break the consolidation-package headless_authoring import cycle (#237) - #277

fix: break the consolidation-package headless_authoring import cycle (#237)#277
cdeust merged 8 commits into
mainfrom
fix-consolidation-import-cycle-237

cdeust commented Jul 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cdeust commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Follow-up work folded into this PR (issue #276, closed by this PR)

Completion Ledger (§13.2)

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cdeust commented Jul 30, 2026 •

edited

Loading