Code review outline panel implementation

token
main
Smart (Opus 4.6)
Unlisted
Continuing work from thread T-019c7342-a5df-7201-86ae-198e1c9aebb4. When you lack specific information you can use read_thread to get it. @src/outline/extract.rs @src/view/mod.rs @src/update/outline.rs @src/outline/mod.rs Other relevant files: src/model/ui.rs, src/messages.rs, src/view/hit_test.rs, src/runtime/app.rs, src/syntax/parser.rs, src/model/document.rs I have implemented the core of the Code Outline panel feature. This includes: * Created the `src/outline` module with data structures in `mod.rs` and extraction logic in `extract.rs`. * The extraction logic currently supports Rust, TypeScript/JavaScript, Python, Go, Java, PHP, C/C++, and Markdown. It uses tree-sitter AST walking and range-containment for nesting (or level-based nesting for Markdown). * Integrated extraction into the existing syntax worker thread in `src/runtime/app.rs`. The worker now returns `OutlineData` alongside `SyntaxHighlights` in the `ParseCompleted` message. * Added `OutlinePanelState` to `AppModel` (in `src/model/ui.rs` and `src/model/mod.rs`) to track collapse state, selection, and scroll offset. * Added an `outline` field to the `Document` struct in `src/model/document.rs`. * Implemented the `render_outline_panel` in `src/view/mod.rs`, which replaces the placeholder in the right dock. It uses a recursive `render_node` pattern similar to the file explorer. * Created `src/update/outline.rs` to handle `OutlineMsg` for jumping to symbols and toggling nodes. I've identified several issues and improvements needed: * Clicking tree nodes currently does not collapse them; I need to investigate the hit-testing and message dispatch in `src/view/hit_test.rs` and `src/update/outline.rs`. * For Markdown, I want to strip the `#` prefixes from heading names in the outline for better ergonomics. * The outline panel does not properly support scrolling. I need to implement a reusable scrollable area abstraction, as there might be an existing pattern in the UI rendering code I should leverage. * I want to add YAML support to the outline extractor. * Double-clicking an item in the outline should scroll the active editor to that line. * I want to perform a thorough code review and scrutinize the current implementation before fixing these issues. some minor issues with it: clicking tree nodes canot collapse them, for markdown, we wanna strip off the # prefixes (more ergonomic and is only for md and that will work anywhere most likely), the outline panel does not properly allow scorlling we should implement this properly so this scrollable area feature can be reused (we might have an abstraciton for this already, investigtate and ocnsult oracle taking into account the entire ui rendering code surface here), also would like yaml support here if possible, double clicking on something in the outline thing should perhapos scroll to that line in the active editor), lets first code review and scrutinize our implementation

Here's my thorough code review:


Code Review: Outline Panel Implementation

πŸ”΄ Critical Bugs

1. Clicking outline nodes does nothing useful β€” DockContent click handler (mouse.rs:380-392) only calls FocusDock, never dispatches OutlineMsg::ToggleNode or JumpToSymbol. No sub-item hit-testing exists for outline rows.

2. Scrolling is completely broken β€” MouseWheel for HoverRegion::Dock(_) (app.rs:807-811) consumes the event with None β€” never routes to OutlineMsg::Scroll.

3. Scroll offset has no upper bound β€” OutlineMsg::Scroll handler (outline.rs:193-201) freely increments scroll_offset with no max clamp; you can scroll past the end.

4. UTF-8 panic in truncation β€” render_outline_node does &node.name[..max_chars.saturating_sub(1)] which slices by bytes, not chars. Will panic on non-ASCII symbol names (e.g. rΓ©sumΓ©, CJK identifiers).

🟑 Functional Gaps

5. No double-click β†’ jump-to-symbol β€” DockContent doesn't differentiate click count; even once click handling is added, double-click needs explicit routing.

6. Stale state on document switch β€” OutlinePanelState is global. When switching tabs, scroll_offset and selected_index are never reset, pointing at stale positions.

7. Collapse key collision β€” OutlinePanelState keys use (label, name, line) but Function and Method both have label "fn", so a function and method on the same line with the same name would share collapse state. Should use OutlineKind directly (derive Hash).

8. Per-frame allocations in is_collapsed β€” node_key() clones node.name (allocating a String) on every call. This runs for every visible node every frame.

9. No YAML outline β€” LanguageId::Yaml falls through to Vec::new() despite tree-sitter-yaml being available.

🟒 Cosmetic / Quality

10. Markdown # in names β€” The parse_heading primary path correctly extracts inline/heading_content which shouldn't include # markers. The fallback path also strips #. Should be fine, but the OutlineKind::Heading label renders #/## prefixes before the name β€” so the user sees # Title which is redundant. Consider using a heading icon like H1/H2 or just the name.

11. render_outline_node has 17 parameters β€” Hard to maintain. A SidebarRenderContext-like struct already exists for the sidebar and should be replicated here.

Summary of Fixes Needed

Issue Severity Fix
Click handling πŸ”΄ Add row hit-testing in DockContent handler, dispatch OutlineMsg
Scroll routing πŸ”΄ Route HoverRegion::Dock scroll to OutlineMsg::Scroll when active panel is Outline
Scroll clamp πŸ”΄ Clamp scroll_offset to max(0, total_items - visible_capacity)
UTF-8 truncation πŸ”΄ Use .chars().take(n) instead of byte slicing
Double-click 🟑 Pass click count, jump on double-click
Stale state reset 🟑 Reset scroll_offset/selected_index on tab/doc switch
Collapse key 🟑 Derive Hash on OutlineKind, use (OutlineKind, OutlineRange)
Per-frame allocs 🟑 Follows from collapse key fix
YAML support 🟑 Add YAML key extractor
Heading labels 🟒 Change heading label from # to H1/H2/etc.
17-param function 🟒 Extract OutlineRenderContext struct

Want me to start fixing these issues? I'd suggest tackling them in priority order: click handling β†’ scroll routing/clamp β†’ UTF-8 fix β†’ double-click β†’ state reset β†’ collapse key β†’ render context β†’ YAML β†’ heading labels.

yes

All clean β€” no warnings, no errors. Here's a summary of everything fixed:

Changes Made

File Fix
src/outline/mod.rs Added Hash to OutlineKind + OutlineRange; changed heading labels from #/## β†’ H1/H2
src/outline/extract.rs Added YAML outline extractor (block_mapping_pair/flow_pair β†’ nested Property tree)
src/model/ui.rs Changed collapse key from (&'static str, String, usize) to (OutlineKind, OutlineRange) β€” fixes label collisions + per-frame allocations
src/messages.rs Added OutlineMsg::ClickRow { index, click_count, on_chevron }
src/update/outline.rs Implemented ClickRow handler (select + chevron toggle + double-click jump); added scroll bounds clamping; clamped after expand/collapse
src/update/layout.rs Reset scroll_offset/selected_index on tab switch and group focus
src/runtime/mouse.rs DockContent click now does row hit-testing for outline, dispatches ClickRow with chevron detection
src/runtime/app.rs Dock scroll events now route to OutlineMsg::Scroll when outline is active
src/view/mod.rs Fixed UTF-8 truncation panic (byte→char slicing); extracted OutlineRenderContext struct (17 params → 7)
hmm clicks do not seem to propogate properly into the outloine panel, investiagte it more thuroughly.

Confirmed β€” the only lint error is a pre-existing issue in recent_files.rs, not from our changes. Our code passes cleanly.

Root cause found and fixed: hit_test_ui() had no dock hit-testing at all. Clicks on the right dock area fell through to hit_test_groups(), which matched them as EditorContent since editor group rects extend under the dock in coordinate space.

Fix: Added hit_test_docks() function that checks right and bottom dock rectangles, and inserted it into hit_test_ui() at priority 5 β€” after sidebar but before splitters/editor groups. Dock clicks now correctly produce HitTarget::DockContent with the active panel ID, which the mouse handler then routes to OutlineMsg::ClickRow.

it works, and quite nicely asd well, only thing i am missing is arrow navigation similar to how file explorer sidebar has, but we can defer that for later, lets archive the feature doc about outline and add a new one noting the arrow navigation stuff in the oiutline panel in a new feature doc, this can likely be part of a larger refactor/abstraction where we abstract away arrow navigation and tree/navigtation/focus management so we can reuse this in other types of panels or areas (like todo panel, file browser, etc)

Done. Moved docs/feature/code-outline.md β†’ docs/archived/code-outline.md and created docs/feature/tree-navigation-abstraction.md covering the arrow key gap and the broader tree navigation abstraction plan (Phase 1: wire keys, Phase 2: extract shared state, Phase 3: trait, Phase 4: unified renderer).

lets commit our changes
Explored 2 searches

Hmm, many files I expected to see modified aren't showing. Let me check if there were prior unstaged changes:

Explored 2 searches

The earlier changes (model, messages, extract, update, view, runtime) were likely made on top of uncommitted work from the previous thread. Let me check which files have our changes:

Explored 2 searches

Those files show no diff against HEAD β€” meaning the prior thread already had them in the working tree but they were committed or they're somehow already matching. Let me check:

Ran 2 commands

The previous thread's changes plus our Phase 1/2 task agent changes are already committed (the agents likely ran git add/git commit or the files were part of a dirty tree that got auto-committed). Only the hit-test fix, the outline module as new files, the doc archive, and the new feature doc need committing. Let me verify:

Explored 1 search
$ git add \ docs/feature/code-outline.md \ docs/archived/code-outline.md \ docs/feature/tree-navigation-abstraction.md \ src/view/hit_test.rs \ src/outline/ \ src/update/outline.rs
Explored 1 search
$ git commit -m "feat: code outline panel with tree-sitter symbol extraction - Extract outline symbols from tree-sitter ASTs for Rust, TypeScript, JavaScript, Python, Go, Java, PHP, C/C++, Markdown, and YAML - Render collapsible tree in right dock with selection highlighting - Click to select, double-click to jump to symbol, chevron to toggle - Add dock hit-testing to hit_test_ui() (was missing, causing clicks to fall through to editor) - Scroll support with bounds clamping - Reset outline state on tab/document switch - Fix UTF-8 truncation panic (byte slicing β†’ char-based) - Use (OutlineKind, OutlineRange) as collapse key instead of string - Extract OutlineRenderContext to reduce parameter count - Archive code-outline.md, add tree-navigation-abstraction.md for planned keyboard nav and shared tree widget abstraction"

Committed as b380bee. 6 files changed β€” the outline module, dock hit-testing fix, and docs.