Skip to content

fix: state page position and size on saved comment search hits - #225

Draft
atlas-coding-agent wants to merge 1 commit into
devhims:mainfrom
atlas-coding-agent:fix/saved-comment-read-completeness
Draft

atlas-coding-agent wants to merge 1 commit into
devhims:mainfrom
atlas-coding-agent:fix/saved-comment-read-completeness

Conversation

@atlas-coding-agent

@atlas-coding-agent atlas-coding-agent commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Reported live on Oct 24 2026, 07:34 to 07:41 IST, in session https://www.video2ctx.dev/dashboard/sessions/51df11c5-33c4-4afd-af67-3896bda6340f for video WUvTyaaNkzM. Two saved provider pages hold 20 + 20 distinct top-level comments.

Run Request Expected Actual
T1 8b9c21b4-5887-4575-b8b7-829d01d2dd84 First 40 top-level comments 40 rows Answer reviewed 28. Reads returned 8, 8, 4, 8, and the last one carried COMMENT_PAGE_REMAINING
T2 e016d9e6-cc91-45fa-ba45-8c9111748a97 The missing original rows, no new fetch The 12 unread rows Only "Prepared Answer" ran. 13 rows were readable
T3 c247c6b6-24d3-45aa-a1c1-adde360c5c68, after reload The actual final 4 rows Page 2 rows 17 to 20 Named page 2 row 15 as the last row. Memory saved that claim

The saved pages were unchanged before and after reload. That proves the answer left rows out and put one in the wrong position. It does not prove which code path caused it.

I confirmed one deterministic defect. A saved comment returned by search_context carried a false read receipt. readSavedComment recorded savedPageCount: 1 for a 20-row page, and search then dropped the artifact. So the model was told the page size "was not recorded" and that it was unknown whether more entries followed. The answer model sees ref_N labels in place of positional IDs. With no receipt, row 15 and row 20 looked the same to it.

What changed

  • A comment search hit now carries the same receipt as a paged read. It gives the row's position, the page size, the provider total, the next offset with pageVersion, and whether the provider reported a further page.
  • Page 2 row 15 now reads "starts at page entry 15", followed by "5 later entries", then "no further page".
  • These hits are original saved evidence, delivered as a saved read. No analyst, reanalysis or provider fetch is involved.
  • Paging, budgets, routing, billing and storage are unchanged.

Tests

I added two integration tests to session-evidence.integration.test.ts:

  • Across two saved pages: 40 unique rows in provider order, tail rows 37 to 40, and the receipt at the 8, 8, 4, 8 stop point.
  • In a later turn after reopen: rows 1 to 8 and 37 to 40 read by pageVersion, with no provider call.
  • Search receipts for page 2 row 15, page 2 row 20 and page 1 row 1.

I could not run tests locally. This sandbox gets 403 from the npm registry, so npm ci fails. CI run 40066930218 passed all 8 jobs on this head, including Platform checks, where npm test runs these DO integration tests. The branch is based on 3d11c12. I ran no live dashboard tests.

Needs an owner decision, not fixed here

  1. Finalize turns cannot read uncited saved rows. The T2 symptom fits this path: the finalizer runs with toolChoice: 'none', and only cited packets carry forward. A follow-up without a URL stays on finalize. The options are: (a) route saved-row follow-ups to inspect_video, (b) add a saved-page read to finalize, or (c) carry whole saved pages forward. Each one changes routing or cost. I recommend (a), because it reuses existing pageVersion reads, which are priced as session reuse.
  2. T1 stopped at 28 rows, and the receipt it saw was truthful. Enforcing the requested count is a product decision.
  3. readCommentEvidence is unused in production and omits the further-page flag. I left it alone.

@vercel

vercel Bot commented Oct 24, 2026

Copy link
Copy Markdown

@kiro-agent is attempting to deploy a commit to the devhims' projects Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants