Replace a reply that reports a create as a change to an existing record (closes #3668)

[malibio]

A held turn refuses update_node for a record outside its offered set, and the model's fallback is a create on that record's type, which it then narrated as the update it was asked for ("The signed-off date ... has been updated"). The existing no-op guard cannot see this because the create persisted its fields.

New guard beside it: when every landed write in the turn is a create_node and a sentence of the reply claims an update (updated/changed/corrected, same per-sentence negation and question handling as contains_action_claim), the reply is replaced with a notice that the turn made a new record and left the existing one as it was, followed by the usual summary of what ran. The create stands. A turn with any other landed write, or none, is untouched.

Evidence for the rest of #3668 is on the issue: the turn was held in 3/3 reps on the merged guard change, held-on-menu-type 3/3, and held-off-menu-type's premise is broken independently (#3693).

Tests: loop-level cases (create narrated as update is replaced; honest create and a real update are left alone) and a unit test for negated/questioned sentences. test:changed passed.

🤖 Generated with Claude Code

[malibio]

Code Review Summary

Initial review (no prior reviews or comments). Verdict: approve with improvements, no blockers. The change is small, atomic, and in the established guard idiom: it sits after the no-op guard, reuses landed_write, contains_action_claim and summarize_executions, and keeps the real write while replacing the false account of it. It targets the one remaining #3668 criterion ("the reply does not report an update when the turn created a record"). The guard-extension criteria were met by #3690, and the diagnosis is recorded on the issue. The new notice does not trip contains_action_claim: "made a new record" is not "i made ", and "wasn't accurate" is not a passive claim. The later guards therefore don't misread the replacement. I did not run the test suite. The review is from the diff and the surrounding source.

Findings

Critical Issues

None.

Suggested Improvements

• [Improvement] agent_loop.rs, claims_update_after_only_creating: the trigger is a co-occurrence, not an update claim. It requires an update word and any action claim in the same sentence. The action claim can be the create claim itself, with the update word used as an adjective. Examples: "I created a follow-up with the corrected date.", "The task has been created with the updated due date.", "I've created it and changed the title to X." All of these are honest create reports, and each is replaced with an assertion that is itself false or unverifiable. The replacement says the existing record "was left as it was". This guard is also stricter than the no-op guard in one respect: it fires on a create-only turn where the user asked for a create. It is a real UX regression, and the inline test only covers the negation and question cases. Suggested fix: require the update word to be part of the claim. For example, match update-specific phrases ("has been updated", "was updated", "i updated", "i've updated", "has been changed", "was changed", "was corrected", "successfully updated") against the sentence, and subject them to the same negation, question and reporting qualifiers. Add tests for "I created a record with the corrected date" (must be left alone) and "has been created with the updated date".
• [Improvement] The replacement drops the link to the created record. The issue's observed reply linked the new record. The replacement is the notice plus "• node creation completed", so the user cannot find the record that now exists, and the notice says a record was made without naming it. Appending the created id or title, as the confirmation paths do, makes "Here's what actually ran:" useful. This is the same reasoning behind keeping the write and replacing only the account of it.
• [Improvement] guard_replacement_messages_are_not_action_claims does not list CREATED_NOT_UPDATED_NOTICE. The new constant's doc comment says it is "worded to avoid every contains_action_claim phrase", but nothing pins that, and the existing test is the pin for exactly this property. Add it to the array. The notice is safe today, but a future phrase added to the list would silently turn it into its own trigger.
• [Improvement] Notice wording asserts more than the guard knows. "A new record" is wrong when several creates landed, and "left the existing one as it was" is a claim about records the guard never examined. A failed update followed by a create is fine, but the guard also fires for a model that mislabels one field. Softer wording ("This turn only created records; it did not change an existing one.") is accurate in every case where the guard fires.

Edge cases checked

• Create plus a refused update: landed_write excludes the errored call, so the guard fires if the reply claims an update. That is correct.
• Create plus any other landed write (update_node, create_relationship): the guard is off. That is conservative and fine.
• Create where the reply honestly says an update happened by other means, or earlier in the conversation ("The date was updated earlier. I created a new task."): this is a false positive. It is rare, and it is covered by the first improvement if the claim must be an update-specific phrase.
• Sentence splitting: the guard splits on . ! ? \n and not ;, while contains_action_claim splits again including ;. This is harmless. A decimal or version such as "v1.2 updated" splits mid-token. That only narrows the match, never widens it.
• Ordering: the guard runs after the no-op guard and before tool-failure surfacing. The summary's failed-call bullets satisfy the failure-surfacing guard. A recovered retry (failure first, then create) renders as completed, which is right.

Nitpicks

• Nit: UPDATE_WORDS is declared after the early return inside the function. It reads better as a module const next to the notice.
• Nit: the run_guard_turn tests assert exact strings. That is fine, but add the false-positive cases above before merge so the guard's boundary is pinned both ways.

[malibio]

Code Review Summary

Initial review (no prior reviews or comments). Verdict: approve with improvements, no blockers. The change is small, atomic, and in the established guard idiom: it sits after the no-op guard, reuses landed_write, contains_action_claim and summarize_executions, and keeps the real write while replacing the false account of it. It targets the one remaining #3668 criterion ("the reply does not report an update when the turn created a record"). The guard-extension criteria were met by #3690, and the diagnosis is recorded on the issue. The new notice does not trip contains_action_claim: "made a new record" is not "i made ", and "wasn't accurate" is not a passive claim. The later guards therefore don't misread the replacement. I did not run the test suite. The review is from the diff and the surrounding source.

Findings

Critical Issues

None.

Suggested Improvements

• [Improvement] agent_loop.rs, claims_update_after_only_creating: the trigger is a co-occurrence, not an update claim. It requires an update word and any action claim in the same sentence. The action claim can be the create claim itself, with the update word used as an adjective. Examples: "I created a follow-up with the corrected date.", "The task has been created with the updated due date.", "I've created it and changed the title to X." All of these are honest create reports, and each is replaced with an assertion that is itself false or unverifiable. The replacement says the existing record "was left as it was". This guard is also stricter than the no-op guard in one respect: it fires on a create-only turn where the user asked for a create. It is a real UX regression, and the inline test only covers the negation and question cases. Suggested fix: require the update word to be part of the claim. For example, match update-specific phrases ("has been updated", "was updated", "i updated", "i've updated", "has been changed", "was changed", "was corrected", "successfully updated") against the sentence, and subject them to the same negation, question and reporting qualifiers. Add tests for "I created a record with the corrected date" (must be left alone) and "has been created with the updated date".
• [Improvement] The replacement drops the link to the created record. The issue's observed reply linked the new record. The replacement is the notice plus "• node creation completed", so the user cannot find the record that now exists, and the notice says a record was made without naming it. Appending the created id or title, as the confirmation paths do, makes "Here's what actually ran:" useful. This is the same reasoning behind keeping the write and replacing only the account of it.
• [Improvement] guard_replacement_messages_are_not_action_claims does not list CREATED_NOT_UPDATED_NOTICE. The new constant's doc comment says it is "worded to avoid every contains_action_claim phrase", but nothing pins that, and the existing test is the pin for exactly this property. Add it to the array. The notice is safe today, but a future phrase added to the list would silently turn it into its own trigger.
• [Improvement] Notice wording asserts more than the guard knows. "A new record" is wrong when several creates landed, and "left the existing one as it was" is a claim about records the guard never examined. A failed update followed by a create is fine, but the guard also fires for a model that mislabels one field. Softer wording ("This turn only created records; it did not change an existing one.") is accurate in every case where the guard fires.

Edge cases checked

• Create plus a refused update: landed_write excludes the errored call, so the guard fires if the reply claims an update. That is correct.
• Create plus any other landed write (update_node, create_relationship): the guard is off. That is conservative and fine.
• Create where the reply honestly says an update happened by other means, or earlier in the conversation ("The date was updated earlier. I created a new task."): this is a false positive. It is rare, and it is covered by the first improvement if the claim must be an update-specific phrase.
• Sentence splitting: the guard splits on . ! ? \n and not ;, while contains_action_claim splits again including ;. This is harmless. A decimal or version such as "v1.2 updated" splits mid-token. That only narrows the match, never widens it.
• Ordering: the guard runs after the no-op guard and before tool-failure surfacing. The summary's failed-call bullets satisfy the failure-surfacing guard. A recovered retry (failure first, then create) renders as completed, which is right.

Nitpicks

• Nit: UPDATE_WORDS is declared after the early return inside the function. It reads better as a module const next to the notice.
• Nit: the run_guard_turn tests assert exact strings. That is fine, but add the false-positive cases above before merge so the guard's boundary is pinned both ways.

[malibio]

Addressed all four suggestions and the nit: the guard now keys on update-specific claim phrases (UPDATE_CLAIMS) so "I created a follow-up with the corrected date" is left alone, with those cases pinned in tests; the replacement lists each created record with its link; the notice says only that records were created and none changed, and is in the not-an-action-claim test; UPDATE_WORDS became a module constant. Skipped: none. Re-review needed: the trigger logic and the replacement output both changed.

[malibio]

Re-review: commit f3adcad (since 6169e60)

Verdict: approve, no blockers. All four suggestions and the nit are addressed. I did not run the tests; this is from the diff and the surrounding source.

Prior findings

1. Co-occurrence trigger: fixed. UPDATE_CLAIMS is a set of update-specific phrases, and the three honest-create examples are pinned as non-triggers, plus one positive. Gaps are listed below.
2. Replacement drops the link: fixed. created_record_lines lists each landed create with a link, and the test covers two creates, including an id that already has the nodespace:// prefix.
3. CREATED_NOT_UPDATED_NOTICE missing from the not-an-action-claim test: fixed.
4. Notice wording: fixed. "only created records; it did not change an existing one" is accurate whenever the guard fires.
5. Nit (UPDATE_WORDS as a module const): fixed.

New findings

Critical

None.

Suggested Improvements

• [Improvement] UPDATE_CLAIMS coverage gaps (false negatives). The new set is deliberately narrow. It misses common model phrasings:

  • Sentence-initial or subjectless: "Updated the due date to Friday." and "Changed the title."
  • Other tenses: "have been updated", "is now updated", "I just updated", "I went ahead and updated", "successfully changed".
  • Bare "corrected" forms.

  contains_action_claim has no update-specific coverage for these either, so a reply like "Updated the date." never reaches the guard and the original misreport (A held turn answers an update request with create_node, and only the retry's exact title keeps a duplicate from being created #3668) survives. It is fine to ship the narrow set as the safe side of the trade-off. I suggest at least adding "have been updated", "i just updated", "successfully changed", and a sentence-start check for "updated "/"changed ". The last one would need a separate action-claim condition, since contains_action_claim won't fire on it.

• [Improvement, low] Residual false positive: was updated / has been updated describing the new record. "The task was created and updated with the due date." is an honest create report that still trips. It is rare, and has been updated also appears in contains_action_claim. It is acceptable, but a one-line comment on UPDATE_CLAIMS naming it as a known limit would save the next reader the analysis.

• [Improvement] Record line: untrusted title is unescaped and unbounded. create_node's content is model or user text interpolated into [{title}]({uri}).

  • A ] or ) in the title, such as "Fix [prod] bug", breaks the link syntax. It renders as a partial link or plain text, and the same applies to the markdown link regex used by the node-link guard around line 2762.
  • A multi-line or very long content (a whole note body) becomes a multi-line bullet, so the "one line per record" claim in the doc comment is false.
  • This matches the unescaped formatting at line ~1248, so it is not a regression. But this is the only place where an arbitrary note body can land inside the link text. Suggest taking the first line, truncating (e.g. char_preview(title, 80)), and replacing [/] with parentheses or backslash-escaping them.

• [Improvement, trivial] Leftover trailing newline. format!("{NOTICE}\n\n{}\n{}", summary, lines) ends with a stray \n whenever created_record_lines is empty. That happens when a landed create has no id in its result, which landed_write allows. Build the text from the non-empty parts joined with \n. Also add one test for the empty-lines case, since the current test pins only the populated one.

Checks that came out clean

• The replacement does not trip contains_action_claim, and the test pins the notice. The only later scanner of the replacement is the earlier no-tool-call guard at ~4023, which runs before this one. The title is not scanned by any later guard, so a title like "I created a thing" cannot re-trigger.
• Record lines contain only real ids from landed results, so they cannot be flagged by the fabricated-link guard.
• The summary still reports "node creation completed", and the recovered-retry behaviour is unchanged.

Nitpicks

• Nit: the doc comment for UPDATE_CLAIMS says a bare "updated" "in a sentence that also claims a create must not read as a claim of an update". It would be clearer worded as a positive: "only phrases that assert the update itself".

[malibio]

Re-review: commit f3adcad (since 6169e60)

Verdict: approve, no blockers. All four suggestions and the nit are addressed. I did not run the tests; this is from the diff and the surrounding source.

Prior findings

1. Co-occurrence trigger: fixed. UPDATE_CLAIMS is a set of update-specific phrases, and the three honest-create examples are pinned as non-triggers, plus one positive. Gaps are listed below.
2. Replacement drops the link: fixed. created_record_lines lists each landed create with a link, and the test covers two creates, including an id that already has the nodespace:// prefix.
3. CREATED_NOT_UPDATED_NOTICE missing from the not-an-action-claim test: fixed.
4. Notice wording: fixed. "only created records; it did not change an existing one" is accurate whenever the guard fires.
5. Nit (UPDATE_WORDS as a module const): fixed.

New findings

Critical

None.

Suggested Improvements

• [Improvement] UPDATE_CLAIMS coverage gaps (false negatives). The new set is deliberately narrow. It misses common model phrasings:

  • Sentence-initial or subjectless: "Updated the due date to Friday." and "Changed the title."
  • Other tenses: "have been updated", "is now updated", "I just updated", "I went ahead and updated", "successfully changed".
  • Bare "corrected" forms.

  contains_action_claim has no update-specific coverage for these either, so a reply like "Updated the date." never reaches the guard and the original misreport (#3668) survives. It is fine to ship the narrow set as the safe side of the trade-off. I suggest at least adding "have been updated", "i just updated", "successfully changed", and a sentence-start check for "updated "/"changed ". The last one would need a separate action-claim condition, since contains_action_claim won't fire on it.

• [Improvement, low] Residual false positive: was updated / has been updated describing the new record. "The task was created and updated with the due date." is an honest create report that still trips. It is rare, and has been updated also appears in contains_action_claim. It is acceptable, but a one-line comment on UPDATE_CLAIMS naming it as a known limit would save the next reader the analysis.

• [Improvement] Record line: untrusted title is unescaped and unbounded. create_node's content is model or user text interpolated into [{title}]({uri}).

  • A ] or ) in the title, such as "Fix [prod] bug", breaks the link syntax. It renders as a partial link or plain text, and the same applies to the markdown link regex used by the node-link guard around line 2762.
  • A multi-line or very long content (a whole note body) becomes a multi-line bullet, so the "one line per record" claim in the doc comment is false.
  • This matches the unescaped formatting at line ~1248, so it is not a regression. But this is the only place where an arbitrary note body can land inside the link text. Suggest taking the first line, truncating (e.g. char_preview(title, 80)), and replacing [/] with parentheses or backslash-escaping them.

• [Improvement, trivial] Leftover trailing newline. format!("{NOTICE}\n\n{}\n{}", summary, lines) ends with a stray \n whenever created_record_lines is empty. That happens when a landed create has no id in its result, which landed_write allows. Build the text from the non-empty parts joined with \n. Also add one test for the empty-lines case, since the current test pins only the populated one.

Checks that came out clean

• The replacement does not trip contains_action_claim, and the test pins the notice. The only later scanner of the replacement is the earlier no-tool-call guard at ~4023, which runs before this one. The title is not scanned by any later guard, so a title like "I created a thing" cannot re-trigger.
• Record lines contain only real ids from landed results, so they cannot be flagged by the fabricated-link guard.
• The summary still reports "node creation completed", and the recovered-retry behaviour is unchanged.

Nitpicks

• Nit: the doc comment for UPDATE_CLAIMS says a bare "updated" "in a sentence that also claims a create must not read as a claim of an update". It would be clearer worded as a positive: "only phrases that assert the update itself".

[malibio]

Addressed: record line uses the title's first line, bounded to 80 characters, brackets removed; no trailing newline when a create returned no id (tests for both); UPDATE_CLAIMS doc states its limits and the nit is reworded.
Skipped: broader phrase coverage and a sentence-start check. A bare "Updated the date." passing through is the safe direction, and widening the list risks replacing honest creates. Revisit if a measured turn shows the misreport getting past.