Page back to a bounded read's window instead of filtering one page (comms-rldp) - #221
Draft
GraemeF wants to merge 3 commits into
Draft
Page back to a bounded read's window instead of filtering one page (comms-rldp)#221GraemeF wants to merge 3 commits into
GraemeF wants to merge 3 commits into
Conversation
Zulip's GET /messages selects a page by anchor and count and has no timestamp predicate, so since and until never reached the realm: they filtered whatever a newest-anchored page happened to return. A window lying below that page came back empty while looking authoritative. readWindow walks back one anchored page at a time until the window's lower bound is crossed, the cap is met, or history runs out. It is sequential by construction, dedupes by id because a Zulip anchor is a range hint that returns a neighbour rather than an exact match, and maps each page against the query that fetched it, which is what the rendered-content read has to re-issue. readChannel, readThread and the boot catch-up path all go through it. An unbounded read still asks for one page and gets the newest limit.
read_channel and read_thread promised a result "bounded by optional since/until/limit". The since/until half is true now that a window is read out of history. The limit half is not what it sounds like: it cuts from the old end, so a result holding exactly limit messages may be missing older ones inside the window it was asked for, and looks identical to a complete read. Both tool descriptions and the limit field now say that, and name the check that separates the two cases: compare the oldest message returned against since. agent-experience.md gains the divergence rather than losing one. A human scrolling back sees where they stopped; an agent whose read is capped does not.
The directory read is forked so it overlaps the walk rather than sitting in front of it, and the join lives where the mapping needs it. A window that turns out empty maps no page, so nothing joined and a /users failure was dropped with the interrupted fiber. The read then answered [] — the same shape as a channel with nothing in the window. Both call sites now join unconditionally before returning.
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
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.
Zulip's
GET /messagesselects a page by anchor and count and has no timestamp predicate, sosinceanduntilnever reached the realm. They filtered whatever a newest-anchored page happened to return. A window lying below that page came back empty, and an empty result looks the same as a channel with nothing in it.comms-rldpcarries the measured runs.readWindowin the newpackages/zulip/history-paging.tswalks back one anchored page at a time until the window's lower bound is crossed, the cap is met, or history runs out.history.readChannel,history.readThreadandinbox.replayall go through it.Three properties the walk has to hold, each already established somewhere in the tree:
renderedContentForBatchre-issues the query it is handed, so mentions have to be resolved per page against that page's own query. Rendering once for the whole walk would index one page and report no mentions on every other.What
limitmeans nowIt is still a cap and it still cuts from the old end, keeping the newest. What changed is that the walk keeps going while the cap is unmet and the bound is unreached, which is the case that returned the false zero.
So a bounded read that hits
limitis still sheared at the old end of its window, and looks identical to a complete one. That is the commoner failure of the two, since anything sought near a window's start sits exactly there. Both tool descriptions and thelimitfield now say so, and name the check that separates the cases: compare the oldest message returned againstsince.docs/agent-experience.mdgains it as a divergence, since a human scrolling back sees where they stopped.Calls worth a look
limitand its budget is one — the request shape is unchanged. A bounded read pages atmax(limit, 100), because pages it walks past cost a request each and alimitof 3 would otherwise crawl.HISTORY_MAX_PAGESis 20. A window far enough back to outrun it returns a short read rather than walking the realm.limitstops the common case long before it./usersread was otherwise dropped with the interrupted fiber and answered[]./messagesresponses. The walk asks a third time, so it now has a sticky empty page behind the script, the way a realm would. Its assertion is untouched.inbox.replay.REPLAY_NUM_BEFOREis now the page size rather than the ceiling, so the roughly two days of realm history it covered is no longer a limit on catch-up.Closes
comms-rldp.