Closed
Conversation
- Collapse `query` subcommands into single command with composable flags (`--ancestors-of`, `--dead-ends`, filter flags). Old subcommand forms replaced. - Unify stdin convention across `render`, `merge`, `query`, `validate`: omit `-i` or pass `-` to read from stdin. `validate --input` now optional. - Extract shared `io` module for document read/write and pretty-print, replacing ad-hoc `fs::read_to_string` + `Document::from_json` + pretty ternaries duplicated across six commands. - Extract `source::require_native` helper and refactor emscripten guards from in-body `#[cfg]` branches to function-level splits — native bodies no longer indented under a `cfg(not(emscripten))` block. - Preserve `haiku` command per CTO mandate. Net diff: -444/+338 LOC. All 161 CLI tests pass; clippy clean.
|
🔍 Preview deployed: https://d7073ccd.toolpath.pages.dev |
Version bump deferred — publishing is manual via scripts/release.sh, so we don't need to bump here. The 0.4.0 entry in CHANGELOG can land with the eventual publish commit.
eliothedeman
changed the title
akesling
approved these changes
Apr 24, 2026
Contributor
akesling
left a comment
akesling
left a comment
There was a problem hiding this comment.
One design consideration/concern, otherwise LGTM
| let (steps, _) = extract_steps(&doc); | ||
|
|
||
| fn apply_filters<'a>( | ||
| steps: &'a [toolpath::v1::Step], |
Contributor
There was a problem hiding this comment.
Won't this mean that actor entries, etc. won't be filtered as we're operating on slice step only and not other sections?
Is that what we were doing already? If so, we can punt but maybe we should fix it while we're here? Up to you.
Collaborator
Author
There was a problem hiding this comment.
Good catch — yes, that was pre-existing behavior (only the first inline path in a Graph was inspected). Fixed in 3d3ca65: collect_paths now returns a view per inline path, and each of --dead-ends, --ancestors-of, and the filter flags iterate all views and union results (deduped by step id). Added regression tests for the multi-path Graph case.
Previously `query` on a Graph only inspected the first inline path — subsequent paths' steps were invisible to `--dead-ends`, `--ancestors-of`, and filter flags. This was pre-existing behavior but the reviewer flagged it as worth fixing while we're here. - Replace `extract_steps` (returned one slice) with `collect_paths` which returns a view per inline path. - `collect_ancestors` / `collect_dead_ends` / `collect_filtered` each iterate all views and union results, deduplicating by step id. - Rewrite filter logic as a single predicate over steps instead of three intersect-by-id passes, trimming allocations. - Add regression tests for the multi-path Graph case: dead-ends union, filter union, ancestors-of lookup across paths, duplicate-id dedup.
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
2 participants
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
Simplify the
pathCLI by collapsingquerysubcommands, unifying stdin conventions, and extracting shared I/O helpers. Net diff in the refactor commit: −444 / +338 LOC.Version bump deferred — publishing is manual via
scripts/release.sh, so thetoolpath-cliversion stays at 0.3.0 for now and the CHANGELOG entry will land with the eventual publish commit.Changes
Breaking (minor)
querysubcommands collapsed into a single command with composable flags:path query ancestors --input f.json --step-id s3→path query -i f.json --ancestors-of s3path query dead-ends --input f.json→path query -i f.json --dead-endspath query filter --input f.json --actor human:→path query -i f.json --actor human:validate --inputis now optional. Omit it or pass-to read from stdin.Non-breaking
render,merge,query,validateall accept-i -(or omit-i) to read from stdin.iomodule centralizes document read/write, stdin/stdout handling, and pretty-print — replaces ad-hocfs::read_to_string+Document::from_json+to_json_prettyternaries duplicated across six commands.source::require_nativehelper centralizes WebAssembly guard messaging. Emscripten guards refactored from in-body#[cfg]branches to function-level splits — native bodies no longer indented under acfg(not(emscripten))block.Preserved
haikucommand andranddependency — CTO mandate.track's 8 sub-ops untouched. Internal refactor (splitcmd_track.rsinto a module) flagged for a follow-up PR.list,derive,render,merge,validatekeep their existing command surfaces.Test plan
cargo test --workspace— all 161 CLI tests pass (137 unit + 12 integration + 12 snapshot), workspace-wide green.cargo clippy --workspace -- -D warnings— clean.derive git | validate -i -— stdin round-trip works.queryforms (--ancestors-of,--dead-ends,--actor) return expected results.path haikustill prints verse (CTO-mandated).Deferred
Internal refactor of
cmd_track.rs(2,433 LOC →src/track/{state,session_io,ops}.rs) is a worthwhile follow-up but was scoped out of this PR — it needs a deeper read before committing.