Skip to content

Fix conversation engine editing bugs (versioning, RetireDefinition, llm update) - #249

Merged
ktjn merged 17 commits into
mainfrom
worktree-conversation-engine-editing-fixes
Jul 29, 2026
Merged

Fix conversation engine editing bugs (versioning, RetireDefinition, llm update)#249
ktjn merged 17 commits into
mainfrom
worktree-conversation-engine-editing-fixes

Conversation

@ktjn

@ktjn ktjn commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the shared LLM conversation/editing engine (used by CLI chat, VS Code, and the web playground) so edit requests either succeed or fail with a reason the user understands, instead of silently no-oping, throwing confusing internal errors, or behaving differently across surfaces.

  • Session-scoped draft continuation: a ref created earlier in a conversation (already applied) stays editable without requiring a version bump, threaded through WorkspaceEditor → ConversationBackend → ConversationEngine.
  • RetireDefinition (an operation the LLM could propose that always threw, since the .mdl language has no retirement construct yet) is no longer offered to the LLM; the offline heuristic correctly classifies retire/deprecate requests instead of misreading them as queries.
  • The planner's system prompt now documents draft-mode/version-bump rules for every mutating operation kind (not just field additions) and CEL join/filter syntax.
  • engine.py's conflict handling (behind the separate modelable llm update CLI command) now raises instead of silently skipping conflicting edits, matching the rest of the codebase.
  • modelable llm update is fully rewired off its own separate schema/heuristic parsing onto the shared ConversationSession/WorkspaceEditor path (via a new direct_edit_mode flag). It now requires a configured LLM provider — an explicitly approved behavior change, since the removed offline heuristic had no real NL understanding.
  • CLI chat and the VS Code conversation service now surface a proactive "no provider configured" notice up front instead of only on the first failed edit attempt.

See docs/superpowers/specs/2026-07-28-conversation-engine-editing-fixes-design.md and docs/superpowers/plans/2026-07-28-conversation-engine-editing-fixes.md for the full design and implementation plan.

Test plan

  • Full test suite passes: 1573 passed, 21 skipped
  • ruff format --check / ruff check clean
  • mypy baseline ratchet clean (no new errors)
  • Every task went through implementer → task-reviewer → fix-loop-where-needed
  • Final whole-branch review (cross-task consistency, integration gaps) clean after one fix wave (stale mypy baseline, stale doc, a new mypy error, and an added end-to-end session-continuity test)

🤖 Generated with Claude Code

ktjn and others added 17 commits July 28, 2026 19:23
@ktjn @claude
docs: add conversation engine editing fixes design
21722c1 
Covers the shared-engine failures found while investigating "LLM edits
don't work": offline mode's silent edit no-ops, RetireDefinition's
guaranteed throw, cross-turn versioning rejections, incomplete planner
prompt guidance, and the engine.py/workspace_editor.py conflict-handling
split. First of three sub-projects (engine fixes, retirement language
feature, web playground UI parity).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn
docs: add conversation engine editing fixes implementation plan
3407a01
@ktjn
feat(llm): let WorkspaceEditor accept session-scoped editable refs
c45b0d5
@ktjn
feat(llm): thread session_editable_refs through both conversation bac…
c9c3793 
…kends

Adds a session_editable_refs keyword parameter to ConversationBackend.preview_source_change
and forwards it to WorkspaceEditor.preview/.apply in the filesystem and browser backends,
so a future ConversationEngine caller can supply session-tracked editable refs on every turn.
@ktjn @claude
test(llm): cover session_editable_refs on BrowserConversationBackend
3e1f7e7 
Add a direct test constructing BrowserConversationBackend and calling
preview_source_change with a non-default session_editable_refs to
exercise the forwarding in browser/conversation.py:98-100. Mirrors the
existing FilesystemConversationBackend coverage and confirms the same
plan is rejected without the kwarg.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
feat(llm): let ConversationEngine carry editable refs across turns in…
64a2983 
… a session

Accumulate refs from every successfully applied change set into
_session_editable_refs, and pass the accumulated set into
backend.preview_source_change on every subsequent turn, so a ref
created in one turn stays editable in a later turn of the same
session without a version bump.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
fix(llm): stop offering retire_definition until the language supports it
4a208a4 
RetireDefinition is a real ChangeSetPlan operation type but
workspace_editor.py unconditionally rejects it since the .mdl language
has no definition-level retirement construct yet. Stop the LLM planner
from proposing it: add a schema-filtering helper that strips an
operation kind from the JSON schema's discriminator mapping, oneOf
list, and now-orphaned $defs entry, wire it into the planner's request
building to exclude retire_definition, and make the offline heuristic
classify retire/deprecate requests as UnsupportedPlan instead of a
query.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn
docs(llm): document draft-mode rules for every operation and CEL join…
85eac2f 
…/filter syntax
@ktjn @claude
fix(llm): raise on conflicting field edits in engine.py instead of si…
6832dae 
…lently skipping

Conflict branches in _apply_model_change, _apply_projection_change,
_apply_model_update, and _apply_projection_update now raise ValueError
for already-exists/not-found cases instead of appending a warning and
continuing, matching workspace_editor.py's hard-error behavior for the
same conflicts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn
test(llm): cover offline-heuristic conflict raise for model and proje…
c5f35e2 
…ction updates

_apply_model_update and _apply_projection_update in engine.py raise ValueError
on a conflicting field add, but only the provider-driven path had coverage.
Add tests that call update_definition with provider=None so the no-provider
heuristic branch (update_definition around line 350/364) exercises both
functions directly.
@ktjn
feat(llm): add direct_edit_mode so a caller can pin edits to a named …
ec7558f 
…ref in draft mode
@ktjn @claude
feat(llm): rewire modelable llm update through the shared Conversatio…
a24f8d4 
…nSession/WorkspaceEditor path

Replaces update_definition()'s separate UpdatePlan/UpdateChange schema and heuristic
regex-parsing with the shared ConversationSession/WorkspaceEditor path already used by
CLI chat, VS Code, and the web playground, driven in direct_edit_mode so the planner
rewrites the named ref in place instead of appending a version. modelable llm update now
requires a configured LLM provider (the offline heuristic fallback is gone).

Also fixes ConversationReply.assumptions never being populated from
PendingChangeSet.assumptions in preview_source_change, which update_definition needs to
surface change-set assumptions as UpdateResult.warnings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
fix(llm): honor --output redirect, report real repair count, and test…
81c443b 
… direct_edit_mode forwarding

Code review on the ConversationSession/WorkspaceEditor rewire found three issues:

- update_definition always applied to the workspace source even when --output
  pointed elsewhere, silently mutating the source with no provenance record.
  Restore the old "redirect, don't touch source" semantics: when output is set
  and differs from source_path, discard the change set instead of applying it
  and write only to output.
- diagnostics_repaired was hardcoded to 0 after the rewire, so audit sidecars
  falsely reported zero repairs. _ProviderResponseTracker now counts provider
  completions per turn and exposes repairs_used, which update_definition
  surfaces as UpdateResult.diagnostics_repaired.
- direct_edit_mode forwarding (ConversationSession.turn -> begin_turn, and
  update_definition -> session.turn) had no test coverage; deleting
  direct_edit_mode=True from engine.py would leave the suite green. Added a
  session-level test capturing PlannerContext.direct_edit_mode via a
  CapturingPlanner, and an update_definition test asserting the direct-edit
  instruction line reaches the LLM request.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
feat(llm): surface a proactive no-provider notice in CLI chat and the…
95d2b29 
… VS Code conversation service

Adds ConversationSession.no_provider_notice, printed once at CLI chat
startup and prepended to the first-turn reply of newly created LSP
conversation sessions, so the offline-mode limitation is visible
upfront instead of discovered after a failed edit attempt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
test(lsp): assert no-provider notice omitted on second turn
a0c4af5 
Closes a coverage gap flagged in review of 95d2b29: the plan requires
the no-provider notice to prepend only on the first turn of a new
session, but only the positive case had a regression test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
fix: regenerate mypy baseline, fix new type error, update stale docs,…
00d533c 
… add end-to-end session-continuity test

- Re-baseline cli/mypy-baseline.txt for the 4 pre-existing engine.py errors
  that drifted line numbers after a later commit shifted the file; branch
  was red on CI's mypy ratchet gate.
- Fix a genuinely new mypy error in conversation_plan.py's
  _exclude_operation_kinds (schema.get("$defs") typed as object) instead of
  baselining it; also drop a redundant re-fetch of the same dict.
- Update docs/cli-reference.md's stale description of `modelable llm update`
  offline fallback, which Task 7 removed in favor of a hard required-provider
  error.
- Add an end-to-end test exercising the real ConversationSession ->
  FilesystemConversationBackend -> WorkspaceEditor chain across two turns,
  proving session_editable_refs survives a real turn boundary (not just
  fakes at each layer), plus the negative case for an untouched ref.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn @claude
test(web): add required assumptions/changed/affected fields to chat m…
5d35bf6 
…essage fixtures

AssistantGenerateChatMessage gained these fields in a prior PR but several
test fixtures across ChatPanel.test.tsx and App.test.tsx were never
updated, leaving main red (type-check failures plus runtime crashes in
AssumptionsList reading .length off undefined). Pre-existing on main,
unrelated to this branch's changes; fixed here since it was blocking this
PR's CI.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ktjn
ktjn merged commit 20d9d5a into main Jul 29, 2026
9 checks passed
@ktjn
ktjn deleted the worktree-conversation-engine-editing-fixes branch July 29, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ktjn