feat(use-apply): pre-generate commit msg before activation - #628
feat(use-apply): pre-generate commit msg before activation#628cooper (czxtm) wants to merge 1 commit into
Conversation
Fire generateCommitMessage() as an unawaited side effect at the start of handleApply — the diff is final before activation starts, and the save panel can regenerate on failure. An inference failure must not block or delay activation. Merge-section's existing effect must not clobber the in-flight suggestion: short-circuit when commitMessageSuggestion is already set.
🎨 Storybook previewUpdated for e317587
|
|
| Scenario | With this change | Without this change |
|---|---|---|
| Build slower than inference | Message ready on save-panel open | Message generated after save-panel open |
| Build faster than inference | Two concurrent generations (current behavior) | One generation after save-panel open |
| Build fails | Inference ran but is discarded; next Apply regenerates | No inference ran |
The probabilistic nature is worth acknowledging but does not change the recommendation — degraded behavior is identical to the status quo.
2. An inference failure is silently swallowed and never surfaces to the user during the apply flow; the save panel's regenerate button is the recovery path.
Commit message suggestion is an enhancement on top of the save flow, not a gating requirement. If the LLM call fails, activation still completes, the save panel still opens, and the user can regenerate manually. Blocking or surfacing an error for a non-critical enhancement would create disproportionate friction. The trade-off is that users may occasionally open the save panel with no pre-populated suggestion and no explanation — but a regenerate affordance makes this graceful.
3. The save panel skips re-generation when a suggestion is already present, preventing it from clobbering an in-flight or completed pre-generated message.
The guard correctly prevents the common double-inference case: if Apply-time inference finishes before the save panel opens, the existing suggestion is preserved and no redundant call is made. Adding commitMessageSuggestion to the effect dependency array means the guard correctly re-arms if the suggestion is later cleared (e.g., by a user-triggered regenerate).
The concern is narrow: the guard is implemented as a React effect early-return, which means it only fires once the component mounts. If the save panel is mounted while generation is still in flight (suggestion is still null), the guard does not fire and merge-section starts its own generation. This is the same race noted in the main decision — the guard handles the 'inference faster than build' case well but offers no protection in the reverse case.
4. A stale commit message suggestion from a failed Apply attempt persists until the user clicks Apply again, at which point the new apply-click call regenerates it.
If Apply fails and the user edits their config before trying again, the suggestion from the first attempt sits in state. When they click Apply again, useApply fires generateCommitMessage() unconditionally, which updates the suggestion regardless of what was there before. The save panel's merge-section short-circuit fires for any in-progress inference, but the direct call from useApply runs independently of the effect and is the source of truth. The user sees the correct message by the time the save panel opens. No action needed.
5. The save panel's regenerate handler is unchanged, meaning it continues to work identically as the fallback for any pre-generation miss or failure.
The PR correctly treats regeneration as the invariant safety net and does not disturb it. Every failure mode — inference timeout, network error, build-faster-than-inference race, Apply failure followed by config change — ultimately resolves via the same regenerate button that existed before. The change adds an optimization path without narrowing the fallback.
Open questions
-
When the user clicks Apply and it fails, the suggestion from that attempt remains in state. If the user makes config changes before retrying, what does the save panel show between the moment Apply is clicked (second time) and when the new inference completes? Is there a visible flash of the old message?
-
Is there a maximum time the user waits on the save panel before giving up and regenerating manually? If inference is slow (>5s), does the panel indicate that generation is in progress, or does it appear empty until the result arrives?
-
Does
generateCommitMessage()without theclearoption overwrite an existing suggestion in place, or does it clear it first and then write? If it clears first, there is a window where the save panel shows an empty field on every new Apply, undoing the optimization's user-visible benefit.
Recommendation
Ship it
The optimization is directionally correct: moving inference earlier reduces latency on the happy path, and every failure mode degrades gracefully to the existing behavior with no regression. The guard prevents the most common double-inference case. The open questions are about edge-case UX polish rather than structural correctness.
![prelint[bot]](https://avatars.githubusercontent.com/in/2822344?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
apps/native/src/components/widget/layout/merge-section.tsx (line 32) [behavior_change]: Adding commitMessageSuggestion to the useEffect dependency array creates a duplicate-request loop during regeneration. generateCommitMessage() uses clear: true by default (use-summary.ts line 18), which synchronously sets the suggestion to null; that null triggers the effect again (short-circuit on line 26 does not fire for falsy values), launching a second concurrent network call alongside the one already started by handleRegenerate. Pre-PR, clicking Regenerate fired exactly one call; post-PR it fires two concurrent calls on every regeneration, with the last-response-wins outcome being silent and untested.
📋 PR Overview
🔬 Coverage
|
Juanpe Bolívar (arximboldi)
left a comment
Juanpe Bolívar (arximboldi)
left a comment
There was a problem hiding this comment.
LGTM!
2 participants
Fire generateCommitMessage() as an unawaited side effect at the start of
handleApply — the diff is final before activation starts, and the save
panel can regenerate on failure. An inference failure must not block or
delay activation.
Merge-section's existing effect must not clobber the in-flight
suggestion: short-circuit when commitMessageSuggestion is already set.