Skip to content

Make session comment focus opt-in#163

Merged
benvinegar merged 3 commits intomainfrom
fix/session-comment-focus-default
Apr 4, 2026
Merged

Make session comment focus opt-in#163
benvinegar merged 3 commits intomainfrom
fix/session-comment-focus-default

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • make hunk session comment add leave the current viewport alone by default
  • replace --no-reveal with explicit --focus help/docs and parser coverage
  • add live session regression coverage that proves only --focus changes selection and opens notes

Testing

  • bun run typecheck
  • bun test test/cli.test.ts test/session-cli.test.ts

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR flips hunk session comment add to a non-disruptive default: the live session's viewport is left alone unless the caller explicitly passes --focus, replacing the old --reveal/--no-reveal toggle pair with a single opt-in flag. The change touches the CLI parser, the session bridge hook, documentation, and both unit and integration test suites consistently.

  • src/core/cli.ts — Drops --reveal / --no-reveal, adds --focus; maps parsedOptions.focus ?? false to the reveal field so the default is now false.
  • src/ui/hooks/useHunkSessionBridge.ts — Single-line fix: reveal ?? truereveal ?? false to match the new CLI default.
  • skills/hunk-review/SKILL.md — Documents the new opt-in flag and advises agents to use --focus sparingly.
  • README.md — Adds a --focus example and a prose summary of the default behavior.
  • test/cli.test.ts — Existing test updated to verify reveal: false as default; new test confirms --focus produces reveal: true.
  • test/session-cli.test.ts — Integration test expanded with a reset → no-focus → focused sequence to prove the viewport only moves when --focus is supplied.

Confidence Score: 5/5

Safe to merge — change is additive, consistently implemented across CLI, bridge, docs, and tests, with only a minor test-reliability note.

All remaining feedback is P2 (style / test quality). The core behavioral change is correctly implemented in both the CLI parser and the session bridge hook, the internal reveal field is always a concrete boolean before it reaches the bridge, and the new integration test exercises both the default and --focus paths end-to-end. No correctness, data-integrity, or security issues were found.

test/session-cli.test.ts — the synchronous unchanged context check could be strengthened with a waitUntil guard, but does not affect production behavior.

Important Files Changed

Filename Overview
src/core/cli.ts Replaces --reveal/--no-reveal with --focus; maps focus flag to reveal field with false as new default; usage strings updated consistently
src/ui/hooks/useHunkSessionBridge.ts Single-line change: reveal ?? true → reveal ?? false to align bridge with the new opt-in focus default
test/cli.test.ts Existing default test drops --no-reveal and retains reveal: false expectation; new test verifies --focus sets reveal: true
test/session-cli.test.ts Integration test expanded to verify default comment add does not change viewport; the unchanged context check runs synchronously without waitUntil, which could produce a false-positive if the session processes the message asynchronously
skills/hunk-review/SKILL.md Docs updated from --no-reveal to --focus with guidance to use --focus sparingly
README.md README updated with --focus example and prose description of the new default behavior

Sequence Diagram

sequenceDiagram
    participant Agent as Agent / CLI
    participant CLI as cli.ts parser
    participant Bridge as useHunkSessionBridge
    participant UI as Session UI

    Agent->>CLI: hunk session comment add --file F --new-line N --summary S
    Note over CLI: focus flag absent → reveal = false
    CLI->>Bridge: { action: "comment-add", reveal: false }
    Bridge->>UI: setLiveCommentsByFileId(...)
    Note over Bridge: reveal ?? false → skip jumpToFile / openAgentNotes
    Bridge-->>Agent: { commentId, filePath, hunkIndex }
    Note over UI: Viewport unchanged

    Agent->>CLI: hunk session comment add ... --focus
    Note over CLI: focus flag present → reveal = true
    CLI->>Bridge: { action: "comment-add", reveal: true }
    Bridge->>UI: setLiveCommentsByFileId(...)
    Bridge->>UI: jumpToFile(fileId, hunkIndex)
    Bridge->>UI: openAgentNotes()
    Bridge-->>Agent: { commentId, filePath, hunkIndex }
    Note over UI: Viewport jumps to new note
Loading

Reviews (1): Last reviewed commit: "Make session comment focus opt-in" | Re-trigger Greptile

@benvinegar benvinegar merged commit 19cb532 into main Apr 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant