Skip to content

refactor: Phase 4 module splits — canvas/, twin/, twin workspace components, size guardrails - #98

Merged
WKJBryan merged 30 commits into
mainfrom
worktree-refactor-module-splits
Jul 10, 2026
Merged

WKJBryan merged 30 commits into
mainfrom
worktree-refactor-module-splits

Conversation

@WKJBryan

Copy link
Copy Markdown
Owner

Summary

Phase 4 (final phase) of the production-readiness refactor (plan): behavior-identical modularization of the three oversized files, plus guardrails so it stays lean.

  • commands/canvas.rs (4,797 lines) → commands/canvas/ — mod facade + shared/context/streaming/debate/session/test_support. Move-purity mechanically verified (sort-and-diff of non-plumbing content); the twin Simulation gate moved byte-identical; 18/18 IPC commands unchanged, main.rs untouched.
  • services/twin_store.rs (6,797 lines) → services/twin/ — mod facade + shared/traces/records/constitution/decisions/digest/export. Seal-integrity logic byte-identical; 40/40 public methods preserved. One permitted logic fix: three commands/twin.rs read-modify-write commands took read locks and could lose concurrent updates — now write locks, with a concurrency regression test.
  • TwinReviewView.vue (2,409 → 147 lines) — new stores/twin.js Pinia store (all 19 twin API calls funnel through it, per-action loading flags, race-safe toasts), 13 components under components/twin/, namespaced twin-workspace.css (143 = 143 rules, PR Redesign Twin Workspace: fix scrolling, grouped nav rail, task-oriented layout #85 scroll contract preserved verbatim).
  • Guardrails — CLAUDE.md: ~1,500-line authoring target, required write/delete chokepoints (commit_note_write/commit_note_delete/write_atomic), mod-facade split pattern; CI: check-file-sizes.cjs fails the lint job at 2,500 lines (validated at both thresholds).
  • CLAUDE.md accuracy pass — the vault optimizer's "LLM spend" fiction corrected in both places it appeared (the optimizer is rule-based; _llm_enabled is a reserved no-op locked in by a characterization test; caps limit disk writes); structure claims updated; every new claim fact-checked against code with line numbers.

Owner decision (2026-07-10), recorded in the plan: commands/canvas/context.rs at 2,279 lines is accepted above the plan's original <2,000 goal — keeping the twin gate and its 21 tests together outweighs the line count; tripwire set at 2,500.

Test plan

  • Rust 281/281 and Vitest 425/425 — green at every one of the 30 commits, re-run independently by the final reviewer
  • MCP feature-set build clean per twin-split commit; zero new clippy lints (42 baseline)
  • Move purity mechanically verified per split (content sort-and-diff, byte-identical security-adjacent functions, test-count ledgers 30/30 and 50/50)
  • Per-task reviews + whole-branch final review; plan acceptance amended per owner decision
  • e2e suite (manual-only, needs live tauri:dev IPC) — run with Phase 5 verification

🤖 Generated with Claude Code

WKJBryan and others added 30 commits July 10, 2026 13:56
Pure rename ahead of splitting canvas.rs into focused submodules.
No content changes.
Move model-routing helpers (ModelRoute/ModelProviderRoute, resolve_model_route,
effective_model_ids, is_vault_context_prompt, source_tile_context_provider) and
append_canvas_trace into canvas::shared — used by streaming, debate, and session
command handlers alike. Pure move; tests moved verbatim with their code.
Move twin/context prompt-building (build_note_context_prompt,
build_chunk_context_prompt, build_twin_context_prompt,
resolve_twin_prompt_context, the twin Simulation identity gate, and the
conversation-history reconstruction helpers) into canvas::context. Pure
move; the twin gate logic and its tests move verbatim.

Shared test builders (build_response/build_tile) used by both context's
and streaming's test suites are factored into a small #[cfg(test)]
canvas::test_support module rather than duplicated.
Move send_prompt, add_models_to_tile, regenerate_response, and the
per-model stream task helpers (persistence-error emitters, timeout
handling, response classification) into canvas::streaming. Pure move;
tests move verbatim, reusing canvas::test_support for tile builders.

canvas::mod.rs now glob re-exports streaming's tauri commands
(`pub use streaming::*;`) rather than naming them individually — the
`#[tauri::command]` macro emits a hidden `__cmd__<name>` sibling item
that a named re-export wouldn't carry along, which broke
`tauri::generate_handler!` resolution in main.rs.
Move start_debate, continue_debate, and emit_debate_persist_error into
canvas::debate. Pure move; no logic changes. mod.rs now glob re-exports
debate's tauri commands for the same __cmd__ macro-visibility reason as
the streaming extraction.
Move session CRUD, tile/node position updates, viewport, auto_arrange,
and export_to_note into canvas::session. mod.rs is now a thin surface:
module wiring plus glob re-exports from shared/streaming/debate/session,
with context.rs holding the twin/note prompt-assembly internals used by
streaming.
mod.rs already gates the module declaration with #[cfg(test)], so the
inner attribute duplicated it and tripped a new clippy error (43 vs the
42-error baseline). Removing it restores the baseline count.
Renames the services::twin_store module to services::twin (pure move,
no logic change) so Task 4.2's domain submodules can live alongside it.
Updates the 6 external reference sites (main.rs AppState import,
commands/settings.rs, commands/mod.rs test helper, canvas
shared/streaming/context) to the new module path.
Moves the quarantine/load-or-quarantine file helpers and cross-cutting
text/payload utilities (lexical_terms, excerpt, payload_string family,
event-text extraction) into services/twin/shared.rs. These are used by
every remaining domain (records, constitution, decisions, digest,
traces) via `pub(super)` visibility, so mod.rs gains `use shared::*;`
for its still-inline code. Verbatim move; corrupt-file quarantine test
suite (Task 1.5 coverage) moves with load_or_quarantine since it
exercises the shared helper across every domain's listing loop.
Moves session trace append/read (append_trace_event, get_session_trace,
get_or_create_trace_mut), the session-trace listing loop, and
evidence-ref resolution (resolve_evidence_refs, resolve_event_evidence,
event_summary) into services/twin/traces.rs. list_session_traces,
resolve_evidence_refs, trace_file_path, and read_trace_file are marked
pub(super) since decisions.rs (decision_feedback_events) and
records.rs/digest.rs (evidence resolution) still call them across
module boundaries. Verbatim move; tests move with their code.
Moves user record CRUD (list/get/create/update_user_record), promotion
(set_user_record_promotion), context selection (select_context_records),
and behavioral inference (run_twin_inference, SignalAccumulator,
infer_behavioral_records) into services/twin/records.rs. ensure_record_cache
and write_record_file are marked pub(super): constitution/digest/export
still call ensure_record_cache across module boundaries, and the shared
corrupt-file quarantine test suite calls write_record_file directly.
Verbatim move; tests move with their code.
Moves Constitution items, action gaps, guided setup, and constitution
inference (including interview-note extraction and vault-derived
staleness pruning) into services/twin/constitution.rs. constitution_file_path,
read_constitution_file, action_gap_file_path, read_action_gap_file, and
normalize_key_text are marked pub(super)/kept accessible: decisions.rs's
build_decision_evidence_packet reads constitution/action-gap labels by
id, and digest.rs's memory_digest_cluster_key falls back to
normalize_key_text. Verbatim move; tests move with their code.
Moves decision episodes (record/list/update_decision_outcome), sealed
twin predictions (attach_twin_prediction, mark_twin_prediction_failed,
parse_twin_prediction and its parsing helpers), reflection cards
(record_reflection_card, scoring), and the decision-mirror config into
services/twin/decisions.rs. The seal-integrity logic and its comment
block move verbatim -- attach_twin_prediction still refuses to overwrite
a sealed prediction or seal one after the outcome is recorded, and
list_decision_episodes_with_reflections still redacts an unrevealed
sealed prediction before it crosses IPC. `parse_twin_prediction` is
re-exported via `pub use decisions::parse_twin_prediction;` in mod.rs
so commands/canvas/context.rs's import path is unchanged. Verbatim
move; tests move with their code.
Moves memory digest clustering and review (list_memory_digest,
review_memory_digest_item, clustering/staleness helpers) into
services/twin/digest.rs. Verbatim move; tests move with their code.
Moves the JSONL/manifest export bundle (export_bundle, per-split-bucket
helpers) into services/twin/export.rs. mod.rs now shrinks to the
TwinStore facade: struct fields, new(), write_pretty_json, and
validate_file_id -- the handful of helpers used by every domain
submodule, kept here so they're visible to all descendants without
pub(super). Public API is unchanged from the pre-split twin_store.rs.
Verbatim move; tests move with their code.
The domain split left each file with the full pre-split kitchen-sink
`use crate::models::twin::{...}` block; most entries were dead weight
once each domain only needed its own subset. Trims every submodule down
to what it actually references, cfg(test)-gating symbols that are only
used by that file's own test module so `cargo build --features mcp`
(which doesn't compile tests) stays warning-free too. Zero behavior
change; cargo test (280/280), the mcp build, and clippy (still ~42
pre-existing warnings, none added) all confirm this is imports-only.
update_decision_mirror_config, create_constitution_item, and
update_constitution_item took twin_store.read() despite update_constitution_item
and update_decision_mirror_config doing a read-modify-write file sequence
(read current file, apply the update, write the whole thing back). Two
concurrent calls under a shared read lock can both read the same
pre-update state; the second writer then silently clobbers the first's
change. Switches all three to .write() so the read-modify-write sequence
is fully serialized.

Adds a regression test demonstrating that two concurrent
update_constitution_item calls to the same item -- one changing claim,
the other priority -- both persist under the write lock instead of one
clobbering the other.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the Pinia store, shared formatting utils, and shared stylesheet that
TwinReviewView.vue's tabs will be migrated onto in follow-up commits
(Task 4.3). Not yet wired into the view.
Moves the static Guide tab (walkthrough + button reference) out of
TwinReviewView.vue verbatim. No store dependency for this tab.
…onfigTab

All Twin Workspace state and API calls now live in stores/twin.js — the
view's template reads/writes twinStore.* instead of local refs, and the
Config tab (presets, advanced weights, export benchmark) is extracted into
its own component that consumes the store directly. The CSS that was a
single scoped block covering all tabs moves to a shared, non-scoped
stylesheet (components/twin/twin-workspace.css) with every selector
namespaced under .twin-workspace, since Vue's scoped-style attribute only
reaches a child component's root element, not markup nested inside it —
see task-4.3-report.md for the full rationale. Existing view/component
specs updated to provide an active Pinia instance.
Moves the Setup tab (twin identity + operating priors form) out of
TwinReviewView.vue verbatim, consuming the twin store directly.
Moves the Constitution tab out of TwinReviewView.vue verbatim, and promotes
the ReviewActions render-function component to its own file since the
Constitution, Action Gaps, and Memory Review tabs all use it.
…ents

Moves the Action Gaps tab and its per-gap render-function row out of
TwinReviewView.vue verbatim.
Moves the Decisions tab and the decision-card render-function component
(context trace, sealed predictions, outcome recording) out of
TwinReviewView.vue verbatim. Route-derived highlight/trace-open state is
computed inside the tab from the same route query.
Moves the Memory Review tab (adaptive digest + user records with promotion
actions and evidence drawer trigger) out of TwinReviewView.vue verbatim.
…ents

Moves the Overview tab (metric tiles, tutorial intro, recent decisions,
current action gaps) and the cross-tab evidence drawer overlay out of
TwinReviewView.vue verbatim. The view is now a 147-line shell: header,
grouped nav rail, tab routing, scroll ownership, and the save toast.
…ehavior

Fixes the vault-optimizer "prevents runaway LLM spend" claim (it has no
LLM call path; proposals are rule-based and the caps limit disk writes,
locked in by a characterization test) and updates structure references
left stale by the module splits: commands/canvas.rs -> commands/canvas/,
services/twin_store.rs -> services/twin/, Pinia stores (3) -> (4) with
twin.js, components/twin/, and TwinReviewView.vue as a thin shell. Adds
a Note Write & Delete Chokepoints section documenting commit_note_write/
commit_note_delete, atomic_io::write_atomic, index_commit.rs, and the
canonical lock order, plus a size-convention entry under Working
conventions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a size tripwire so the module-split work in this refactor doesn't
silently regress: frontend/scripts/check-file-sizes.cjs walks
src-tauri/src/**/*.rs and src/**/*.{vue,js}, excluding test files by
path convention (__tests__/, *.spec.js, Rust test-only filenames), and
fails with a sorted list of offenders past the threshold. Wired into
the lint job via a new npm run check:file-sizes step. Current largest
file (commands/canvas/context.rs, 2280 lines) has ~220 lines of
headroom before this trips.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…larify tripwire comment

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@WKJBryan
WKJBryan merged commit d193d4e into main Jul 10, 2026
10 checks passed
@WKJBryan
WKJBryan deleted the worktree-refactor-module-splits branch July 10, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant