perf(tui): hot linear scans → map lookups (#4152)

[Hmbown]

Fixes #4152

Summary

Perf-only (behavior-identical) pass over the three candidate files for issue #4152: replace hot linear Vec/slice scans that are repeatedly queried by a stable key with an O(1) map lookup, where — and only where — building the map genuinely amortizes.

One genuinely hot site was converted; the other candidate scans were audited and left as linear scans because converting them would be neutral or slower.

Per-file audit

crates/tui/src/core/engine/tool_catalog.rs — CONVERTED

• should_default_defer_tool → membership over DEFAULT_ACTIVE_NATIVE_TOOLS. Was !DEFAULT_ACTIVE_NATIVE_TOOLS.iter().any(|core_tool| core_tool == &name) — a linear scan of a 28-element array. apply_native_tool_deferral calls this once per native catalog tool, and the catalog is rebuilt every turn (build_model_tool_catalog_with_surface, called from engine.rs handle_deepseek_turn), so the real cost was O(catalog_len × 28) per turn. Converted to an O(1) contains against a process-lifetime OnceLock<HashSet<&'static str>> side index (default_active_native_tools_set()). The OnceLock builds the set once for the whole process, so it amortizes across every turn/tool rather than per-call.
• Ordering preserved: the DEFAULT_ACTIVE_NATIVE_TOOLS array is kept as the source of truth for ordered iteration (tool_catalog_consistency_issues iterates it; engine::default_active_native_tool_names() returns it). Only a side HashSet is added; the membership test carries no ordering dependency, so results are hit/miss-identical.
• Left as-is: catalog_contains_tool (used in unavailable_core_action_tools_*) loops over only 4 cached fallbacks, and runs solely on a tool_search invocation — not hot enough to justify a per-call name set. tool_catalog_consistency_issues, active_tool_list_from_catalog, and initial_active_tools already use HashSets.

crates/tui/src/features.rs — LEFT AS-IS

• Feature::info, is_known_feature_key, feature_from_key, feature_spec_by_key all scan FEATURES, but it is a 7-element compile-time constant and the lookups run at config-load / deserialization time, not per-turn. apply_map loops config entries calling feature_from_key, but over 7 static entries and once at load. Building a HashMap for 7 entries queried a handful of times would be slower, not faster. No hot repeated-key path here.

crates/tui/src/core/engine/turn_loop.rs — LEFT AS-IS

• resolve_tool_definition (tool_catalog.iter().find(|d| d.name == …)) is called once per tool in the per-batch tool_uses loop. Batches are frequently a single tool call, so a per-batch HashMap build would be a net loss for the common case (the issue explicitly says one-off lookups should stay linear scans). Left as a linear scan.
• command_allows_tool / command_denies_tool scan user allowed_tools/disallowed_tools rule lists, but matching supports trailing-* wildcards (rule.strip_suffix('*')) — there is no pure stable key to hash, so a set/map is not applicable.
• fold_tool_call_before_results, the plans.iter().any/filter, and content_blocks.iter().any scans are each one-off passes over a batch, not repeated keyed lookups.

Tests

Added default_defer_lookup_matches_linear_scan_over_active_native_tools (in core/engine/tests.rs): asserts the new side-set lookup returns the same hit/miss as an explicit linear scan over the ordered array — every array member is a hit (not deferred), and names outside the array (plus request_user_input) miss (deferred). The pre-existing core_native_tools_stay_loaded_in_yolo_mode / non_yolo_mode_retains_default_defer_policy tests continue to exercise the converted function.

Verification (CI gate, from worktree root)

• cargo build -p codewhale-tui --locked — ok
• cargo test -p codewhale-tui --all-features --locked — ok except the known skill_hotbar_action_* env skill-cache flake
• cargo fmt --all -- --check — clean
• cargo clippy --workspace --all-features --locked -- -D warnings … — clean
• cargo test --workspace --all-features --locked — ok except the same known skill_hotbar_action_* flake
• cargo build --release — ok

Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 1m 42s —— View job

---

Review: perf(tui): hot linear scans → map lookups (#4152)

• Gather context (PR diff, description, changed files)
• Understand the request (code review)
• Review tool_catalog.rs changes for correctness
• Review tests.rs changes for coverage
• Check provider/model route-safety invariant (not touched by this PR)
• Check for reuse/simplification opportunities
• Post final review summary

Summary

This is a small, well-scoped, low-risk perf change. I reviewed the full diff (crates/tui/src/core/engine/tool_catalog.rs +19/-3, crates/tui/src/core/engine/tests.rs +35) against origin/main. No correctness bugs found.

tool_catalog.rs (tool_catalog.rs#L128-156)

• default_active_native_tools_set() follows the exact same OnceLock-backed lazy-init pattern already used a few lines up for cached_fallbacks() (line 108-126), so it's consistent with existing code style rather than introducing a new idiom.
• should_default_defer_tool correctly preserves early-return semantics (always_load check, then is_tool_search_tool) before falling through to the new O(1) membership check — behavior-identical to the old .iter().any(...) scan.
• DEFAULT_ACTIVE_NATIVE_TOOLS (the ordered array) is left untouched and still used for ordered iteration elsewhere (tool_catalog_consistency_issues, engine::default_active_native_tool_names) — the PR correctly adds a side index rather than replacing the source of truth, so no ordering-dependent call site is at risk.
• HashSet<&'static str>::contains(&str) works via Borrow<str> for &'static str — no type issue.

tests.rs

• The new default_defer_lookup_matches_linear_scan_over_active_native_tools test is a solid parity guard: it independently re-derives a linear scan over the ordered array and asserts identical hit/miss against the converted lookup, for both array members and several known non-members (including REQUEST_USER_INPUT_NAME). This directly covers the risk of this kind of refactor (hash/linear-scan divergence).
• Pre-existing core_native_tools_stay_loaded_in_yolo_mode / non_yolo_mode_retains_default_defer_policy tests still exercise the converted function and were left intact.

Scope discipline: The PR body's per-file audit for features.rs and turn_loop.rs (left as linear scans, with specific reasoning — small constant array, one-off per-batch lookups, wildcard matching that can't be hashed) is accurate based on reading those call sites, and the PR correctly doesn't touch them. Good restraint — not over-converting scans that wouldn't actually benefit.

Provider/route safety: N/A — this PR touches only local tool-catalog deferral logic, no model/provider selection code.

Reuse/dead code: None found. No new dependencies added; reuses the existing OnceLock idiom already present in the same file.

No blocking issues. This looks safe to merge from a correctness standpoint.