FRESH REVIEW 2026-08-23 · snapshot: master 4824098 (257 commits) · rendered from SOURCE.md by /architecture
This review supersedes the 2026-07-24 one at 1517bba, which was not decayed but DESTROYED - the repository was deleted and re-created on 2026-08-10 (ticket 28.20), so that commit no longer exists anywhere and could not be diffed against. Every file:line below was RE-DERIVED at 4824098 and re-verified first-hand, so the decay banner this page carried is retired.
Method: the named review skill is not enabled in this session, so 5 parallel read-only agents substituted for it - the same lens split as before, plus a new lens for archive.py, a 900+ line module that did not exist at the last review. Two findings turned out to be live bugs, not architecture debt, and were fixed the same session (see C12 and C6).
Build state: v1 is CLOSED. Since the last review, a major archive-first rewrite landed (tickets 19/21/25/27/29/30) that this board had never looked at until now.
The contract in contract/ is locked and is never edited by this board. A lens report is EVIDENCE to reconcile, never a settled finding - the two consequential claims below were independently re-read from source, not trusted from an agent report alone.
In plain English.
This page lists the code changes most worth making, ranked by how much they'd help. This time, the review found something better fixed than filed: the same small bug (a saved file quietly not getting updated when it should) had already been caught and fixed once earlier the same day, in one of three near-identical copies of the same logic. The review found the SAME unfixed bug hiding in a second copy - and fixed it on the spot, along with a separate real gap where a safety check that exists for one command was missing on another. Both are done. The bigger, still-open idea both point at: stop keeping three copies of the same rule in the first place.
Top recommendation (re-ranked this review)
C12 - One replace-if-larger primitive, not three copiesPROPOSEDStrong
NEW this review, and it moved straight to 1st because it is not argued, it is PROVEN: the same defect (equal size silently read as equal content, F1) was found and fixed once in write_session_folder earlier this session, then found AGAIN, independently, still live in the sibling write_subagent - worse there, since a sub-agent has no manifest to even record the loss. Both specific bugs are now fixed; the reason a second copy could still be broken - the rule lives in three places kept in agreement by hand - is not.
The board - re-ranked 2026-08-23
1st · C12 (NEW) - One replace-if-larger primitive, not three copies
PROPOSEDStrong
archive.py
write_session_folder
equal-size bug FIXED ticket 30 (earlier today)
write_subagent
SAME bug found live, FIXED this review (no manifest to record it in - F6)
write_source
left as-is: write_session_folder always runs after it on the same bytes and decides what renders
Problem. Three inline copies of "larger replaces, smaller is refused, equal size means equal content." That last clause was WRONG (R1/F1: size only answers ordering, never identity) and got fixed in one copy by ticket 30 earlier the same session. This review found the identical unfixed defect independently in a second copy, write_subagent - worse there, since sub-agents have no manifest.json, so the loss was not mis-rendered, it was invisible.
Fixed this session, both specific bugs.
write_subagent now compares bytes at equal size (a new SubagentResult.refused_equal_size field). write_source's stale comment, which claimed to follow write_session_folder's rule "exactly," was corrected. The CANDIDATE itself - one shared rule instead of three copies - is not done.
Solution direction. One shared _replace_if_larger(path, data) -> ReplaceOutcome all three callers use, so a future fourth defect in this rule is fixed once, not remembered and reapplied three times under pressure - which is exactly what nearly happened here.
Contract ties: R1 (as amended), R5, F1, F6 · Tier: VERIFIED at 4824098 (both bugs independently re-read from source, not trusted from the agent report)
2nd · C6 - cli.py: verbs hand back results, not cursors
Problem (a, b, d unchanged; c had a live consequence). (a) render no-row-vs-superseded policy stranded in _render_session (cli.py:863-920). (b) project-exists SELECTs inline in _run_project (cli.py:1036-1097) though registry.py owns the edits. (d) already fixed by an earlier, unrelated change - relocate's halted-run recovery now uses a typed accessor, confirmed by reading current code. (c) _out_under_warehouse (cli.py:933-949) guards write_projection at exactly one of five call sites - the two in share.py (both force=True) had none.
This was live: ccw share --out could write into the warehouse store (F9).
Confirmed by running it: ccw share s:<key> --out <warehouse>/objects/x wrote the file, no error. An unguarded, force=True path into content-addressed storage.
Fixed this session.
_run_share now calls the same _out_under_warehouse guard before writing. The structural problem - the guard lives at the CLI layer, not inside write_projection itself, so a SIXTH call site could reintroduce the same gap - is not fixed, only this session's instance of it.
Solution direction. Verbs own their reads and guards; the write-guard moves INTO write_projection itself so every caller, present or future, gets it automatically.
Contract ties: F9 · overlaps C2/C4/C12 · Tier: VERIFIED at 4824098 (the live gap independently confirmed by execution)
3rd · C3 - Make the risky transforms public and I/O-free
4 public entry points now (share/prepare/commit/discard), all still I/O-only
Cost, confirmed still real. Zero in-process private-helper calls across 8 test files now (5 for relocate, 3 for share, including the newer --EXPOSED path). Every exercise of this logic is a run_ccw subprocess.
Solution direction. Promote thin public transforms - redact_payload(text, patterns), scan_secrets(text), rewrite_bytes(path, text, patterns) - so tests assert against the transform directly.
Tier: VERIFIED at 4824098 · Why 3rd (was 1st): unchanged Strong reasoning, but C12/C6 both now have a PROVEN live consequence this review; C3's cost is real but has not (yet) caught a live defect.
4th · C1 - Parse once: one parse product
PROPOSEDStrong, worse
render.py · build.py · archive.py
flowchart LR
A[one payload] --> M[render_markdown 1185-1199]
A --> H[render_html 2187-2204]
A --> B[build_manifest 2238-2295]
A --> W[write_session_folder 603/652/663]
M --> P1[build_conversation + parse_session]
H --> P2[build_conversation + parse_session]
B --> P3[build_conversation]
W --> P4[parse_session x2 + sha256 again]
W -.iter_projection_files.-> M
Problem, grown since last review. render.py's own redundancy is unchanged (build_conversation x3, parse_session x2, sha256 x3 per write). NEW: archive.py's write_session_folder parses/hashes again on top of that, and build.build() can call BOTH write_projection and write_session_folder for the same payload when both trees are configured - up to build_conversation x6, parse_session x5, sha256 x7 per head. Multiplicative, not additive.
Solution direction. Hoist parsing to the orchestrator; build models once, pass into every emitter and into archive.py's own decision points.
Problem, grown from 4 to 6 copies. Two new modules since the last review each added their own lock-acquire shell instead of reusing one. cli.py's report/tally half also grew from 4 to 6 instances, plus a 7th partial in _run_reindex.
Solution direction. Unchanged - a with_lock(root, name) -> BatchReport runner plus a shared render_batch(report) -> exit_code in reports.py.
thinking_withheld (parser.py:641, ticket 20) is a real Block kind, handled correctly in render.py:576 - but was never added to the Block docstring's enumeration (parser.py:439-446). The exact drift this card warns about, already happened.
Fan-out is larger than described: beyond the ~7 originally-named sites, a census found kind comparisons at roughly 15+ sites, clustered in _phase_meta (render.py:694-761) alone.
Solution direction. A typed discriminant (Literal or enum, stdlib, R7-safe) driving dispatch through mappings, so a missing arm is a type error.
Contract ties: F6, R7 · Tier: AGENT-REPORTED at 4824098
7th · C13 (NEW) - folder_is_current: pure decision logic welded to the filesystem
PROPOSEDWorth exploringNEW 2026-08-23
archive.py
Problem._current_manifest (archive.py:454-499) fuses a file read with three PURE dict comparisons (source_hash, config, renderer_version) that decide "is this folder current." No way to unit-test a renderer_version mismatch without writing 5 real files first - the same shape C11 already names, in a module every incremental build/migrate call goes through.
Solution direction. A pure _manifest_matches(manifest: dict, source_hash, options) -> bool taking an already-loaded dict.
Same shape as C11 · Tier: AGENT-REPORTED at 4824098 · archive.py's first appearance on this board
8th · C4 - Give the head concept one home: a catalog read seam
PROPOSED, mostly resolveddowngraded from Strong
build.py · catalog.py · share.py
The headline finding is CLOSED - not by this board, by ticket 29 mechanism 1 (2026-08-20).
The head predicate, join, and ranking are now ONE shared fragment, _HEAD_RANK_CTE (build.py:323-334), used by both _heads() and head_for_short(). The false "shares the join" docstring the last review flagged is now TRUE.
What remains: a small cosmetic duplication (each function's own final SELECT + row-unpacking, ~15-20 lines). New, unrelated: archive._session_rows (:835-861) overlaps the head query on two columns - already self-flagged in its own docstring, not deduplicated.
Contract ties: R6, R8, R9 (the R8/F6 overclaim risk is now resolved) · Tier: VERIFIED at 4824098
9th · C5 - One turn walker, two serializers
PROPOSEDWorth exploring
render.py
Problem. The turn walk is still written twice: markdown via _turn_body (:999-1015) + _user_md (:1018-1036); HTML via _claude_inner (:1984-1997) + _turn_html (:2000-2072). Leaf rendering confirmed still single-owner via _render_block. Correction: the old citation _render_turn no longer exists by that name.
Solution direction. One turn-walk yielding typed events, each emitter supplying only leaf rendering.
Contract ties: R9 · pairs with C1 · Tier: AGENT-REPORTED at 4824098
10th · C9 - One source walker, two policies
PROPOSEDStrong
sweep.py · migrate.py · relocate.py
Good news: the worried-about THIRD copy did not happen. migrate._walk was made public as migrate.walk_jsonl specifically so ticket 25.4's import_tree.py could reuse it - confirmed, no new os.walk there.
Problem, unchanged. sweep.py and migrate.py's walker scaffolds are still near-verbatim duplicates. New adjacent finding: relocate._scan_content (:298-413) has its own THIRD walker of the same general shape, serving a different job.
Solution direction. One walk_transcripts(root, *, classify) owning the scaffold.
Tier: AGENT-REPORTED at 4824098
11th · C14 (NEW) - No owned 3-way session/subagent/not-a-session discriminant
PROPOSEDWorth exploringNEW 2026-08-23
archive.py · import_tree.py
Problem. Only two booleans exist (is_subagent, is_session); the one caller needing the 3-way classification, import_tree._kind (:84-101), reinvents it as a plain str compared by literal equality - the C7 shape, at lower stakes (one caller today).
Solution direction. A small classify(data) -> Literal[...] combinator any future second caller can share.
Same shape as C7 · Tier: AGENT-REPORTED at 4824098
12th · C10 - A catalog read-scope for relocate
TICKETED-12b
relocate.py
Unchanged verdict, re-verified. The O(N)-per-candidate churn is still gone (_encoded_moves, :464-517). What remains is O(1) per-apply churn in _project_for_cwd/_encoded_owner, called from _preflight with no live connection available to hand it - constant per apply, not a scaling bug.
Contract ties: F5; ticket 12b list · Tier: VERIFIED + CONTRACT at 4824098
13th · C11 - Split the short-key band math from its query
PROPOSEDWorth exploring
catalog.py
Unchanged._short_key (catalog.py:146-163) still welds the pure prefix-band math to the conn.execute collision query in the same loop iteration.
Solution direction. Split the pure "choose a length" function from the collision lookup. Same shape as the new C13.
Contract ties: F5 · Tier: VERIFIED at 4824098
In plain English.
Thirteen ideas, two of them brand new, one mostly finished by an unrelated fix, and two real bugs already patched along the way. The clearest theme: this codebase has three or four places where the exact same small rule gets copy-pasted, and every time that has happened, at least one copy has eventually drifted or been forgotten.
Verified healthy - the deep end, leave it alone
The store foundation.store.py hides O_EXCL takeover races and atomic writes behind put / get / acquire_lock / verify_walk. Deep by construction.
The ingestion seam.capture_transcript is one deep leverage point with three adapters (hook, sweep, migrate).
The parser and render public seam.parse_session, build_conversation, render_markdown, render_html and build_manifest are public and tested in-memory. This is the positive model C3 asks the two risky verbs to match.
The oracle suite discipline.Black-box only, the F5 zero-object-reads negative proven with an audit hook, atomic_write fault-injected cleanly.
atomic_write mode preservation.relocate exposed a silent permission reset present since slice 01; fixed once in the one primitive, every caller inherited it.
C8 page-chrome seam - BUILT (2026-07-24).The RenderOptions.hljs mode plus render._hljs_block, with share setting inline at both call sites, implemented DESIGN 15 item 8.
The head predicate's duplication and its false docstring - CLEARED 2026-08-20.Fixed by ticket 29 mechanism 1, not by this board. See C4 for what small piece remains.
Not on the board - owned elsewhere
The remaining hand-rolled flag scanners in cli.py
Ticket 13 closed WITHOUT removing them; two survive deliberately (_sweep_source, _render_flags) and now document why.
config.toml parsed in three modules - RESOLVED 2026-07-24
config.py is now the only module in src/ importing tomllib.