From cdf33a5170bed2797ed7271219a81139adaeb39c Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Thu, 24 Sep 2026 15:43:16 -0400 Subject: [PATCH 1/2] fix(s014): the prompt window is the recent conversation, not the first 500 rows A session holding more than five hundred messages was building every subsequent request from its first five hundred. Store.history/2 orders ascending and then limits, so it returns the oldest rows; build_request/1 called it with limit: 500. Past that point the assistant answered from the opening of the session with the current exchange absent from the request entirely, and compaction does not delete rows, so any long session reached that point by design rather than by accident. It explains a second symptom. Compactor.plan/1 derives covered_to from the latest compaction in the window it is handed, so a window pinned to the oldest rows let a plan re-cover ground and record two compactions with the same to_seq, spending a model call to learn nothing. The fix is a second accessor rather than a change to the first. recent_history/2 takes the newest rows descending and reverses them, so the prompt reads oldest to newest as a conversation must; history/2 keeps its forward paging for the interface and the export, with a test asserting it at a row count where the two differ. Two callers with opposite needs sharing one function is how this happened. No existing test reached above the window, which is why it went unseen: the longest ran 200 turns and only crossed the line once compaction itself added 195 rows. The new test works at 520 rows throughout, because below the window the defect is invisible by definition. Proven by mutation rather than by the fix passing. Reverting recent_history/2 to ascending makes the new test report the defect in its own failure message: the prompt window ends at seq 500 of 520. Found by accident at slice 110, which registered one tool and pushed a small test window past a threshold. The tool surface is not the problem and this slice does not treat it as one: the defect was there at any window size, waiting for a session to pass five hundred rows. mix gate: 750 passed (14 properties, 736 tests), 18 excluded. Signed-off-by: Ayla Croft --- CHANGELOG.md | 21 ++++++ coverage.tsv | 1 + lib/trinity/sessions.ex | 8 ++ lib/trinity/sessions/session.ex | 5 +- lib/trinity/sessions/store.ex | 24 ++++++ test/trinity/sessions/prompt_window_test.exs | 78 ++++++++++++++++++++ 6 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 test/trinity/sessions/prompt_window_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index dac5f3b..e34da9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,27 @@ evidence for each increment is retained by the maintainers and summarised here. ## 2026-09-24 +### `slice/014` — The prompt window is the recent conversation +A session that held more than five hundred messages was building every subsequent request from its +**first** five hundred. The query that fetched the window ordered oldest-first and then took a +limit, which is right for the interface and the export, which page forwards through a conversation, +and wrong for building a prompt. Past that point the assistant was answering from the opening of the +session with the current exchange absent from the request entirely. Compaction does not delete rows, +so any long-running session reached that point by design rather than by accident. + +It explains a second symptom too. Compaction decides what still needs summarising from the most +recent compaction *in the window it is given*, so a window pinned to the oldest rows let it +re-summarise ground it had already covered, spending a model call to learn nothing. + +The fix is a second accessor rather than a change to the first: the prompt and the compactor ask for +the most recent rows in reading order, and the forward pager keeps its behaviour, with a test saying +so. Two callers with opposite needs sharing one function is how this happened. + +Found by accident. Slice 110 registered one tool, which pushed a deliberately small test window past +a threshold, which made compaction frequent enough to take a session past five hundred rows for the +first time in any test. Proven by reverting the fix and watching the new test report the defect in +its own failure message, rather than by the fix passing. + ### `slice/004` — Property-based testing, and the assertions it enables The canonicalisers, the argument validator, the tighten-only state modifier and the receipt chain are now exercised by generated input rather than only by chosen input. That matters most where the code diff --git a/coverage.tsv b/coverage.tsv index c95dcd1..75ae532 100644 --- a/coverage.tsv +++ b/coverage.tsv @@ -31,3 +31,4 @@ slice_id percent sha date 029 80.31 16e42ad 2026-09-24 120 80.31 78ee950 2026-09-24 004 80.31 91de943 2026-09-24 +014 80.31 1a5d480 2026-09-24 diff --git a/lib/trinity/sessions.ex b/lib/trinity/sessions.ex index 7b4b57d..3a5321e 100644 --- a/lib/trinity/sessions.ex +++ b/lib/trinity/sessions.ex @@ -63,6 +63,14 @@ defmodule Trinity.Sessions do @spec history(session_id(), keyword()) :: [Message.t()] def history(session_id, opts \\ []), do: Store.history(session_id, opts) + @doc """ + The most recent `limit` rows, ascending: the window a prompt is built from (slice 014). + + Not `history/2`, which pages forwards from the beginning and returns the oldest rows when limited. + """ + @spec recent_history(session_id(), keyword()) :: [Message.t()] + def recent_history(session_id, opts \\ []), do: Store.recent_history(session_id, opts) + @doc "Marks a session archived." @spec archive(SessionRow.t()) :: {:ok, SessionRow.t()} | {:error, Ecto.Changeset.t()} def archive(%SessionRow{} = session), do: Store.update_session(session, %{status: "archived"}) diff --git a/lib/trinity/sessions/session.ex b/lib/trinity/sessions/session.ex index 8aeebfb..c47e385 100644 --- a/lib/trinity/sessions/session.ex +++ b/lib/trinity/sessions/session.ex @@ -355,7 +355,10 @@ defmodule Trinity.Sessions.Session do persona = session.persona_id && Store.get_persona(session.persona_id) # Slice 020: the declared surface of this turn, into the request and onto the row. tools = Trinity.Tools.to_llm_tools() - history = Trinity.Sessions.history(id, limit: 500) + # Slice 014: the **recent** 500 rows, not the first 500. `history/2` orders ascending and then + # limits, so past 500 rows it would hand this the opening of the session for ever and the + # current exchange would not be in the request at all. + history = Trinity.Sessions.recent_history(id, limit: 500) # Slice 033: the project's AGENTS.md, read now, so a change is in this turn (live reload). context = [ diff --git a/lib/trinity/sessions/store.ex b/lib/trinity/sessions/store.ex index b40da94..7ca85a2 100644 --- a/lib/trinity/sessions/store.ex +++ b/lib/trinity/sessions/store.ex @@ -113,6 +113,30 @@ defmodule Trinity.Sessions.Store do |> Repo.all() end + @doc """ + The most recent `limit` rows, in ascending order (slice 014). + + `history/2` above orders ascending and then limits, so it returns the **oldest** rows: it is the + forward pager the interface and the export walk a session with, and it is right for that. It is + wrong for building a prompt. A session that holds more rows than the window would otherwise be + answered from its opening, with the current exchange missing from the request entirely, and + compaction handed the same stale window would re-summarise ground it had already covered. + + Descending with a limit takes the newest; the reverse puts them back in the order a conversation + has to be read in. + """ + @spec recent_history(String.t(), keyword()) :: [Message.t()] + def recent_history(session_id, opts) do + limit = Keyword.get(opts, :limit, 200) + + Message + |> where([m], m.session_id == ^session_id) + |> order_by([m], desc: m.seq) + |> limit(^limit) + |> Repo.all() + |> Enum.reverse() + end + @spec get_persona(String.t()) :: Persona.t() | nil def get_persona(id), do: Repo.get(Persona, id) diff --git a/test/trinity/sessions/prompt_window_test.exs b/test/trinity/sessions/prompt_window_test.exs new file mode 100644 index 0000000..dc110ce --- /dev/null +++ b/test/trinity/sessions/prompt_window_test.exs @@ -0,0 +1,78 @@ +# SPDX-FileCopyrightText: Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 +defmodule Trinity.Sessions.PromptWindowTest do + @moduledoc """ + Slice 014 AC1 and AC2: the window a prompt is built from is the **recent** conversation. + + No existing test reaches a session longer than the window, which is why this went unseen. Every + assertion here is at a row count above it, because below it the defect is invisible by definition. + """ + use Trinity.DataCase, async: false + + alias Trinity.Sessions + + @window 500 + @rows 520 + + defp session_with_rows!(n) do + session = Trinity.Factory.session!() + + rows = + for seq <- 1..n do + now = DateTime.utc_now() + + %{ + id: Trinity.UUID.generate(), + session_id: session.id, + seq: seq, + role: if(rem(seq, 2) == 1, do: "user", else: "assistant"), + content: "message #{seq}", + parts: %{"text" => "message #{seq}"}, + inserted_at: now, + updated_at: now + } + end + + Trinity.Repo.insert_all(Trinity.Sessions.Message, rows) + session + end + + test "AC1: the window ends at the newest row, not the five-hundredth" do + session = session_with_rows!(@rows) + + seqs = Sessions.recent_history(session.id, limit: @window) |> Enum.map(& &1.seq) + + assert List.last(seqs) == @rows, + "the prompt window ends at seq #{List.last(seqs)} of #{@rows}. Past the window the " <> + "assistant answers from the opening of the session with the current exchange missing" + + assert length(seqs) == @window + assert List.first(seqs) == @rows - @window + 1 + end + + test "AC2: the window reads oldest to newest, because a conversation is an order" do + session = session_with_rows!(@rows) + seqs = Sessions.recent_history(session.id, limit: @window) |> Enum.map(& &1.seq) + + assert seqs == Enum.sort(seqs), + "the window came back newest-first, which would present the conversation backwards" + end + + test "a session shorter than the window is unchanged by any of this" do + session = session_with_rows!(10) + seqs = Sessions.recent_history(session.id, limit: @window) |> Enum.map(& &1.seq) + assert seqs == Enum.to_list(1..10) + end + + test "AC3: history/2 still pages forwards from the beginning, for the callers that walk a session" do + session = session_with_rows!(@rows) + + first_page = Sessions.history(session.id, limit: 10) |> Enum.map(& &1.seq) + second_page = Sessions.history(session.id, limit: 10, offset: 10) |> Enum.map(& &1.seq) + + assert first_page == Enum.to_list(1..10), + "history/2 is the forward pager the UI and the export use; this slice must not change it" + + assert second_page == Enum.to_list(11..20) + end +end From 685abe867f461fb95ce4dbf529dd5224aa74e518 Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Thu, 24 Sep 2026 15:52:58 -0400 Subject: [PATCH 2/2] fix(s090): the trace test waits for the turn, not for the model call The FIPS leg failed on slice 014's branch while its sibling run of the same commit passed. That is R26's signature, and R26's own counter-example entry says the signature is not the diagnosis, so the stack trace was read first. It was not R26. Trinity.TelemetryTest, two turns are two traces, sent its second user message as soon as it received the llm call stop event from the first. That event fires when the model call ends, not when the turn does: the session is still thinking while it writes the result, so the second message returned {:error, {:busy, :thinking}}. It has been racy since slice 090 and passed on the ordinary gate for days because that leg is faster where it matters. The FIPS leg is about half again as slow, which is enough to lose the race reliably. The test now waits for idle, which is what it meant: two turns is a claim about two turns, and the end of a model call is not the end of a turn. A poll against a deadline rather than a sleep, so it flunks naming the state it was stuck in rather than trading one race for a slower one. Signed-off-by: Ayla Croft --- test/trinity/telemetry/telemetry_test.exs | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/trinity/telemetry/telemetry_test.exs b/test/trinity/telemetry/telemetry_test.exs index aafa8b3..15e906b 100644 --- a/test/trinity/telemetry/telemetry_test.exs +++ b/test/trinity/telemetry/telemetry_test.exs @@ -170,12 +170,38 @@ defmodule Trinity.TelemetryTest do {:ok, _} = Sessions.send_user_message(session.id, "first") assert_receive {^ref, [:trinity, :llm, :call, :stop], _, first}, 5_000 + # The stop event fires when the model call ends, and the session is still `:thinking` while it + # writes the result. Sending the next message on that event is a race: it passed on the + # ordinary gate for weeks and failed on the FIPS leg, which is slower, with + # `{:error, {:busy, :thinking}}`. Waiting for idle is what the test meant all along. + await_idle(session.id) + {:ok, _} = Sessions.send_user_message(session.id, "second") assert_receive {^ref, [:trinity, :llm, :call, :stop], _, second}, 5_000 refute first.trace_id == second.trace_id end + defp await_idle(id, timeout \\ 5_000) do + deadline = System.monotonic_time(:millisecond) + timeout + await_idle(id, deadline, :first) + end + + defp await_idle(id, deadline, _) do + case Sessions.state(id) do + %{state: :idle} -> + :ok + + other -> + if System.monotonic_time(:millisecond) > deadline do + flunk("session #{id} was #{inspect(other)} and never became idle") + else + Process.sleep(10) + await_idle(id, deadline, :again) + end + end + end + test "outside a trace an event carries no trace id, rather than an invented one" do ref = make_ref() me = self()