Refactor: introduce shared tree-sitter traversal helpers (A / with sview) - #9
Closed
jolestar wants to merge 1 commit into
Closed
Refactor: introduce shared tree-sitter traversal helpers (A / with sview)#9jolestar wants to merge 1 commit into
jolestar wants to merge 1 commit into
Conversation
Extract the duplicated parser setup, recursive AST walk, and preview generation from rust.rs and javascript.rs into a new traverse.rs module. The shared layer provides: - NodeClassifier trait: language-specific node classification with an observe_sibling hook for sibling-level state (e.g. Rust #[test] attribute tracking) - analyze_tree_sitter: parse + traverse + build_tree in one call - ast_preview: shared first-line preview extraction - collect_items: recursive named-children walk with FlatItem collection Language-specific logic (node kind matching, naming, impl signature extraction) remains local to each language module. Closes #2
Contributor
Author
|
Superseded by #8, which was selected as the A/B winner: same passing behavior with fewer files changed, lower abstraction overhead, lower token usage, fewer model rounds, and shorter elapsed time. Closing this experimental candidate. |
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
1 participant
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.
A/B analysis report — issue #2
Task: refactor
sviewto introduce shared tree-sitter traversal helpers for the Rust and JavaScript/TypeScript analyzers.Candidates
sviewskillcargo fmt --check,cargo testpassedsviewskillcargo fmt --check,cargo testpassedImplementation comparison
src/traverse.rsmodule with aNodeClassifiertrait and mutable sibling-observation hook, then wires Rust and JavaScript analyzers through it. This is flexible, but adds a second tree-related module next to the existingsrc/tree.rsand more abstraction surface.src/tree.rsmodule withparse(),collect_items(), andast_preview(), then keeps language-specific classification local tosrc/rust.rsandsrc/javascript.rs. This is smaller and aligns with the shared tree-building helper introduced by the previous refactor.sview observations
A reports that
sviewprovided a compact structural map and helped identify duplicated parser/traversal/preview boundaries. However, issue #2 still involves small source files, so thesviewoverview did not translate into lower runtime cost: A used ~42% more tokens and 20 more model rounds than B, with similar self-counted tool-call volume.Decision
Recommendation: merge B (#8) and close/supersede A (#9).
Rationale: B achieves the same behavior with fewer files changed, less new abstraction, lower token cost, fewer model rounds, and about half the elapsed time. A is a useful experiment sample showing that
sviewhelped orientation, but this task size still did not produce a measurable efficiency win.