Skip to content

Fix commit message loading fallback#360

Open
Farhan (fkb032) wants to merge 15 commits into
developfrom
fkb/eng-485-commit-message
Open

Fix commit message loading fallback#360
Farhan (fkb032) wants to merge 15 commits into
developfrom
fkb/eng-485-commit-message

Conversation

@fkb032

Copy link
Copy Markdown
Contributor

Summary

Fixes the Save/Commit step getting stuck on Loading... when the backend has no stored generated commit message yet.

  • Treat missing generated_commit_message as expected pending state instead of a backend error.
  • Keep the commit subject editable and show a deterministic fallback after the pending state goes stale.
  • Preserve user edits when a generated message arrives late.
  • Prevent empty/whitespace commit submits while the suggestion is still loading.
  • Add focused unit coverage for pending, fallback, late-ready, error, commit reset, churn, and empty-submit behavior.

Test Plan

  • bun run test:unit src/components/widget/layout/merge-section.test.tsx
  • bun run test:unit
  • cargo test --manifest-path apps/native/src-tauri/Cargo.toml summarize::pipelines::commit_message
  • cargo test --manifest-path apps/native/src-tauri/Cargo.toml
  • bun run build
  • bunx oxlint apps/native/src/components/widget/layout/merge-section.tsx apps/native/src/components/widget/layout/merge-section.test.tsx apps/native/src/hooks/use-summary.ts apps/native/src/ipc/api.ts
  • git diff --check
  • Tauri WDIO smoke: bun run test:wdio:smoke passed earlier in this worktree before the final empty-submit guard change.
  • Actual app visual smoke: seeded real app-support state/SQLite with generated_commit_message = NULL; the live nixmac app reached Save step and showed fallback subject/helper text instead of staying on Loading....
  • $claude-review Opus 4.8 code review: no blocking issues; accepted/fixed the empty-submit finding.

Docs

  • No docs update needed

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🎨 Storybook preview

Open Storybook preview

Updated for 6689d2b

@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: 2b718cc40a

ℹ️ 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 (@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 (@codex) address that feedback".

Comment on lines +164 to +166
store.setCommitMessageSuggestion(null);
store.setEvolvePrompt("");
resetCommitMessageDraft();

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 Preserve the draft when commit fails

When tauriAPI.git.commit fails (for example a hook rejects the commit or git returns an error), handleCommit catches the exception and only records the error in use-git-operations.ts, so the await handleCommit({ message }) here still resolves. These reset calls therefore run on failed commits too, clearing the user's edited subject/body and stored suggestion while they remain on the commit step and need to retry.

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

📋 PR Overview

Lines changed 804 (+772 / -32)
Files 4 added, 6 modified, 0 deleted
Draft / WIP no
Has Test Plan yes
No Test Plan Needed no
New UI components no
New Storybook stories no
New Rust modules no
New TS source files yes (1)
New tests yes (2)
package.json touched no
Cargo.toml touched no
Infra / CI touched no

🔬 Coverage

Report Lines Statements Functions Branches
apps/native/coverage/coverage-summary.json 28.3% 28.3% 26.3% 20.4%

Generated by 🚫 dangerJS against 6689d2b

@fkb032 Farhan (fkb032) force-pushed the fkb/eng-485-commit-message branch from 2b718cc to e26f997 Compare June 9, 2026 23:10

@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: e26f9975af

ℹ️ 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 (@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 (@codex) address that feedback".

Comment on lines +124 to +126
if (statusRef.current !== "loading") {
return;
}

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 stale suggestions when lookup goes pending

When the Save/Commit step already has a generated suggestion and the underlying changeMap changes to a state whose backend lookup returns pending (for example, the user edits files after the suggestion was generated and no generated_commit_message exists for the new hashes yet), this branch exits because statusRef is still "ready". The old subject/body remain enabled and can be committed for a different set of changes instead of showing loading/fallback for the new map; useSummary.generateCommitMessage now returns pending on a null backend result, so this is the path for a missing current suggestion.

Useful? React with 👍 / 👎.

@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: 267b4f5f8d

ℹ️ 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 (@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 (@codex) address that feedback".

Comment on lines +77 to +79
const suggestionFingerprintRef = useRef<string | null>(
commitMessageSuggestion ? changeMapFingerprintRef.current : null,
);

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 Revalidate stored suggestions on remount

When MergeSection remounts with a non-null commitMessageSuggestion, this initializes the suggestion fingerprint to the current change map even though the store value may have been generated for an earlier map. CommitStep/ManualCommitStep unmount this component when the user clicks “Continue editing”, so if files/changeMap change while editing and the user clicks “Back to commit”, the stale global suggestion is treated as ready for the new fingerprint; if the backend lookup is pending, the statusRef.current !== "loading" path keeps the old subject/body indefinitely and enables committing it for the new changes. The suggestion needs to be cleared or revalidated on mount instead of assuming it matches the current map.

Useful? React with 👍 / 👎.

@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: 9e2fc09a9a

ℹ️ 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 (@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 (@codex) address that feedback".

.getState()
.setCommitMessageSuggestion(result.message);
setCommitMessageStatus("ready");
setCommitBody(parsed.body);

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 Preserve manual one-line commits when suggestions arrive

If the user types their own subject while the lookup is still pending, a later generated suggestion does not overwrite that subject but this still unconditionally stores the generated body. Because the body is not editable, submitting after that point sends custom subject\n\n<generated body> even though the user only authored a one-line message, which can produce mismatched or unintended commit messages in the late-ready path.

Useful? React with 👍 / 👎.

…-message

# Conflicts:
#	apps/native/src-tauri/src/evolve/edit_nix_file.rs
#	apps/native/src-tauri/src/evolve/tools.rs
#	apps/native/src-tauri/src/evolve/tools/edit_nix_file.rs
#	apps/native/src-tauri/src/state/mod.rs
…-message

# Conflicts:
#	apps/native/scripts/run-storybook-tests-ci.mjs
#	apps/native/src-tauri/src/evolve/tools/edit_nix_file.rs

@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: 0bb8fbfeb0

ℹ️ 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 (@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 (@codex) address that feedback".

Comment on lines +168 to +170
staleTimer = setTimeout(() => {
showFallback("fallback");
}, STALE_COMMIT_MESSAGE_MS);

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 Cancel stale fallback timers when suggestions arrive

When generateCommitMessage() returns pending, this schedules an 8-second fallback that is not cancelled if commitMessageSuggestion is populated before the timeout fires. In that case the second effect accepts the real suggestion and marks the draft ready, but this timer still calls showFallback("fallback") later and overwrites the generated subject/body with the fallback text for the same change map. Guard the timeout against a non-loading status or clear it when a suggestion is accepted.

Useful? React with 👍 / 👎.

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.

As mentioned in the inner comment, this is too complex, I can't even understand what the expected behavior is from looking at the code, even less the correctness of the implementation. I suspect there is a problem of design (missing rationalization of the expected behavior) and in any case the implementation should be refactored and simplified to make it intelligible and to properly represent such specification.

import { useEffect } from "react";
import { useEffect, useRef, useState } from "react";

const STALE_COMMIT_MESSAGE_MS = 8_000;

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.

What's this? This needs a comment.

return hashes.sort().join("\0");
}

export function MergeSection() {

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.

It is impossible for me to understand for me the correctness of this component now, there are 3 different useState plus 8 different useRef, massive useEffect with all kinds of asynchrnous ping-pong, and no comments. Even Claude is struggling to follow and tell me what it's doing, and only the tests are difficult to understand.

I think this requires extracting the logic parts into a hook, for example, a useCommitMessageDraft(changeMap) hook returning { subject, body, status, setSubject, reset } that makes all the commit message dance testable and specifiable in separately from the UI. It should also be documented.

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.

3 participants