A fork inherits the transcript, not the bill for having produced it - #125
Conversation
`fork` copied `usage` along with the messages, and `Stats.aggregate` sums that field over the rows of the index. So every fork added an entire history to a grand total that had only ever been spent once: fork a long session three times and `smith stats` reports roughly four times its cost. The copy was always there. What changed under it was the meaning of the field: before #102 `usage` held the *last run*, and a fork inherited one run's tokens — small, and defensible. #102 made it the session's *lifetime*, and the same line quietly began inheriting all of it. Zero, not a subtraction. `parent_id` does exist — contrary to the issue's note, `fork` has always set it — so the alternative was reachable: keep the copy and have `aggregate` discount inherited usage once per fork. It is more machinery to arrive at a number that is already available by not writing it down, and "what this session spent" is the only reading of the field that a sum over sessions can be taken of. The price is named rather than swallowed, in the code, the README and the changelog: a fork's own COST column does not show what its inherited context costs on the first request. That is an understatement of one request against an overstatement of a whole history, and only one of the two can be added up. Spec forks, runs a turn on each side and checks the total, and fails without the change. Closes #117 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ks stay wrong Review found the named trade-off backwards, in all three places it was named — and the issue says the same thing, which is where I took it from. "A fork's COST column does not show what its inherited context costs on the first request" is false. `build_agent` takes the baseline from the very field `fork` stopped copying, so it is zero; the agent is built with the whole inherited transcript; the first response's prompt tokens cover all of it; `persist` writes `0 + that`. Re-sending what it inherited is billed to the fork and lands in its own column. Two different things were run together: the cost of *producing* a transcript, which stays with the parent and was the double count, and the cost of *re-sending* it, which is the fork's and always was. Nothing about the code changes. The sentence did, in the comment, the README and the changelog, because a wrong description of a cost figure is the same defect as a wrong cost figure, one level up. Also named rather than implied: forks written before this keep the inherited number. It does not decay, it compounds, since each later run persists on top of it. Not migrated on purpose — `parent_id` says which session a fork came from, but nothing recorded what that session had spent *at the time of the fork*, and the parent has run since, so any correction would be a guess. Second spec, at the CLI level, where #117 meets #102: fork, resume, one turn. It asserts the fork's own 120, the parent's untouched 240, and 360 as the sum over the index. Fails without the change with `Expected: 120, got: 360` — the store-level spec proves what `fork` writes, this proves what a resumed fork does with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ries rather than keeps Two more from review, both on sentences I had already corrected once. "The first response's prompt tokens cover all of it" is the shorthand this codebase specifically warns against — `Usage#billed_prompt_tokens` exists because `prompt_tokens` alone is the *uncached remainder*. A fork is the warm-cache case by construction: its transcript is byte-identical to the one the parent just sent, so most of it comes back as `cache_read_tokens`. The conclusion survives, since `Pricing.cost` prices all four fields, but the mechanism named was wrong, and naming a wrong mechanism in the paragraph about not misreporting costs is not a small thing. "Keeps the inherited number" reads as frozen. It is not: the baseline for every later run is read from that same field, so a legacy fork re-adds the excess on every save and stays over by exactly the inherited amount for the life of the session. "Carries", with the mechanism, and with the one thing a reader will ask next — there is no way to reset it short of deleting the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Review durch einen Reviewer-Agenten, drei Runden. Erste REQUEST CHANGES, danach zweimal APPROVE WITH NITS — und alle Befunde betrafen die Prosa, nicht den Code. Der Code ist seit der ersten Fassung unverändert. Runde 1: der benannte Preis war rückwärtsKriterium 3 des Issues verlangt, den Preis auszusprechen statt ihn stillschweigend festzulegen. Ich hatte ihn ausgesprochen — und falsch:
Das stimmt nicht. Nachgerechnet: Ich hatte zwei Dinge vermengt: die Kosten, ein Transkript zu erzeugen — die beim Elternteil bleiben und der Doppelzähler waren — und die Kosten, es erneut zu senden, die dem Fork gehören und immer schon dort standen. Das Issue selbst behauptet denselben Fehler („die Kostenzeile eines Forks unterschlägt, was der geerbte Kontext beim ersten Request kostet"). Ich habe ihn ungeprüft übernommen. Eine falsche Beschreibung einer Kostenzahl ist derselbe Defekt wie eine falsche Kostenzahl, eine Ebene höher — in einem Projekt, dessen Regel „eine falsche Kostenzahl ist schlimmer als gar keine" lautet. Runde 2: derselbe Satz, feinerer FehlerMeine Korrektur sagte „the first response's prompt tokens cover all of it". Das ist die Abkürzung, vor der
Und ein Fork ist der Warm-Cache-Fall par excellence: sein Transkript ist byteidentisch mit dem, was das Elternteil gerade gesendet hat. Das meiste kommt als Ebenfalls aus Runde 2: „keeps the inherited figure" liest sich wie eingefroren. Ist es nicht. Die Baseline jedes späteren Laufs wird aus demselben Feld gelesen, also addiert ein alter Fork den Überschuss bei jedem Speichern erneut und bleibt dauerhaft genau um den geerbten Betrag zu hoch. Jetzt „carries", mit Mechanismus — und mit der Antwort auf die nächste Frage: außer die Session zu löschen gibt es keinen Weg zurück. Zweiter Spec, auf CLI-EbeneDer Store-Spec belegt, was Der Reviewer hat beide Specs gegengeprüft: die Aggregat-Zeile allein fängt den Bug auch dann, wenn man die Einzelzusicherungen darüber löscht ( Was gegengeprüft und sauber warJeder Konsument von
AusgegliedertDer Reviewer fand nebenbei, dass |
1 participant
Closes #117
Session::Store#forkcopiedusagealong with the messages, andStats.aggregatesums that field over the rows of the index. Every fork therefore added an entire history to a grand total that had only ever been spent once.Why now, and not before
The copy was always there. What changed under it is the meaning of the field. Before #102
usageheld the last run, so a fork inherited one run's tokens — small, and defensible. #102 made it the session's lifetime, and the same line quietly began inheriting all of it. This is #102's own bug, arriving a release later.One correction to the issue
It does —
forkhas always setparent_id: source.id(session.cr:501), andIndexEntrycarries it too. So variant 2 was reachable: keep the copy, haveaggregatediscount inherited usage once per fork.It is still the wrong one. It is more machinery to arrive at a number already available by not writing it down, it makes every consumer of the index responsible for knowing about forks, and "what this session spent" is the only reading of the field that a sum over sessions can be taken of at all. Variant 1 as decided, but the reason in the issue was not the true one.
The price, named
Criterion 3 asked for this to be said out loud rather than settled silently. A fork's own
COSTcolumn does not show what its inherited context costs on the first request. That is an understatement of one request, against an overstatement of an entire history — and only one of the two can be added up. Said in three places: the comment where the field is not copied, the README'ssmith forkparagraph, and the changelog.Not touched
context_ratiois still inherited. It is not a cost, it is what was learned about measuring the transcript, and the fork inherits the transcript — the reason the line above it gives is unchanged and still right.Spec
Forks, runs a turn on each side, and checks the sum over the index — criterion 2 verbatim. Verified failing with the copy restored:
crystal spec: 1427 examples, 0 failures.crystal tool format --check: clean.🤖 Generated with Claude Code