refactor: split action/mod.rs + worker/event/autocomplete test coverage#55
Merged
Conversation
Pulls ~280 lines out of action/mod.rs into two cohesive submodules that already had a clear seam — the self-update flow and the editor-session lifecycle (switch/new/delete + debounced save). mod.rs is still the dispatch entry point; the public surface (action:: try_promote_pending_update, action::flush_session, action:: schedule_session_save) is preserved via re-exports. First chunk of the action/mod.rs split called out in the code review.
Adds a focused test module to src/worker/mod.rs covering the previously untested infrastructure boundary: clean exit on Close, clean exit on command-channel drop, no-connection guards (Cancel/Reset/Execute/ Introspect), Connect failure surfacing, and a sqlite Connect + Execute + Reload smoke test that also asserts SchemaCache mutation.
Adds App-backed integration tests to src/event.rs covering the overlay precedence rules in translate_key: Help over Normal, Command over modal screens, Connecting swallowing all keys (but Ctrl+C still quits), ConfirmRun/UpdateAvailable/ConfirmToolUse key bindings, overlay swap mid-session, conn-form screen input routing, and bracketed-paste overlay routing.
Moves the large `all()` schema-vector and the `function_tool()` builder into src/llm/tools/schemas.rs. `tools.rs` keeps the constants, mode filtering, and dispatch logic; the prompt-engineering surface lives next door so wording tweaks don't clutter the dispatcher's git log. tools.rs: 1638 -> 1392 lines.
Adds 9 parameterized tests for the gaps called out in the code review: subquery-in-FROM, subquery-in-JOIN, scalar subquery in projection, UNION/EXCEPT/INTERSECT binding scope, three-way join alias collection, chained CTEs in outer scope, PostgreSQL double-quoted identifiers, MySQL backtick identifiers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses all high-leverage and most medium-priority findings from the recent architectural review.
High-leverage (3/3)
src/action/mod.rssplit — 3074 → 1594 lines (48% reduction) via 5 cohesive submodules:action::update,action::session,action::query,action::schema,action::results. Each handles one domain end-to-end;mod.rsstays the dispatch entry point withpubre-exports preserving the existing API (action::flush_session,action::schedule_session_save,action::try_promote_pending_update).src/worker/mod.rscovering the previously untested infrastructure boundary: clean exit onClose, clean exit on command-channel drop, no-connection guards (Cancel/Reset/Execute/Introspect), Connect-failure surfacing, sqlite Connect + Execute + Reload smoke test that also assertsSchemaCachemutation.src/event.rscovering modal-stack interactions: Help-over-Normal, Command-over-modal, Connecting-swallows-all-keys (but Ctrl+C still quits),ConfirmRun/UpdateAvailable/ConfirmToolUsebindings, mid-session overlay swap, conn-form screen routing, bracketed-paste overlay routing.Medium
Arc<RwLock<SchemaCache>>ordering rules insrc/app.rs: worker writes / main reads, never held across.await, never nested with another lock.llm/tools.rssplit — Extractedall()schema vector +function_tool()builder intosrc/llm/tools/schemas.rs. tools.rs: 1638 → 1392 lines; prompt-engineering surface is now isolated from dispatch logic.src/autocomplete/context.rsfor the review's gap list: subquery-in-FROM, subquery-in-JOIN, scalar subquery in projection, UNION / EXCEPT / INTERSECT binding scope, three-way join, chained CTEs, PostgreSQL"Quoted"identifiers, MySQL backtick identifiers.Skipped — with reasoning
state/schema.rssplit. Re-read the file: 882 lines = ~440 prod + ~440 tests.SchemaPanelis one cohesive abstraction;visible_rows()traverses the tree directly vianodes[].children, so there's no clean tree/view seam. Splitting into two structs would force touching everyapp.schema.<field>call site acrossaction/,event/,ui/for marginal cohesion gain. Deferred — not worth the churn.Version
Bumped patch:
0.16.2→0.16.3.Test plan
cargo test --bin rowdy— 603 passed (was 575 before this branch).cargo clippy --all-targets --no-deps— clean.cargo checkclean after each step (no half-built intermediate states left in the branch).