Skip to content

refactor(ui): isolate current review refresh - #873

Merged
benvinegar merged 2 commits into
mainfrom
refactor/current-review-refresh
Aug 27, 2026
Merged

refactor(ui): isolate current review refresh#873
benvinegar merged 2 commits into
mainfrom
refactor/current-review-refresh

Conversation

@benvinegar

@benvinegar benvinegar commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

  • extract current-review request derivation into a neutral UI module
  • move manual and watch refresh coordination into useCurrentReviewRefreshController
  • preserve AppHost reload/remount authority and in-session view state

Testing

  • bun run typecheck
  • bun run deps:check
  • bun run lint
  • bun run format:check
  • focused refresh, reload, watch, workspace, trust, and editor tests
  • bun test test/pty/watch.test.ts

This PR description was generated by Pi using GPT-5.6 Sol

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 27, 2026 2:04am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR extracts current-review descriptor derivation, refresh operations, and watch coordination from App into a dedicated controller while preserving host-owned reload serialization.

  • Adds pure helpers for retaining live view options in refresh requests.
  • Adds a controller hook for registration, manual refreshes, and watched refreshes.
  • Adds focused unit and hook tests for descriptor replacement, error handling, watch behavior, and non-reloadable inputs.

Confidence Score: 4/5

The PR appears safe to merge after the non-blocking new-file naming convention issue is addressed.

The extracted refresh controller preserves watcher teardown, current-descriptor targeting, and host reload serialization; the only accepted concern is that newly added TypeScript files do not follow the required dash-case naming convention.

Files Needing Attention: src/ui/hooks/useCurrentReviewRefreshController.ts, src/ui/hooks/useCurrentReviewRefreshController.test.tsx, src/ui/currentReviewRefresh.ts

Important Files Changed

Filename Overview
src/ui/App.tsx Replaces inline refresh and watcher orchestration with the extracted controller while retaining existing consumers.
src/ui/currentReviewRefresh.ts Adds pure descriptor and view-option derivation helpers; behavior is covered by focused tests, but the filename violates the repository naming convention.
src/ui/hooks/useCurrentReviewRefreshController.ts Centralizes descriptor registration and stable manual/watch refresh callbacks; no behavioral regression was established, but the new hook filename violates the naming convention.
src/ui/hooks/useCurrentReviewRefreshController.test.tsx Covers descriptor updates, callback stability, rejection handling, watch provenance, and inert non-reloadable inputs; its filename repeats the naming violation.
src/ui/AppHost.tsx Updates the refresh request type import without changing host reload behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  App[App view state] --> Controller[Current review refresh controller]
  Controller --> Descriptor[Refresh descriptor]
  Controller --> Watch[Watched input]
  Descriptor --> Host[AppHost reload queue]
  Watch --> Host
  Write[Workspace write completion] --> Host
  Host --> Reload[Soft review reload]
Loading
Prompt To Fix All With AI
### Issue 1
src/ui/hooks/useCurrentReviewRefreshController.ts:1
**Use dash-case module filenames**

The new hook, its colocated test, and `currentReviewRefresh.ts` use camelCase filenames instead of the repository-required dash-case convention, conflicting with the filename linting policy and establishing an inconsistent module pattern.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "refactor(ui): isolate current review ref..." | Re-trigger Greptile

@@ -0,0 +1,114 @@
/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Use dash-case module filenames

The new hook, its colocated test, and currentReviewRefresh.ts use camelCase filenames instead of the repository-required dash-case convention, conflicting with the filename linting policy and establishing an inconsistent module pattern.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/hooks/useCurrentReviewRefreshController.ts
Line: 1

Comment:
**Use dash-case module filenames**

The new hook, its colocated test, and `currentReviewRefresh.ts` use camelCase filenames instead of the repository-required dash-case convention, conflicting with the filename linting policy and establishing an inconsistent module pattern.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar
benvinegar force-pushed the refactor/current-review-refresh branch from 5fe7c6d to 946f005 Compare August 27, 2026 01:46
@benvinegar
benvinegar merged commit 6d6646e into main Aug 27, 2026
12 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