Skip to content

Commit a1738db

Browse files
authored
Merge pull request #95 from ScriptKittyOS/slice/014-prompt-window-is-recent
Slice 014: the prompt window is the recent conversation, not the first 500 rows
2 parents 1a5d480 + 685abe8 commit a1738db

7 files changed

Lines changed: 162 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,27 @@ evidence for each increment is retained by the maintainers and summarised here.
2424

2525
## 2026-09-24
2626

27+
### `slice/014` — The prompt window is the recent conversation
28+
A session that held more than five hundred messages was building every subsequent request from its
29+
**first** five hundred. The query that fetched the window ordered oldest-first and then took a
30+
limit, which is right for the interface and the export, which page forwards through a conversation,
31+
and wrong for building a prompt. Past that point the assistant was answering from the opening of the
32+
session with the current exchange absent from the request entirely. Compaction does not delete rows,
33+
so any long-running session reached that point by design rather than by accident.
34+
35+
It explains a second symptom too. Compaction decides what still needs summarising from the most
36+
recent compaction *in the window it is given*, so a window pinned to the oldest rows let it
37+
re-summarise ground it had already covered, spending a model call to learn nothing.
38+
39+
The fix is a second accessor rather than a change to the first: the prompt and the compactor ask for
40+
the most recent rows in reading order, and the forward pager keeps its behaviour, with a test saying
41+
so. Two callers with opposite needs sharing one function is how this happened.
42+
43+
Found by accident. Slice 110 registered one tool, which pushed a deliberately small test window past
44+
a threshold, which made compaction frequent enough to take a session past five hundred rows for the
45+
first time in any test. Proven by reverting the fix and watching the new test report the defect in
46+
its own failure message, rather than by the fix passing.
47+
2748
### `slice/004` — Property-based testing, and the assertions it enables
2849
The canonicalisers, the argument validator, the tighten-only state modifier and the receipt chain are
2950
now exercised by generated input rather than only by chosen input. That matters most where the code

‎coverage.tsv‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ slice_id percent sha date
3131
029 80.31 16e42ad 2026-09-24
3232
120 80.31 78ee950 2026-09-24
3333
004 80.31 91de943 2026-09-24
34+
014 80.31 1a5d480 2026-09-24

‎lib/trinity/sessions.ex‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ defmodule Trinity.Sessions do
6363
@spec history(session_id(), keyword()) :: [Message.t()]
6464
def history(session_id, opts \\ []), do: Store.history(session_id, opts)
6565

66+
@doc """
67+
The most recent `limit` rows, ascending: the window a prompt is built from (slice 014).
68+
69+
Not `history/2`, which pages forwards from the beginning and returns the oldest rows when limited.
70+
"""
71+
@spec recent_history(session_id(), keyword()) :: [Message.t()]
72+
def recent_history(session_id, opts \\ []), do: Store.recent_history(session_id, opts)
73+
6674
@doc "Marks a session archived."
6775
@spec archive(SessionRow.t()) :: {:ok, SessionRow.t()} | {:error, Ecto.Changeset.t()}
6876
def archive(%SessionRow{} = session), do: Store.update_session(session, %{status: "archived"})

‎lib/trinity/sessions/session.ex‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,10 @@ defmodule Trinity.Sessions.Session do
355355
persona = session.persona_id && Store.get_persona(session.persona_id)
356356
# Slice 020: the declared surface of this turn, into the request and onto the row.
357357
tools = Trinity.Tools.to_llm_tools()
358-
history = Trinity.Sessions.history(id, limit: 500)
358+
# Slice 014: the **recent** 500 rows, not the first 500. `history/2` orders ascending and then
359+
# limits, so past 500 rows it would hand this the opening of the session for ever and the
360+
# current exchange would not be in the request at all.
361+
history = Trinity.Sessions.recent_history(id, limit: 500)
359362
# Slice 033: the project's AGENTS.md, read now, so a change is in this turn (live reload).
360363
context =
361364
[

‎lib/trinity/sessions/store.ex‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,30 @@ defmodule Trinity.Sessions.Store do
113113
|> Repo.all()
114114
end
115115

116+
@doc """
117+
The most recent `limit` rows, in ascending order (slice 014).
118+
119+
`history/2` above orders ascending and then limits, so it returns the **oldest** rows: it is the
120+
forward pager the interface and the export walk a session with, and it is right for that. It is
121+
wrong for building a prompt. A session that holds more rows than the window would otherwise be
122+
answered from its opening, with the current exchange missing from the request entirely, and
123+
compaction handed the same stale window would re-summarise ground it had already covered.
124+
125+
Descending with a limit takes the newest; the reverse puts them back in the order a conversation
126+
has to be read in.
127+
"""
128+
@spec recent_history(String.t(), keyword()) :: [Message.t()]
129+
def recent_history(session_id, opts) do
130+
limit = Keyword.get(opts, :limit, 200)
131+
132+
Message
133+
|> where([m], m.session_id == ^session_id)
134+
|> order_by([m], desc: m.seq)
135+
|> limit(^limit)
136+
|> Repo.all()
137+
|> Enum.reverse()
138+
end
139+
116140
@spec get_persona(String.t()) :: Persona.t() | nil
117141
def get_persona(id), do: Repo.get(Persona, id)
118142

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# SPDX-FileCopyrightText: Sudo Apt Holdings LLC
2+
# SPDX-License-Identifier: Apache-2.0
3+
defmodule Trinity.Sessions.PromptWindowTest do
4+
@moduledoc """
5+
Slice 014 AC1 and AC2: the window a prompt is built from is the **recent** conversation.
6+
7+
No existing test reaches a session longer than the window, which is why this went unseen. Every
8+
assertion here is at a row count above it, because below it the defect is invisible by definition.
9+
"""
10+
use Trinity.DataCase, async: false
11+
12+
alias Trinity.Sessions
13+
14+
@window 500
15+
@rows 520
16+
17+
defp session_with_rows!(n) do
18+
session = Trinity.Factory.session!()
19+
20+
rows =
21+
for seq <- 1..n do
22+
now = DateTime.utc_now()
23+
24+
%{
25+
id: Trinity.UUID.generate(),
26+
session_id: session.id,
27+
seq: seq,
28+
role: if(rem(seq, 2) == 1, do: "user", else: "assistant"),
29+
content: "message #{seq}",
30+
parts: %{"text" => "message #{seq}"},
31+
inserted_at: now,
32+
updated_at: now
33+
}
34+
end
35+
36+
Trinity.Repo.insert_all(Trinity.Sessions.Message, rows)
37+
session
38+
end
39+
40+
test "AC1: the window ends at the newest row, not the five-hundredth" do
41+
session = session_with_rows!(@rows)
42+
43+
seqs = Sessions.recent_history(session.id, limit: @window) |> Enum.map(& &1.seq)
44+
45+
assert List.last(seqs) == @rows,
46+
"the prompt window ends at seq #{List.last(seqs)} of #{@rows}. Past the window the " <>
47+
"assistant answers from the opening of the session with the current exchange missing"
48+
49+
assert length(seqs) == @window
50+
assert List.first(seqs) == @rows - @window + 1
51+
end
52+
53+
test "AC2: the window reads oldest to newest, because a conversation is an order" do
54+
session = session_with_rows!(@rows)
55+
seqs = Sessions.recent_history(session.id, limit: @window) |> Enum.map(& &1.seq)
56+
57+
assert seqs == Enum.sort(seqs),
58+
"the window came back newest-first, which would present the conversation backwards"
59+
end
60+
61+
test "a session shorter than the window is unchanged by any of this" do
62+
session = session_with_rows!(10)
63+
seqs = Sessions.recent_history(session.id, limit: @window) |> Enum.map(& &1.seq)
64+
assert seqs == Enum.to_list(1..10)
65+
end
66+
67+
test "AC3: history/2 still pages forwards from the beginning, for the callers that walk a session" do
68+
session = session_with_rows!(@rows)
69+
70+
first_page = Sessions.history(session.id, limit: 10) |> Enum.map(& &1.seq)
71+
second_page = Sessions.history(session.id, limit: 10, offset: 10) |> Enum.map(& &1.seq)
72+
73+
assert first_page == Enum.to_list(1..10),
74+
"history/2 is the forward pager the UI and the export use; this slice must not change it"
75+
76+
assert second_page == Enum.to_list(11..20)
77+
end
78+
end

‎test/trinity/telemetry/telemetry_test.exs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,38 @@ defmodule Trinity.TelemetryTest do
170170
{:ok, _} = Sessions.send_user_message(session.id, "first")
171171
assert_receive {^ref, [:trinity, :llm, :call, :stop], _, first}, 5_000
172172

173+
# The stop event fires when the model call ends, and the session is still `:thinking` while it
174+
# writes the result. Sending the next message on that event is a race: it passed on the
175+
# ordinary gate for weeks and failed on the FIPS leg, which is slower, with
176+
# `{:error, {:busy, :thinking}}`. Waiting for idle is what the test meant all along.
177+
await_idle(session.id)
178+
173179
{:ok, _} = Sessions.send_user_message(session.id, "second")
174180
assert_receive {^ref, [:trinity, :llm, :call, :stop], _, second}, 5_000
175181

176182
refute first.trace_id == second.trace_id
177183
end
178184

185+
defp await_idle(id, timeout \\ 5_000) do
186+
deadline = System.monotonic_time(:millisecond) + timeout
187+
await_idle(id, deadline, :first)
188+
end
189+
190+
defp await_idle(id, deadline, _) do
191+
case Sessions.state(id) do
192+
%{state: :idle} ->
193+
:ok
194+
195+
other ->
196+
if System.monotonic_time(:millisecond) > deadline do
197+
flunk("session #{id} was #{inspect(other)} and never became idle")
198+
else
199+
Process.sleep(10)
200+
await_idle(id, deadline, :again)
201+
end
202+
end
203+
end
204+
179205
test "outside a trace an event carries no trace id, rather than an invented one" do
180206
ref = make_ref()
181207
me = self()

0 commit comments

Comments
 (0)