engine: put an interface at the subprocess seam - #3
Merged
Merged
Conversation
Behaviour-preserving. Every non-test change is a rename or argument plumbing, and no code compares the engine against nil, so promoting a pointer to an interface cannot change a branch. Engine is now the vocabulary the UI speaks: seven calls, nothing about the wire format. claudeEngine is the implementation. The UI already worked in `entry` values and knew nothing about NDJSON, so this only writes down a boundary that was there. It pays for itself immediately. fakeEngine drives the send path with no subprocess, which is what lets a test assert the thing an earlier self-review found unpinned: the transcript shows what the user typed, and the wire also carries the standing-instruction reminder. Deleting the withReminder call from sendTurn now fails a test instead of passing quietly. newModel takes a launchConfig rather than six positional arguments. Four of them were adjacent strings, where transposing a mode and a session id is silent, and a new setting shifted every caller along.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Behaviour-preserving refactor. No feature change.
What
Engine(backend.go) is now the vocabulary the UI speaks — seven calls, nothing about the wire format.claudeEngineis the implementation. The UI already worked inentryvalues and knew nothing about NDJSON, so this writes down a boundary that already existed.newModeltakes alaunchConfig(launch.go) instead of six positional arguments. Four were adjacent strings, so transposing a mode and a session id was silent.Why it is not speculative surface
The interface has a second implementation in this diff.
fakeEnginedrives the send path with no subprocess, which is what letsTestSendTurnAppliesTheReminderToTheWireOnlypin something an earlier self-review found unpinned: the transcript shows the typed text while the wire also carries the reminder. Verified discriminating — deleting thewithRemindercall fromsendTurnfails the test.Behaviour claim, checked
Every non-test hunk is a rename or argument plumbing. Nothing compares the engine against nil (
grep 'engine == nil\|engine != nil'is empty), so promoting a pointer to an interface cannot flip a branch — the usual trap here.Not verified
Tests and
go vetonly. I have not driven the built binary through a live turn, an approval and a diff.