feat(file-manager): Win2K Explorer view with top menu bar + click-to-navigate

[AndrewAltimit]

Summary

• New Win2K-style Grid view for the file manager (icon grid, folder tree, address bar, status row) — now the default presentation.
• Old dual-panel List view is still available; toggle between them via the new View menu in a Notepad-style top menu bar (File / Edit / View).
• Click-to-navigate: tree-pane rows navigate on a single click; grid tiles select on first click and activate on the second (approximates double-click without timestamp info). Files open in the embedded viewer; folders enter on the next refresh tick.

Implementation notes

• Both views share oasis_ui::menu_bar::MenuBar. Windowed mode uses MenuBar::draw_bar / draw_dropdown directly; SDI fullscreen mode mirrors the layout via pooled SDI objects under app_fm_menubar_* / app_fm_dd_*. Hit-tests route through MenuBar::hit_test in App::handle_click.
• App::handle_click doesn't receive a vfs reference, so clicks that need it queue a NavTarget and App::refresh(vfs) drains the queue. A new generic AppRunner::refresh_app(vfs) forwarder is invoked from the WmEvent::ContentClick dispatcher right after handle_click, so navigation lands on the same frame.
• compute_explorer_geom is theme-independent so the click handler computes the exact rects the renderer drew. Tile geometry is cached in Cell<usize> so input ticks navigate by the right grid metrics.
• Existing dual-panel runner tests now flip view_mode = Dual since Explorer is the default; new tests cover the menu actions, click-to-open-dropdown, single-click tree navigation, and double-click folder activation.

Test plan

• cargo fmt --all -- --check
• cargo clippy --workspace -- -D warnings
• cargo test --workspace (39 file-manager tests + workspace)
• cargo build --release -p oasis-app
• Manual: launch File Manager, click View > List / View > Grid to toggle, click into a folder tile twice (or single-click in tree pane) and confirm navigation, open a text file via the Grid view to confirm the embedded viewer still works.

Generated with Claude Code

[github-actions]

Openrouter AI General Review

Issues (if any)

• [BUG] crates/oasis-app-file-manager/src/lib.rs:808 - SDI geometry calculation mismatch

  • compute_explorer_geom expects ch to be the full height from cy to the bottom of the content area (including status bar), but update_sdi_explorer passes body_h which has already subtracted statusbar_height and bottombar_height.
  • This causes status_y to be calculated incorrectly, placing the status bar too high and overlapping the icon grid in SDI mode.
  • Fix: Pass at.screen_h.saturating_sub(at.app.title_bar_height) as ch to compute_explorer_geom, or adjust the function to accept pre-subtracted heights.

• [WARNING] crates/oasis-app-file-manager/src/lib.rs:1035 - Hardcoded tree line height in hit-test

  • tree_hit_test uses let line_h = 13i32; assuming default font_hint. If a theme changes font size, click detection will misalign with rendered rows.
  • Fix: Pass at.font_hint or compute line_h dynamically inside the function.

• [WARNING] crates/oasis-app-file-manager/src/lib.rs:458 - Potential out-of-bounds navigation on empty directory

  • handle_explorer_input calls p.enter_selected(vfs) when is_file is false, even if lines is empty or abs_idx is out of bounds. enter_selected likely assumes a valid cursor position and may panic or misbehave.
  • Fix: Add if p.lines.is_empty() { return AppAction::None; } before processing Confirm.

Previous Issues (for incremental reviews)

• (none)

Suggestions (if any)

• crates/oasis-app-file-manager/src/lib.rs:433 - Consider extracting the Confirm button logic into a shared activate_index-like method to avoid duplicating directory/file parsing between keyboard and mouse paths.

Notes

• The click-to-navigate queue (pending_navigation) correctly bridges the vfs-less handle_click and the synchronous refresh cycle.
• Menu bar and dropdown SDI pooling (update_menu_bar_sdi / update_menu_dropdown_sdi) is efficiently implemented and correctly hidden when switching view modes.
• Double-click simulation via last_click_tile works reliably for mouse input, though it doesn't clear on directory refresh (acceptable since navigation resets cursor/scroll).

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[github-actions]

Claude AI Security & Correctness Review

Issues (if any)

• [BUG] crates/oasis-app-file-manager/src/lib.rs:2081 — handle_click computes Explorer geometry with body_top = title_h + FM_MENU_H, but both renderers use body_top = title_h (windowed: line 1126; SDI: line 810). This shifts g.body_y by FM_MENU_H = 18px relative to where elements are actually drawn, causing tree-pane and grid click detection to be offset by 18 pixels in both rendering modes. The unit tests pass only because they were written to match the click handler's (mis)calculation — compute_explorer_geom(0, 38, …) in the test corresponds to the handler, not the renderer.

  • Fix: change let body_top = title_h + FM_MENU_H as i32; → let body_top = title_h; to match the renderers. compute_explorer_geom already carves out FM_MENU_H internally (addr_y = menu_y + menu_h), so adding it again here double-counts.

• [WARNING] crates/oasis-app-file-manager/src/lib.rs:1480 — tree_hit_test hardcodes let line_h = 13i32, which assumes font_hint equals the default value. The renderer computes tree_line_h = (at.font_hint as i32 + 2).max(11). Any theme that overrides font size will cause misaligned row hit-testing.

  • Fix: pass font_hint into tree_hit_test and replicate the same formula.

Previous Issues (for incremental reviews)

• [RESOLVED] SDI geometry calculation mismatch (Openrouter flagged status bar offset — the code correctly subtracts statusbar_height + bottombar_height before passing to compute_explorer_geom, which is intentional; the FM status strip is placed at the bottom of the app area, above the system bars)
• [STILL UNRESOLVED] Hardcoded tree line height in hit-test (line_h = 13i32, same as Openrouter's warning)
• [STILL UNRESOLVED] (partially) Empty-directory safety in handle_explorer_input Confirm branch — the is_file check via get(abs_idx).is_some_and(...) means is_file=false when lines is empty, and enter_selected(vfs) is called on empty panel. Whether enter_selected panics depends on its implementation (not visible in this diff). Verify its bounds checking.

Suggestions (if any)

• crates/oasis-app-file-manager/src/lib.rs:2040 — The _fullscreen parameter in handle_click is silently ignored. In fullscreen SDI mode, ch = screen_h (includes system bars), whereas the SDI renderer subtracts statusbar_height + bottombar_height from body_h. The 18px fix above is the primary concern, but _fullscreen could carry the info needed to correctly replicate SDI geometry in the click handler if that becomes an issue.

Notes

• The geometry double-count (FM_MENU_H added in handle_click but compute_explorer_geom already places the menu at cy) is the only clear correctness bug. All three tests for click-to-navigate (double_click_on_folder_navigates_after_refresh, click_on_tree_row_navigates_immediately, click_on_view_label_opens_dropdown) would need their compute_explorer_geom baseline adjusted to match the fixed renderer geometry.
• The pending_navigation queue pattern is sound; no TOCTOU risk since refresh is called synchronously after handle_click in input.rs:205.
• No path traversal risk beyond what the VFS layer already enforces; paths are constructed from VFS listings, not free-form user text.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [BUG] STILL UNRESOLVED crates/oasis-app-file-manager/src/lib.rs (handle_click, Explorer section) — body_top = title_h + FM_MENU_H is passed as cy to compute_explorer_geom, which then adds FM_MENU_H again for addr_y. Result: g.body_y is 18px below the renderer's body_y, so tree-pane and grid hit-tests require the user to click 18px below where icons are actually drawn.

  • Renderer (SDI): compute_explorer_geom(0, title_h, w, body_h) → body_y = title_h + FM_MENU_H + FM_ADDR_H + pad
  • Click handler: compute_explorer_geom(0, title_h + FM_MENU_H, cw, …) → body_y = title_h + 2*FM_MENU_H + FM_ADDR_H + pad
  • Fix: let body_top = title_h; (matching update_sdi_explorer). The unit tests were written to match the handler's miscalculation (compute_explorer_geom(0, 38, …) with title_h=20), so they will need their baseline adjusted.

• [BUG] crates/oasis-app-file-manager/src/lib.rs (handle_click, grid hit section) — on every grid tile click, panel.scroll [UNVERIFIED - File does not exist and is not in PR] is unconditionally reset to (abs / cols) * cols, which scrolls the view so the clicked row becomes the first visible row. Since grid_hit_test only returns tiles already within g.rows [UNVERIFIED - File does not exist and is not in PR], the tile is guaranteed visible; the scroll reassignment is wrong and causes jarring view jumps on any non-first-row click.

  • Fix: remove panel.scroll = (abs / g.cols.max(1)) * g.cols.max(1);; only update panel.cursor = abs - panel.scroll;.

• [WARNING] STILL UNRESOLVED crates/oasis-app-file-manager/src/lib.rs:1480 (tree_hit_test) — let line_h = 13i32; hardcoded; renderer uses (at.font_hint as i32 + 2).max(11). Any non-default theme breaks row alignment.

Previous Issues (for incremental reviews)

• [RESOLVED] lib.rs:808 — SDI geometry calculation mismatch (status bar placement). update_sdi_explorer now correctly subtracts system bars from body_h before passing to compute_explorer_geom.
• [STILL UNRESOLVED] lib.rs:1480 — Hardcoded line_h = 13 in tree_hit_test.
• [STILL UNRESOLVED] lib.rs — handle_explorer_input Confirm branch calls enter_selected when lines is empty (is_some_and returns false → is_file=false → enters the else branch unconditionally).
• [STILL UNRESOLVED] (Claude prior) — handle_click body_top double-counts FM_MENU_H.

Suggestions (if any)

• crates/oasis-app-file-manager/src/lib.rs (handle_click) — _fullscreen is still silently ignored. SDI mode subtracts statusbar_height + bottombar_height from the geometry, but the click handler doesn't. If a theme has a non-zero status/bottom bar, the lower portion of the grid will be unclickable (hits fall outside g.body_h). At minimum, assert or document why fullscreen mode doesn't need separate geometry.

Notes

• 2 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• The NavTarget queue pattern and refresh_app forwarder are clean solutions to the vfs-less click handler constraint.

• The new activate_index method correctly unifies the keyboard and mouse activation paths.

• Tests pass only because they mirror the click handler's miscalculated geometry, not the renderer's — the test baseline compute_explorer_geom(0, 38, …) would need to change to (0, 20, …) after the fix.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 1)

Status: Changes committed, pushing...

Commit: 6261fa0

Fixed Issues

• [BUG] handle_click body_top double-counted FM_MENU_H (crates/oasis-app-file-manager/src/lib.rs:2081): Changed let body_top = title_h + FM_MENU_H as i32; to let body_top = title_h; so the click handler matches both renderers (update_sdi_explorer and draw_windowed_explorer). compute_explorer_geom already carves out the menu strip internally; adding it again offset hit-tests by 18 pixels.
• [BUG] Grid click reset panel.scroll, jumping the view (crates/oasis-app-file-manager/src/lib.rs:2095): Removed the panel.scroll = (abs / cols) * cols reassignment. grid_hit_test only returns indices for tiles already visible, so cursor = abs - scroll is sufficient and no scroll shift is needed.
• [WARNING] tree_hit_test hardcoded line_h = 13 (crates/oasis-app-file-manager/src/lib.rs:1480): Added font_hint: u16 parameter and used the renderer's (font_hint as i32 + 2).max(11) formula. Added cached_font_hint: Cell<u16> to FileManagerApp, populated in both render paths so the click handler can pass the live theme value.
• Test baselines: Updated double_click_on_folder_navigates_after_refresh and click_on_tree_row_navigates_immediately to use compute_explorer_geom(0, 20, 600, 380) (matching the fixed handler geometry: body_top = title_h = 20, body_h = ch - title_h).

Ignored Issues

• [WARNING] Empty-directory enter_selected panic risk: Verified FilePanel::enter_selected (lines 85–106) already guards with let Some(line) = self.lines.get(abs_idx) else { return; };. No fix needed.
• [Suggestion] _fullscreen parameter ignored in handle_click: Skipped — the SDI renderer subtracts statusbar_height + bottombar_height from body_h, but the click handler doesn't have &ActiveTheme. With non-zero system bars, the bottom strip of the grid would be unclickable in fullscreen mode. Fixing this would require caching another theme value; deferred since the primary 18px offset (the visible hit-test bug) is now resolved and grid_hit_test already bounds-checks via the visible row count.
• [Suggestion] Refactor Confirm into shared activate_index: Speculative refactor; the keyboard path has additional LaunchAppWithFile logic that doesn't trivially merge.

Deferred to Human

• None.

Notes

• Verified with cargo build -p oasis-app-file-manager, cargo test -p oasis-app-file-manager (39/39 pass), cargo build -p oasis-core -p oasis-app, and cargo fmt --check.
• The 8 clippy unwrap_used errors in test setup (e.g. vfs.write(...).unwrap()) are pre-existing and unrelated to these changes.

---

Automated summary of agent fixes.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app-file-manager/src/lib.rs (handle_click, line ~2102) — In SDI/fullscreen mode, body_h_local = ch - body_top uses the full content height, but update_sdi_explorer subtracts statusbar_height + bottombar_height before passing to compute_explorer_geom. The difference equals those system bar heights. In themes where either is non-zero, handle_click computes a taller geometry than was rendered: the bottom portion of the hit-test grid overlaps the system bars, and tiles in that strip respond to clicks even though no icon was drawn there.
  • Fix: when _fullscreen is true, subtract at.statusbar_height + at.bottombar_height from body_h_local; cache these values alongside cached_font_hint.

Previous Issues (for incremental reviews)

• [RESOLVED] lib.rs — handle_click double-counted FM_MENU_H in body_top. Now body_top = title_h in all three paths.
• [RESOLVED] lib.rs:1480 — Hardcoded line_h = 13 in tree_hit_test. Now passes cached_font_hint and uses (font_hint as i32 + 2).max(11) matching the renderer.
• [RESOLVED] lib.rs:458 — Empty-directory safety in Confirm branch. Verified FilePanel::enter_selected guards with self.lines.get(abs_idx) early return.

Suggestions (if any)

• crates/oasis-app-file-manager/src/lib.rs (update_menu_bar_sdi / update_menu_dropdown_sdi) — Both functions compute label widths as chars().count() * 7 + 16, independently from MenuBar's internal layout. If MenuBar::draw_bar / hit_test ever switches to measure_text-based widths, SDI label positions and dropdown anchors will silently diverge. Consider exposing a label_width(label: &str) -> i32 helper from MenuBar so both code paths share the same formula.

Notes

• The three previously flagged bugs are all resolved; the iteration-1 fix was clean.
• The remaining [WARNING] only manifests in themes with non-zero statusbar_height or bottombar_height in fullscreen SDI mode — the default skin has both at zero, so it won't be visible in standard testing.
• refresh_app is called unconditionally after every click (not gated on pending_navigation.is_some()). The App::refresh default no-ops so this is harmless, but a cheap Option check before the call would avoid an unnecessary sync_from_delegate on every click.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app-file-manager/src/lib.rs:2102 - handle_click ignores _fullscreen flag, causing hit-test geometry mismatch in SDI fullscreen mode when themes have non-zero statusbar_height or bottombar_height.
  • body_h_local is calculated as ch - body_top, but update_sdi_explorer subtracts system bar heights from body_h. Clicks in the bottom strip will hit invisible grid tiles.
  • Fix: Cache statusbar_height and bottombar_height alongside cached_font_hint, or subtract them from body_h_local when _fullscreen is true.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:808 - SDI geometry calculation mismatch.
• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:1035 - Hardcoded tree line height in hit-test.
• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:458 - Potential out-of-bounds navigation on empty directory.
• [STILL UNRESOLVED] crates/oasis-app-file-manager/src/lib.rs:2102 - SDI fullscreen body_h mismatch (detailed in Issues).

Suggestions (if any)

• crates/oasis-app-file-manager/src/lib.rs (update_menu_bar_sdi / update_menu_dropdown_sdi) - Label width calculation (chars().count() * 7 + 16) is duplicated from MenuBar's internal layout. Consider exposing a label_width helper to prevent silent divergence if MenuBar changes.

Notes

• The NavTarget queue and refresh_app forwarder cleanly bridge the vfs-less click handler constraint.
• grid_hit_test correctly bounds-checks against lines.len(), preventing panics on sparse grids.
• Double-click simulation via last_click_tile is robust and correctly resets on navigation.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none — no new bugs found in the new code)

Previous Issues (for incremental reviews)

• [RESOLVED] lib.rs — handle_click double-counted FM_MENU_H in body_top. Now body_top = title_h in all three code paths.
• [RESOLVED] lib.rs:1480 — Hardcoded line_h = 13 in tree_hit_test. Now accepts font_hint: u16 and uses (font_hint as i32 + 2).max(11) matching the renderer.
• [RESOLVED] lib.rs:458 — Empty-directory enter_selected risk. Verified in previous iteration: FilePanel::enter_selected guards with self.lines.get(abs_idx).
• [STILL UNRESOLVED] lib.rs (handle_click, ~line 2102) — body_h_local = (ch as i32 - body_top).max(20) as u32 ignores system bar heights in SDI/fullscreen mode. update_sdi_explorer subtracts at.statusbar_height + at.bottombar_height before passing to compute_explorer_geom; the click handler does not. In any theme where those values are non-zero, handle_click computes a taller geometry than was rendered, so the bottom strip of the hit-test grid overlaps invisible system bars and registers phantom tile clicks.
  • Fix: cache statusbar_height + bottombar_height alongside cached_font_hint; when _fullscreen is true, subtract the cached sum from body_h_local.

Suggestions (if any)

• lib.rs (update_menu_bar_sdi / update_menu_dropdown_sdi) — Both compute label widths as chars().count() * 7 + 16, independently from MenuBar's internal layout. If MenuBar ever switches to measure_text-based widths, SDI label positions and dropdown anchors will silently diverge. Expose a label_width(label: &str) -> i32 helper from MenuBar so both paths share one formula.

Notes

• The one remaining unresolved warning only fires in themes with non-zero statusbar_height or bottombar_height in fullscreen SDI mode. The default skin has both at zero, so it won't appear in standard testing.
• All three previously confirmed bugs are cleanly fixed; the iteration-1 agent correctly addressed the primary correctness issues.
• refresh_app is called unconditionally after every click (not gated on pending_navigation.is_some()), triggering sync_from_delegate on every click. Harmless given the no-op default, but a cheap guard would avoid unnecessary work.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 2)

Status: Changes committed, pushing...

Commit: 74d531a

Fixed Issues

• [WARNING] handle_click ignored _fullscreen flag, causing hit-test geometry mismatch in SDI fullscreen mode (crates/oasis-app-file-manager/src/lib.rs):
  • update_sdi_explorer subtracts at.statusbar_height + at.bottombar_height from body_h before passing to compute_explorer_geom, but the click handler did not. In themes with non-zero system bars, this caused the bottom strip of the hit-test grid to overlap (invisible) system bars and register phantom tile clicks.
  • Added cached_system_bars: Cell<u32> field to FileManagerApp, populated from both update_sdi_explorer and draw_windowed_explorer with at.statusbar_height + at.bottombar_height.
  • Renamed _fullscreen → fullscreen in handle_click and subtract the cached system-bar sum from body_h_local when fullscreen is true. Windowed mode is unchanged (the window already excludes system bars, so subtracting 0 is correct).

Ignored Issues

• [Suggestion] Expose MenuBar::label_width(&str) -> i32 helper (in update_menu_bar_sdi / update_menu_dropdown_sdi): Speculative refactor. Both reviewers flagged that the chars().count() * 7 + 16 formula is duplicated from MenuBar's internal layout, but MenuBar itself doesn't expose such a helper today. This would be a cross-crate API change for a divergence that doesn't currently exist; deferred as preventive cleanup, not a real bug.
• [Note] refresh_app called unconditionally on every click: Reviewers noted a cheap pending_navigation.is_some() guard would avoid a sync_from_delegate no-op. The default App::refresh is documented as no-op, so this is a micro-optimization — skipped to keep the diff focused on the correctness fix.

Deferred to Human

• None.

Notes

• Verified with cargo build -p oasis-app-file-manager (clean), cargo test -p oasis-app-file-manager (39/39 pass), cargo build -p oasis-core -p oasis-app (clean), cargo fmt --all -- --check (clean), and cargo clippy -p oasis-app-file-manager -- -D warnings (clean).
• The fix is theme-conditional: default skin has both bar heights at 0, so existing tests (which use fullscreen: false) remain valid without changes.

---

Automated summary of agent fixes.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:808 - SDI geometry calculation mismatch
• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:1035 - Hardcoded tree line height in hit-test
• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:458 - Potential out-of-bounds navigation on empty directory
• [RESOLVED] crates/oasis-app-file-manager/src/lib.rs:2102 - handle_click ignores _fullscreen flag, causing hit-test geometry mismatch in SDI fullscreen mode

Suggestions (if any)

• crates/oasis-app-file-manager/src/lib.rs (compute_explorer_geom) - tree_w clamping uses cw.saturating_sub(120), which forces the tree pane to 0 width on windows narrower than 120px. Consider a minimum cw guard or allowing graceful overlap, though this is unlikely on target resolutions (PSP/desktop).

Notes

• All previously flagged geometry and hit-test mismatches are cleanly resolved. The _fullscreen system-bar subtraction and cached_font_hint propagation correctly align input handling with both SDI and windowed render paths.
• The NavTarget queue paired with refresh_app successfully bridges the vfs-less click handler constraint without introducing TOCTOU risks.
• Double-click simulation via last_click_tile correctly resets on navigation and menu interactions.
• Bounds checks in grid_hit_test and tree_entry_path prevent panics on sparse directories or out-of-bounds clicks.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

• [RESOLVED] lib.rs — handle_click body_top double-counted FM_MENU_H. Fixed: body_top = title_h in all paths.
• [RESOLVED] lib.rs:1480 — Hardcoded line_h = 13 in tree_hit_test. Fixed: cached_font_hint propagated; uses (font_hint as i32 + 2).max(11) matching the renderer.
• [RESOLVED] lib.rs:458 — Empty-directory safety in Confirm branch. Verified: enter_selected guards on lines.get(abs_idx).
• [RESOLVED] lib.rs (handle_click, ~line 2102) — _fullscreen ignored, causing hit-test geometry mismatch in SDI fullscreen mode. Fixed: renamed to fullscreen, cached_system_bars: Cell<u32> added and populated by both render paths; subtracted from body_h_local when fullscreen = true.

Suggestions (if any)

• lib.rs (update_menu_bar_sdi / update_menu_dropdown_sdi) — Label width formula chars().count() * 7 + 16 is duplicated from MenuBar's internal layout. If MenuBar ever switches to measure_text-based widths, SDI label positions and dropdown anchors will silently diverge. Exposing a MenuBar::label_width(&str) -> i32 helper would keep the two paths in sync.

Notes

• All five previously flagged issues are now resolved across two iteration passes. The cached_system_bars fix correctly mirrors what update_sdi_explorer does before calling compute_explorer_geom, and draw_windowed_explorer also caches the value so the click handler is always primed.
• refresh_app is still called unconditionally after every click (not gated on pending_navigation.is_some()), triggering a sync_from_delegate no-op on every non-navigation click. Harmless; not worth flagging again.
• No path traversal risk: file/folder names originate from VFS directory listings, not free-form user text input.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

• [RESOLVED] lib.rs:808 — SDI geometry calculation mismatch (status bar placement). update_sdi_explorer correctly subtracts system bars from body_h before passing to compute_explorer_geom.
• [RESOLVED] lib.rs:1480 — Hardcoded line_h = 13 in tree_hit_test. Now accepts font_hint: u16 and uses (font_hint as i32 + 2).max(11) matching the renderer.
• [RESOLVED] lib.rs:458 — Empty-directory enter_selected risk. FilePanel::enter_selected guards on self.lines.get(abs_idx).
• [RESOLVED] lib.rs (handle_click, ~2102) — body_h_local used full content height in SDI fullscreen mode without subtracting system bars. Fixed: cached_system_bars: Cell<u32> added and populated by both render paths; subtracted from body_h_local when fullscreen is true.
• [RESOLVED] lib.rs:2102 — handle_click ignored _fullscreen flag. Now renamed to fullscreen and the system-bar correction is applied.

Suggestions (if any)

• lib.rs (update_menu_bar_sdi / update_menu_dropdown_sdi) — Label width formula chars().count() * 7 + 16 is still duplicated independently from MenuBar's internal layout. If MenuBar ever switches to measure_text-based widths, SDI label positions and dropdown anchors will silently diverge. Exposing a MenuBar::label_width(&str) -> i32 helper would keep both paths in sync.

Notes

• All five previously flagged issues are cleanly resolved across two iteration passes. The cached_system_bars fix in iteration 2 correctly mirrors the update_sdi_explorer subtraction, and draw_windowed_explorer also populates it so the click handler is always primed before a click can land.
• refresh_app is still called unconditionally after every click (not gated on pending_navigation.is_some()), triggering a sync_from_delegate no-op on non-navigation clicks. Harmless given the no-op default.
• The MAX_TILES = 48 SDI pool cap is not applied in draw_windowed_explorer (which uses g.cols * g.rows directly), creating a minor behavioral inconsistency between SDI and windowed rendering at larger resolutions (e.g. modern skin at 800×600 could exceed 48 visible tiles). Not introduced by this iteration, but worth noting.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 3)

Status: Hallucination detected — no commit

Detection: The agent's summary below claims to have applied fixes, > but Claude made zero file-mutating tool calls (Edit / Write / MultiEdit / NotebookEdit) > during the run. The retry was skipped because it uses the same prompting > strategy Claude already ignored.

Stream log: /home/mikunpc/Documents/repos/actions-runner-oasis-os/_work/_temp/review-agent-logs/claude-stream-iter2-1777152656071.jsonl
Uploaded as the review-agent-claude-logs workflow artifact.

Fixed Issues

• (none — no Edit calls made this iteration)

Ignored Issues

• [BUG] handle_click body_top double-counted FM_MENU_H — already fixed in iteration 1 (commit 6261fa0). Current code at lib.rs:2111 reads let body_top = title_h;.
• [WARNING] tree_hit_test hardcoded line_h = 13 — already fixed in iteration 1. cached_font_hint is now propagated and the renderer's (font_hint as i32 + 2).max(11) formula is used.
• [WARNING] Empty-directory enter_selected panic risk — already verified safe in iteration 1. FilePanel::enter_selected guards with self.lines.get(abs_idx).
• [BUG] Grid click reset panel.scroll, jumping the view — already fixed in iteration 1.
• [WARNING] handle_click ignored _fullscreen flag, hit-test geometry mismatch in SDI fullscreen — already fixed in iteration 2 (commit 74d531a). Current code at lib.rs:2116-2121 subtracts cached_system_bars from body_h_local when fullscreen is true.
• [BUG] SDI geometry calculation mismatch (status bar placement) — flagged by Openrouter as a hallucination/intentional behavior; confirmed RESOLVED by Claude.
• [Suggestion] Expose MenuBar::label_width(&str) -> i32 helper — speculative cross-crate refactor for a divergence that doesn't currently exist; already deferred in iteration 2 and reaffirmed here.
• [Suggestion] Guard refresh_app on pending_navigation.is_some() — micro-optimization for a documented no-op default; already deferred in iteration 2.
• [Suggestion] tree_w clamping cw.saturating_sub(120) on narrow windows — edge case unlikely on target resolutions (PSP 480×272, desktop ≥800×600).
• [Suggestion] Refactor Confirm into shared activate_index — already addressed in iteration 1; reviewer's MAX_TILES = 48 inconsistency note is pre-existing and not introduced by this PR.

Deferred to Human

• None.

Notes

• All three latest review reports (Openrouter + Claude security + Claude quality, all against commit 74d531a) explicitly mark every previously flagged Issue/Warning as [RESOLVED]. Only suggestions remain, which were judged as not requiring further code changes.
• No Edit/Write calls were made — the working tree is clean and matches the state already pushed.

---

No file modifications were performed; this iteration produced no commit.