-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
Performance
B4: Redundant tokenization at cascade/fallback boundary — cascade counts tokens, then✅ Resolved in perf: eliminate redundant tokenization and binary search allocation churn #29truncate_to_token_budgetre-counts the same string. Pass the already-computed count through.B5: Binary search in✅ Resolved in perf: eliminate redundant tokenization and binary search allocation churn #29truncate_to_token_budgetbuilds candidate strings viajoinon every iteration. Use precomputed byte offsets for slice-based indexing instead.- B8: Cascade re-parses tree-sitter AST for each mode. Parse once and reuse across mode transformations (requires core API changes).
- S5 (partial):
cascade_from_herereturns&'static [Mode]now (FIXED), but the cascade loop still creates a config per iteration. - B11: Per-iteration tokenization in binary search —
count_tokensis called O(log N) times inside the binary search loop, each O(N). A prefix-sum approach could reduce total to O(N), with trade-off of cross-line token boundary accuracy. - B12: Fast-path
text.to_string()allocation — when text fits within budget, the function copies the entire string. Could returnCow<str>to avoid this allocation (requires API change). - B13: Split-then-rejoin round-trip —
text.lines().collect()thenlines.join("\n")reconstructs the input. Could scantextdirectly for newline byte positions and use&strslices, eliminating both theVec<&str>and thejoinedString.
Complexity
- B6:
process_filespans 120 lines with 5 responsibilities. Extracttry_cached_resultandrun_transformhelpers.main()spans 178 lines with cyclomatic complexity ~14. Extractvalidate_args()and unify dispatch. - S6:
ProcessOptions(6 fields) andMultiFileOptions(8 fields) have 5 overlapping fields. ComposeProcessOptionsinsideMultiFileOptions. - S13: Three identical validation blocks for
--jobs,--max-lines,--tokensshould be a genericvalidate_bounded_arghelper.
Test Coverage ✅ Resolved
All test coverage gaps were addressed before merge in commit 646bd33:
S7: No direct unit tests for→ 5 unit tests added incascade_for_token_budgetmain.rsS8: Missing test for→--tokenswith--mode=typestest_tokens_with_mode_types_single_mode_cascadeS9: No test for binary search zero-content-lines edge case→test_tokens_very_small_budget_fallback_truncationS10: Cache round-trip test missing for→token_budgettest_tokens_budget_invariant_with_fixture
Minor
- S2: Cache metadata stores starting mode, not actual cascade mode. Misleading for debugging.
- B10: Cache key format change orphans existing entries (acceptable, document in release notes).
- S14:
lib.rs:41comment says "for CLI crate" but function is publicly exported. Clarify API stability intent in doc comment (relevant for v1.0.0 release).
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels