Add standalone input routing adapter and contract coverage#134
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new standalone input adapter Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant SIF as StandaloneInputFlow
participant SEF as SelectionEventFlow
participant HLR as HyperlinkOpenRouter
participant Terminal as Terminal
participant Clipboard as Clipboard
Client->>SIF: handle_window_mouse_event(event, geometry)
SIF->>SIF: translate window->selection (single mapping)
SIF->>SEF: handle_routed_mouse_event(translated_event)
SEF-->>SIF: RoutedOutcome(route_target, pty_bytes, consumed, copied)
alt route_target == Local and left-button press
SIF->>HLR: resolve_hyperlink_decision(translated_event.cell)
HLR-->>SIF: Preview / Open / Deny
else
SIF->>HLR: reset preview state (clear)
end
SIF->>Terminal: (optionally) write pty_bytes
SIF->>Clipboard: (optionally) copy selection
SIF-->>Client: StandaloneInputOutcome{consumed, copied, route_target, pty_bytes, hyperlink_decision}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/phases.md (1)
492-501:⚠️ Potential issue | 🟡 MinorKeep the Phase 6 deliverables in sync with the detailed phase doc.
Session profilesis still unchecked here, butdocs/phases/06.mdalready describes--profile <name>support as implemented. Please align the summary checklist so the active-phase snapshot doesn't drift from the detailed phase status.Based on learnings, follow the active phase in
docs/phases.mdand the matching file indocs/phases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/phases.md` around lines 492 - 501, The checklist in docs/phases.md is out of sync: the "Session profiles" item is unchecked while docs/phases/06.md documents implemented --profile <name> support; update the checklist entry for "Session profiles" in docs/phases.md to checked (or otherwise match the status described in docs/phases/06.md) so the summary snapshot matches the detailed phase doc. Also verify any other items in docs/phases.md against docs/phases/06.md and reconcile discrepancies to keep the active-phase summary consistent.
🧹 Nitpick comments (1)
crates/iris-standalone/src/input.rs (1)
46-54: Avoid maintaining a second window-to-cell translation path here.
handle_routed_window_mouse_event(...)andwindow_mouse_adapter.translate(...)are both deriving cell coordinates from the same window event. That works today, but it gives hyperlink hit-testing its own translation path to keep in sync with routed selection behavior. I'd prefer a single source of truth here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iris-standalone/src/input.rs` around lines 46 - 54, The code currently computes cell coordinates twice—once via window_mouse_adapter.translate(event, geometry) and again inside selection_flow.handle_routed_window_mouse_event(...); to fix, collapse to a single translation path: have handle_routed_window_mouse_event return (or expose) the translated cell coordinates along with routed_outcome.route_target (or change it so it accepts precomputed coordinates and reuses them) and then call resolve_hyperlink_decision(terminal, routed_outcome.route_target, translated_coords_from_routed_outcome) instead of using window_mouse_adapter.translate separately; update the signatures/use sites for selection_flow.handle_routed_window_mouse_event and resolve_hyperlink_decision accordingly to remove the duplicated translation (references: window_mouse_adapter.translate, selection_flow.handle_routed_window_mouse_event, resolve_hyperlink_decision, routed_outcome.route_target, translated_event).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/phases.md`:
- Around line 492-501: The checklist in docs/phases.md is out of sync: the
"Session profiles" item is unchecked while docs/phases/06.md documents
implemented --profile <name> support; update the checklist entry for "Session
profiles" in docs/phases.md to checked (or otherwise match the status described
in docs/phases/06.md) so the summary snapshot matches the detailed phase doc.
Also verify any other items in docs/phases.md against docs/phases/06.md and
reconcile discrepancies to keep the active-phase summary consistent.
---
Nitpick comments:
In `@crates/iris-standalone/src/input.rs`:
- Around line 46-54: The code currently computes cell coordinates twice—once via
window_mouse_adapter.translate(event, geometry) and again inside
selection_flow.handle_routed_window_mouse_event(...); to fix, collapse to a
single translation path: have handle_routed_window_mouse_event return (or
expose) the translated cell coordinates along with routed_outcome.route_target
(or change it so it accepts precomputed coordinates and reuses them) and then
call resolve_hyperlink_decision(terminal, routed_outcome.route_target,
translated_coords_from_routed_outcome) instead of using
window_mouse_adapter.translate separately; update the signatures/use sites for
selection_flow.handle_routed_window_mouse_event and resolve_hyperlink_decision
accordingly to remove the duplicated translation (references:
window_mouse_adapter.translate, selection_flow.handle_routed_window_mouse_event,
resolve_hyperlink_decision, routed_outcome.route_target, translated_event).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 210e663c-3112-4ef0-97c0-8851fb721ebf
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CHANGELOG.mdcrates/iris-standalone/Cargo.tomlcrates/iris-standalone/src/input.rscrates/iris-standalone/src/main.rsdocs/phases.mddocs/phases/06.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/iris-standalone/src/input.rs (1)
1-1: Avoid blanket dead-code suppression at module scope.Line 1 disables
dead_codefor the entire file, which can mask real regressions over time. Prefer removing this or scoping#[allow(dead_code)]to specific items temporarily.Suggested diff
-#![allow(dead_code)]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iris-standalone/src/input.rs` at line 1, Remove the module-level blanket attribute `#![allow(dead_code)]` at the top of the file and instead apply `#[allow(dead_code)]` only to specific items that legitimately need it (e.g., individual functions, structs, enums or impls) or use conditional attributes like `#[cfg(test)]` for test-only code; locate the `#![allow(dead_code)]` declaration in this file and either delete it, move the allowance to the exact symbols that require it, or annotate test-only items appropriately so that dead-code warnings remain active for the rest of the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/iris-standalone/src/input.rs`:
- Line 1: Remove the module-level blanket attribute `#![allow(dead_code)]` at
the top of the file and instead apply `#[allow(dead_code)]` only to specific
items that legitimately need it (e.g., individual functions, structs, enums or
impls) or use conditional attributes like `#[cfg(test)]` for test-only code;
locate the `#![allow(dead_code)]` declaration in this file and either delete it,
move the allowance to the exact symbols that require it, or annotate test-only
items appropriately so that dead-code warnings remain active for the rest of the
module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cba20d25-cdc6-492e-a7a2-7730a021eded
📒 Files selected for processing (3)
CHANGELOG.mdcrates/iris-standalone/src/input.rsdocs/phases.md
✅ Files skipped from review due to trivial changes (1)
- docs/phases.md
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
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.
Objective
Implement standalone-side mouse-routing integration so host event handling consumes the shared routing/hyperlink contracts and is covered by parser-driven unit tests.
Scope
StandaloneInputFlow) that composesSelectionEventFlow,InputRouter, andHyperlinkOpenRouter.iris-coredependency.API and Behavior Changes
crates/iris-standalone/src/input.rswithStandaloneInputFlowandStandaloneInputOutcome.HyperlinkOpenRouter.handle_window_mouse_event(...)forwardsiris-platform::Resulterrors directly from routed selection handling.Backward Compatibility
Test Coverage
Added/updated tests in
crates/iris-standalone/src/input.rs:standalone_input_flow_matches_routing_contract_for_app_then_local_selectionstandalone_input_flow_applies_shift_local_override_with_active_mouse_reportingstandalone_input_flow_requires_local_route_for_hyperlink_preview_then_openCoverage strategy:
Verification
Commands run and results:
cargo fmt --all(pass)cargo clippy -p iris-standalone --all-targets -- -D warnings(pass)cargo test -p iris-standalone(pass)cargo clippy --all-targets -- -D warnings(pass)cargo test --all(pass)Files Changed
crates/iris-standalone/src/input.rscrates/iris-standalone/src/main.rscrates/iris-standalone/Cargo.tomlCargo.lockdocs/phases/06.mddocs/phases.mdCHANGELOG.mdNotes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation