Commit eb55f58
committed
refactor: extract ChatTranscript with two-phase commit semantics from Session
Eliminates the catch-cleanup-rethrow pattern in Session.send and
Session.stream by moving transcript management into a new ChatTranscript
class whose API surface enforces the two-phase commit invariant by
construction.
The catch-rethrow pattern fb-contrib flagged
(THROWS_METHOD_THROWS_RUNTIMEEXCEPTION) was the symptom of
"mutate shared state, then if the call fails, undo the mutation". The
ChatTranscript design eliminates the root cause: there is no API to
commit half a round. The only round-commit method, appendRound(user,
assistant), appends both turns atomically; the wire format sent to the
model is built via messagesWithPendingUserTurn(...) which returns a
fresh list WITHOUT mutating the transcript. On model failure, the
caller never reaches appendRound — no rollback logic is required.
Side benefit — testable as running documentation. Extracting the
transcript management decouples the invariant from LlamaModel (whose
static initializer loads the native library and is unmockable in
test environments without the native library). The new
ChatTranscriptTest exercises the invariant with 12 tests across two
categories:
@nested "mechanical API behaviour" (8 tests):
- appendRound commits both turns atomically
- appendUserTurn + appendAssistantTurn match appendRound
- messagesWithPendingUserTurn does NOT mutate the transcript
- messagesWithPendingUserTurn returns a fresh list each call
- snapshot includes the system message when configured
- snapshot omits the system message when null or empty
- snapshot is unmodifiable
- getSystemMessage returns null when absent
@nested "two-phase commit pattern - running documentation" (4 tests):
- fresh transcript untouched when model throws
- existing transcript byte-for-byte unchanged when model throws
- success commits user + assistant atomically
- stream() shape - user turn only, assistant follows via
commitStreamedReply
Each two-phase test composes the ChatTranscript API the same way
Session.send and Session.stream do, so reading the test doubles as
documentation of the design contract.
Net:
- New file: src/main/java/net/ladenthin/llama/ChatTranscript.java
- New file: src/test/java/net/ladenthin/llama/ChatTranscriptTest.java
- Session.java: 247 lines -> 247 lines (no net change; delegates to
ChatTranscript). The catch-rethrow blocks are gone; the
buildParamsWithPendingUserTurn helper now delegates to
ChatTranscript.messagesWithPendingUserTurn.
SpotBugs Max+Low:
- THROWS_METHOD_THROWS_RUNTIMEEXCEPTION goes 2 -> 0 by design, not
by suppression. Cross-repo lifecycle TODO for BAF (PR #4087) can
take inspiration from this refactor on AbstractProducer.produceKeys
if/when that two-phase commit fits.
- IMC_IMMATURE_CLASS_NO_EQUALS goes 2 -> 3 (one new finding on the
new ChatTranscript class — identity-by-design like the existing
CancellationToken and ChatRequest sites; will fold into the
existing class-level suppression block).
Total jllama: 13 -> 12.
Test slice green: 117 tests across ChatTranscriptTest (12),
ChatMessageTest (2), ChatResponseTest (7), InferenceParametersTest
(86), LlamaArchitectureTest (10).1 parent d31a906 commit eb55f58
3 files changed
Lines changed: 468 additions & 41 deletions
File tree
- src
- main/java/net/ladenthin/llama
- test/java/net/ladenthin/llama
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | | - | |
9 | 7 | | |
10 | 8 | | |
11 | 9 | | |
| |||
45 | 43 | | |
46 | 44 | | |
47 | 45 | | |
48 | | - | |
49 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
50 | 54 | | |
51 | 55 | | |
52 | 56 | | |
| |||
86 | 90 | | |
87 | 91 | | |
88 | 92 | | |
89 | | - | |
| 93 | + | |
90 | 94 | | |
91 | 95 | | |
92 | 96 | | |
| |||
101 | 105 | | |
102 | 106 | | |
103 | 107 | | |
104 | | - | |
| 108 | + | |
105 | 109 | | |
106 | 110 | | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
117 | 120 | | |
118 | 121 | | |
119 | 122 | | |
| |||
131 | 134 | | |
132 | 135 | | |
133 | 136 | | |
134 | | - | |
| 137 | + | |
135 | 138 | | |
136 | 139 | | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
146 | 147 | | |
147 | 148 | | |
148 | 149 | | |
| |||
157 | 158 | | |
158 | 159 | | |
159 | 160 | | |
160 | | - | |
| 161 | + | |
161 | 162 | | |
162 | 163 | | |
163 | | - | |
| 164 | + | |
164 | 165 | | |
165 | 166 | | |
166 | 167 | | |
| |||
176 | 177 | | |
177 | 178 | | |
178 | 179 | | |
179 | | - | |
| 180 | + | |
180 | 181 | | |
181 | 182 | | |
182 | 183 | | |
| |||
194 | 195 | | |
195 | 196 | | |
196 | 197 | | |
197 | | - | |
| 198 | + | |
198 | 199 | | |
199 | 200 | | |
200 | 201 | | |
| |||
207 | 208 | | |
208 | 209 | | |
209 | 210 | | |
210 | | - | |
211 | | - | |
212 | | - | |
213 | | - | |
214 | | - | |
215 | | - | |
216 | | - | |
217 | | - | |
| 211 | + | |
218 | 212 | | |
219 | 213 | | |
220 | 214 | | |
| |||
226 | 220 | | |
227 | 221 | | |
228 | 222 | | |
229 | | - | |
230 | | - | |
231 | | - | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
232 | 238 | | |
233 | 239 | | |
234 | 240 | | |
| |||
0 commit comments