cc-warehouse - architecture review board

reports.py · cli.py · sweep.py · build.py · migrate.py · relocate.py

Problem. The deep lock primitive has a two-function bool interface, but four verbs each re-wrap it with an identical sentinel-triple plus acquire-or-refuse plus finally-release, and cli.py mirrors the report-and-tally half four more times (_run_sweep :310-318, _run_build :375-383, _run_migrate :757-768, _run_relocate :882-908). reports.py is a 19-line shell owning only the error filter.

Solution direction. A with_lock(root, name) -> BatchReport runner plus a shared render_batch(report) -> exit_code in reports.py owning the sentinel-triple, the release, the per-action counts and the exit policy.

Contract ties: R9, R10; R14 untouched · Tier: VERIFIED at 1517bba

render.py · build.py · parser.py

flowchart LR
  A[one payload] --> M[render_markdown 892-893]
  A --> H[render_html 1813-1814]
  A --> B[build_manifest 1829]
  M --> P1[build_conversation + parse_session]
  H --> P2[build_conversation + parse_session]
  B --> P3[build_conversation]
            

Problem, RE-LOCATED by the review. Parse-once is honored INSIDE parser.py: parse_session (:161) and build_conversation (:597) both route through one _extract_entries (:122). The "both re-extract" hypothesis is refuted. The real violation is the orchestration seam: build._projection_files (:87-100) invokes all three emitters, each taking raw bytes and privately re-deriving the model, so one projection re-walks the entries build_conversation x3 plus parse_session x2. sha256_hex is likewise recomputed at render.py :894/:1816/:1831.

Solution direction. Hoist parsing to the orchestrator: build the models once in _projection_files and pass them into the three emitters, keeping a thin bytes wrapper for the black-box tests.

Contract ties: R8, R9; DESIGN section 4 · Tier: VERIFIED at 1517bba

build.py · share.py · cli.py · catalog.py

Problem. The head predicate s.hash NOT IN (SELECT supersedes ...) is typed verbatim in _heads (build.py:143) and head_for_short (:173), and the whole SELECT column list is duplicated too (:141-142 vs :170-171) - while head_for_short's docstring (:167) claims "one owner, R9", an R8/F6 overclaim. _Resolved re-declares the head shape in share.py:75.

Solution direction. One _HEAD_SELECT / _HEAD_WHERE fragment and a public head-record type both callers compose; reads stay catalog-only (R6).

Contract ties: R6, R8, R9 · Tier: VERIFIED at 1517bba

cli.py · build.py · registry.py · relocate.py

Problem. Verb logic lives in cli.py that should live in the verb's module: the render no-row-vs-superseded policy stranded in _render_session (:431-444) while build.head_for_short owns only the head selection; project-exists SELECTs inline in _run_project (:585-682) though registry.py owns the edits; _out_under_warehouse (:475-491) a CLI-only guard that the two non-CLI callers of write_projection lose; and halted-run recovery string-matching relocate action detail (:886, :894-905).

Solution direction. Verbs own their reads and guards and return a typed result; registry.py gains the project read side; the write-guard moves into write_projection; relocate exposes typed accessors so cli never string-matches.

Contract ties: F9 · overlaps C2 and C4 · Tier: VERIFIED at 1517bba

sweep.py · migrate.py

Problem. sweep._walk_source (:44-76) and migrate._walk (:42-82) are the same scaffold verbatim - the is_dir guard, the _on_error body, os.walk(onerror), the .jsonl filter, the sorted return - and even the _JSONL_SUFFIX constant is declared twice (sweep :36, migrate :21). The only real divergence is the per-file policy.

Solution direction. One walk_transcripts(root, *, classify) owning the scaffold, each verb supplying only its filename policy.

Tier: VERIFIED at 1517bba

render.py

Problem. The turn traversal is written twice. Markdown: _turn_body (:730-746) and _render_turn (:789-805). HTML: _claude_inner (:1617-1630) and _turn_html (:1633-1697). Deliberately partial: leaf block rendering IS single-owner via _render_block (:356) - only the walk is doubled.

Solution direction. One turn-walk yielding typed events (Phase / Reply / UserHalf), each emitter supplying only leaf rendering.

Contract ties: R9 · pairs with C1 · Tier: VERIFIED at 1517bba

parser.py · render.py

Problem. Block.kind (parser.py:291) is a bare str whose legal set is only in a docstring, switched on at about seven sites (parser _segment_category :356-369, group_segments :394; render _render_block :356-384, _ROW_ICONS :1479-1492, _row_label :1497-1528, _row_icon :1531-1537). A new kind compiles clean while several render switches fall through silently - the F6 class, invisible to pyright strict.

Solution direction. A typed discriminant (Literal or enum, stdlib, R7-safe) driving render dispatch through mappings, so a missing arm is a type error.

Contract ties: F6, R7 · Tier: VERIFIED at 1517bba

catalog.py

Problem. _short_key (catalog.py:131-148) decides whether a 12-hex citation key must extend and to what length - pure string math (the prefix band at :137-140) - but it is welded to conn.execute at :142-145, so the only seam to test the extension rule is standing up a database with a colliding row.

Solution direction. Split the pure "choose a length given existing prefixes" function from the collision lookup. Same public-pure-transform shape as C3 in a lower-risk module.

Contract ties: F5 (keep the PK-index band query) · Tier: VERIFIED at 1517bba, surfaced by the pipeline lens