Skip to content

Move overview highlight with scroll wheel - #1436

Merged
steipete merged 3 commits into
steipete:mainfrom
joshuavial:jv/overview-scroll-highlight
Jun 11, 2026
Merged

Move overview highlight with scroll wheel#1436
steipete merged 3 commits into
steipete:mainfrom
joshuavial:jv/overview-scroll-highlight

Conversation

@joshuavial

Copy link
Copy Markdown
Contributor

Summary

Scrolling the wheel (or trackpad) while the overview tab is open now moves the row highlight up and down, so you can flick through providers without moving the pointer.

Behavior

  • Scroll steps are delivered as synthetic arrow-key events, so AppKit's native menu highlight, submenu, and return-key activation behavior stay intact.
  • Separate step thresholds for precise devices (trackpads, 24px per step) and classic scroll wheels (0.9 lines per step), with directional accumulation reset so reversing direction responds immediately.
  • Momentum-phase events after a flick are swallowed so the highlight doesn't keep racing after fingers leave the trackpad, and a single event moves at most 3 steps.
  • The wheel is left alone while a row submenu is open (e.g. scrollable charts) — only the root overview list translates scrolling into highlight movement.

Tests

  • StatusMenuOverviewScrollTests (7 tests): step thresholds for precise/line devices, multi-step flicks capped at 3, momentum swallowing, direction reversal reset, submenu passthrough, and non-overview menus ignoring the wheel.

Test plan

  • swift test --filter StatusMenuOverviewScroll — 7/7 pass
  • ./Scripts/lint.sh lint — 0 violations
  • Manual verification — scrolled the overview list in a local build; highlight follows the wheel, charts submenu still scrolls natively

@clawsweeper

clawsweeper Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed June 11, 2026, 11:46 AM ET / 15:46 UTC.

Summary
Adds thresholded trackpad and mouse-wheel navigation between Overview provider cards while preserving native card highlighting, submenu scrolling, and keyboard behavior.

Reproducibility: yes. for the introduced defect: retain a sub-threshold precise trackpad delta, then send a same-direction coarse wheel event while the same Overview menu remains open. Source inspection shows the values are combined despite being measured in incompatible units.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 5 files, +350/-1. The behavior is focused, with most additions consisting of new AppKit event routing and its dedicated tests.
  • Focused coverage: 9 tests reported passing. Coverage is substantial but omits the mixed-device transition responsible for the remaining defect.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Reset or separate accumulated deltas when precise/coarse mode changes and add tests for both transitions.
  • Remove the CHANGELOG.md entry.

Risk before merge

  • [P1] Users alternating between a trackpad and an external mouse while the Overview menu remains open can have one wheel notch unexpectedly move the highlight by the three-step cap.
  • [P1] The positive runtime proof covers normal same-device scrolling but does not demonstrate either precise-to-coarse or coarse-to-precise transitions.

Maintainer options:

  1. Decide the mitigation before merge
    Track the active precise/coarse scroll mode and clear the accumulated remainder whenever it changes, add regression coverage for both transition directions, and keep the release-owned changelog entry out of this normal feature PR.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • The remaining blockers are narrow mechanical repairs in clearly identified files, with focused validation and no unresolved product or security decision.

Security
Cleared: The patch only changes local AppKit event handling, controller state, tests, and release text; it adds no dependencies, downloads, permissions, secrets, or supply-chain execution.

Review findings

  • [P2] Reset the accumulator when scroll units change — Sources/CodexBar/StatusItemController+OverviewScroll.swift:42-46
  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7
Review details

Best possible solution:

Track the active precise/coarse scroll mode and clear the accumulated remainder whenever it changes, add regression coverage for both transition directions, and keep the release-owned changelog entry out of this normal feature PR.

Do we have a high-confidence way to reproduce the issue?

Yes for the introduced defect: retain a sub-threshold precise trackpad delta, then send a same-direction coarse wheel event while the same Overview menu remains open. Source inspection shows the values are combined despite being measured in incompatible units.

Is this the best way to solve the issue?

No, not yet. Routing synthetic mouse movement through the real provider cards is appropriate, but accumulation must reset or remain separate when hasPreciseScrollingDeltas changes.

Full review comments:

  • [P2] Reset the accumulator when scroll units change — Sources/CodexBar/StatusItemController+OverviewScroll.swift:42-46
    Trackpad deltas are pixels while classic wheel deltas are lines, but both are added to the same accumulator and compared using only the current event's threshold. For example, a leftover 20-pixel trackpad delta followed by one same-direction wheel notch is treated as roughly 21 lines and emits the three-step cap. Reset or separate state when hasPreciseScrollingDeltas changes and cover both transition directions.
    Confidence: 0.99
  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7
    This repository's release flow owns release-note artifacts, so the normal feature branch should not directly add its own changelog line. Remove the entry and retain the user-visible release context in the PR body or commit message.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 88c43eeb8485.

Label changes

Label justifications:

  • P3: This is contained menu-interaction polish whose remaining defect affects an uncommon mixed-input sequence rather than core availability.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): Fresh-bundle Peekaboo validation reported by the repository owner demonstrates after-fix navigation between real Overview provider cards and submenu opening in both directions; it does not settle the separate mixed-device edge case.
  • proof: sufficient: Contributor real behavior proof is sufficient. Fresh-bundle Peekaboo validation reported by the repository owner demonstrates after-fix navigation between real Overview provider cards and submenu opening in both directions; it does not settle the separate mixed-device edge case.
Evidence reviewed

Acceptance criteria:

  • [P1] swift test --filter StatusMenuOverviewScrollTests.
  • [P1] make check.
  • [P1] git diff --check origin/main...HEAD.

What I checked:

Likely related people:

  • steipete: The current provider-switcher tracking behavior dates to their v0.33.0 work, and they authored the latest branch correction that routes navigation to actual provider-card views. (role: feature owner and recent area contributor; confidence: high; commits: 6cf422512061, 13feb0f38ec6; files: Sources/CodexBar/StatusItemController+ProviderSwitcher.swift, Sources/CodexBar/StatusItemController+OverviewScroll.swift, Sources/CodexBar/StatusItemController.swift)
  • anirudhvee: Commit history connects their earlier work to provider-switcher shortcut handling inside an open menu, adjacent to the event-monitor path extended by this PR. (role: prior feature contributor; confidence: medium; commits: 904549557ec1; files: Sources/CodexBar/StatusItemController+ProviderSwitcher.swift)
  • Yuxin-Qiao: Their recent merged work stabilizes provider-menu positioning and touches the same status-controller and menu lifecycle area. (role: recent adjacent contributor; confidence: medium; commits: 67a9f43f58bf; files: Sources/CodexBar/StatusItemController.swift, Sources/CodexBar/StatusItemController+ProviderSwitcher.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19def14d7e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/CodexBar/StatusItemController+OverviewScroll.swift
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 11, 2026
@steipete
steipete force-pushed the jv/overview-scroll-highlight branch from 66eb328 to c1abd3c Compare June 11, 2026 14:02
@steipete

Copy link
Copy Markdown
Owner

Fixed the runtime blocker on the latest head (c1abd3c9): synthetic arrow keys skipped the custom Overview cards and highlighted Refresh. Scroll navigation now targets the actual provider card views with native mouse-move events.

Proof:

  • swift test --filter StatusMenuOverviewScrollTests — 9 tests passed
  • make check — SwiftFormat clean, SwiftLint 0 violations
  • autoreview — clean, no actionable findings
  • ./Scripts/compile_and_run.sh — fresh signed bundle built and launched
  • Peekaboo: scrolling down from Codex highlighted Devin and opened Devin's submenu; scrolling up from Devin highlighted Codex and opened Codex's submenu

Fresh exact-head CI is now running.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 11, 2026
@steipete
steipete force-pushed the jv/overview-scroll-highlight branch from c1abd3c to ccb17c6 Compare June 11, 2026 14:07
@steipete

Copy link
Copy Markdown
Owner

Rebased onto 1bb6d107 and revalidated exact head ccb17c60.

  • swift test --filter StatusMenuOverviewScrollTests: 9 passed
  • make check: clean
  • structured autoreview: clean, no actionable findings
  • prior fresh-bundle Peekaboo proof still applies to the unchanged PR patch: wheel navigation stays on Overview provider cards in both directions

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccb17c60f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

{
self.overviewScrollAccumulatedDelta = 0
}
self.overviewScrollAccumulatedDelta += delta

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset accumulation when scroll units change

When a menu stays open and the user switches between a precise device and a line-based wheel, this accumulator combines values measured in different units while the threshold below is chosen from only the current event. For example, a leftover 20 pixel trackpad delta followed by one wheel notch with the same sign is compared against the 0.9 line threshold and emits the capped 3 steps instead of one, so users with an external mouse/trackpad can see the highlight jump unexpectedly. Track the last hasPreciseScrollingDeltas mode and clear the remainder when it changes, or keep separate accumulators per unit.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 11, 2026
@steipete
steipete force-pushed the jv/overview-scroll-highlight branch from ccb17c6 to c15ee72 Compare June 11, 2026 14:53
@steipete

Copy link
Copy Markdown
Owner

Rebased onto current main and revalidated the exact branch.

Proof:

  • swift test --filter StatusMenuOverviewScroll (9 passed)
  • git diff --check origin/main...HEAD
  • autoreview: clean, no actionable findings (0.82 confidence)

Reviewed head: c15ee7232b1cf28d830b003978227a7734bc750a

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 11, 2026
@steipete
steipete force-pushed the jv/overview-scroll-highlight branch from c15ee72 to 13feb0f Compare June 11, 2026 15:43
@steipete

Copy link
Copy Markdown
Owner

Rebased on current main (88c43eeb) and pushed head 13feb0f3.

Proof:

  • swift test --filter StatusMenuOverviewScrollTests: 9 tests passed
  • make check: passed, 0 lint violations
  • autoreview: clean, confidence 0.84
  • diff check: clean

The fix remains scoped to Overview-row scroll routing; submenu scrolling and non-Overview menus retain native event handling.

@steipete
steipete merged commit 3c2d23d into steipete:main Jun 11, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants