[Rui Grier (ruigrier)]

Summary

Fixes #441. ENG-580.

When the agent calls done without making edits, nixmac currently opens Review/Save with an empty diff. This also happens if the agent ran a successful build check first.

Mark no-edit completions as Conversational so the existing lifecycle displays the agent's explanation on Describe and skips the review/save workflow. Require edits for the build-verified Generated path; otherwise, a successful build check would bypass the no-edit handling. Completions with verified edits still enter the normal review workflow.

Test Plan

• Added a regression test covering done without edits after no build check, a successful build check, and a failed build check. It checks the conversational state, preserved summary, and loop termination. The regression test failed before the fix.
• Added a test confirming that done with verified edits still produces Generated. Existing tests continue to cover rejection after a failed build.
• Ran the native test suite on a MacBook Air (M2, 2023) running macOS Tahoe 26.6.2: 1,214 passed, 10 ignored. The focused done_ run passed all six matching tests.
• Passed workspace Clippy with warnings denied and cargo fmt --all -- --check, using the development shell's Rust/Cargo 1.96.0.
• Ran the patched app with a controlled localhost provider calling done without edits. The explanation appeared on Describe; Review/Save did not open.
• Repeated the app demonstration with successful build checks before done. The result remained conversational, with the explanation visible on Describe.
• Verified the isolated configuration repository remained clean at the same commit. Restored the previous app state after testing; no system activation was performed.

The demonstrations use deterministic provider responses with the real Rust agent, tool execution, lifecycle, and frontend.

no-edits-direct.mp4

no-edits-build-check.mp4

Docs

• Docs updated (companion PR in darkmatter/nixmac-web: pending)

This changes agent-loop completion behavior. The companion documentation requirement is still unresolved: darkmatter/nixmac-web returned HTTP 404 with my current GitHub access.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[prelint]

No-edit completions stay conversational

Product decisions in this change

1. When the agent finishes with no edits, the user sees the agent's explanation on the Describe screen. The Review and Save screen no longer opens with an empty diff.

An empty review screen asks the user to approve nothing. It looks like a bug. Showing the explanation answers the user's question directly. This is how chat-style assistants handle "nothing to change". It also matches the existing path for genuine conversational replies, so the user learns no new concept.

2. A successful build check no longer turns a no-edit finish into a reviewable result. The result needs at least one edit.

A passing build check on an unchanged configuration proves nothing new for the user. Treating it as a reviewable result opened an empty review screen. The new rule is simple: no changes means no review. Finishes with verified edits still enter the normal review flow, and a new test covers that case.

3. A claim that the request is already satisfied and a normal conversational reply look the same to the user.

The improvement plan names this mapping as the minimal fix. A typed already-satisfied outcome is the full version. The mapping is better than the old empty review screen. The cost is that "Tailscale is already configured" looks like any chat answer, and the user cannot tell it from an answer to a question. The PR does not add a label or any other signal on Describe. This is a UI change, so the team can change it later without moving user data. The risk is trust, not migration.

4. The agent's claim that a request is already satisfied reaches the user as free text with no evidence (host, setting, source file).

The plan documents cases where the agent found a setting in a module that the target host does not use. The agent then said the request was done. Before this PR, the user at least saw an empty review and might notice something was wrong. Now a false claim is a clean, confident reply, and the user may stop looking. This PR does not create the risk, but it makes this path the normal one for no-edit finishes. The follow-up work on structured evidence should land soon. Until then, a short note in the reply that points to the file and the host would help.

Since the last review

• Still open: Already-satisfied completions and genuine conversational replies look identical to the user. (The diff only changes the state to Conversational. It adds no label or typed outcome.)
• Still open: The already-satisfied claim carries no structured evidence the user can check. (The diff passes the free-text summary through. No host, setting, or source file is attached.)
• Still open: Does Describe show any signal that separates a no-edit finish from a normal exchange? (No UI change in the diff or in the PR conversation.)
• Still open: Is there a committed timeline for the typed already-satisfied outcome? (The PR description and conversation give no date or tracking issue.)
• Still open: Can the agent reach the no-edit path through an inactive-module false positive, and is that acceptable? (The PR does not change how the agent checks the target host. The planned reachability work is not in this diff.)
• Still open: The companion docs PR is blocked by a 404, so no plan exists for user documentation. (The PR description still lists docs as pending and blocked by access.)

Open questions

• Does the Describe screen show any label that tells the user a no-edit finish is different from a normal reply?

• Is there a date for the typed already-satisfied outcome with evidence (host, setting, source file)?

• Can the agent still declare a request done based on a module that the target host does not import? Does the team accept that until the follow-up lands?

• How will users learn why the agent sometimes makes no changes, given that the companion docs PR is blocked by an access error?

Recommendation

Ship with changes
The core fix is right: users should not review an empty diff. The gap is trust. A false "already done" claim now looks like a normal answer. The PR is already merged, so the team should track the evidence follow-up and a Describe label as near-term work.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Changes done completions without edits to Conversational, including after a successful build check.

• No-edit completions finish as conversational replies.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["Agent calls done"] --> B{"Any edits?"}
  B -->|No| C["Conversational: show explanation"]
  B -->|Yes| D{"Build verified?"}
  D -->|Yes| E["Generated: review changes"]
  D -->|No| F["Reject done; continue agent"]

Loading

_{Reviews (2) · Last reviewed commit: "fix(evolve): keep no-edit completions co..." · Reviewed by Greptile}

[cooper (czxtm)]

apologies for the delay - as you can see I'm doing very thorough testing to automated code review more and your PR is one of the test subjects. This looks good, I'm merging.

[cooper (czxtm)]

Rui Grier (@ruigrier)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Juanpe Bolívar (arximboldi)]

Hi Rui Grier (@ruigrier)! thank you for your contribution. I specifically appreciated the inclusion of videos demonstrating the fix.

I am approving the fix and will merge when CI passes (somehow needed to reopen the PR to be able to approve CI running).

[cooper (czxtm)]

Thanks for the fix. A few optional follow-ups, none blocking:

• A done with no edits after a failing build_check now ends as a normal chat reply, so the build error is never shown to the user (evolve/mod.rs#L2665-L2668). Surfacing last_build_error in that case would avoid hiding a broken base config, and done_without_edits_is_conversational_regardless_of_build_result currently locks in the present behavior.
• The done arm tests has_edits() in two separate branches (L2621 and L2631). Testing !has_edits() first would state the invariant directly.
• The docs checkbox in the PR body is still open for the nixmac-web companion change.