fix(persona): read a listing to the end, rather than to the first page - #199
Merged
Conversation
`personaAttributeList`, `personaProfileList` and `listBindings` asked for a cursor-paginated listing and returned the first page, dropping `nextCursor`. The specification names that inference directly — "a producer MUST NOT infer exhaustion from a short page — only an absent `nextCursor` means the end" — and the clients' own docstrings promised the opposite of what they did: *enumerate the pool*, *every persona bound in this context*. Nothing downstream could notice. A short array is indistinguishable from a complete one, so past the agent's page size (100 by default) the identity map drew a face pointing at attributes that were not in its own list, `attributeReach` under-reported which contexts a value reaches, and every count on the screen agreed with every other because they were all counting the same truncated array. A picture that reads as complete while being partial is the one wrong answer that pane must never give. - `collectPages` (`core/src/util/pages.ts`) follows the cursor and is where the reasoning lives. `util` is the bottom of the layering, which is why it can be shared by `admin/persona.ts` and `persona/bindings.ts` without a sideways import. - All three clients page through it. `limit` is documented as the page size to ask for, never a cap on the result, and the rest of the request is preserved across pages — a second page of a `typePrefix` query that forgot the prefix would answer a question about one attribute by reading every value the holder has. - `listBindings` returns a document with no `nextCursor`, because there is nothing left to fetch. The two console surfaces that ignored that member are now correct by construction rather than by luck. - The bound **throws**. Returning what had been collected would reintroduce this defect one layer down with a longer array, and a caller cannot tell a truncated answer from a complete one. A repeated cursor is reported as its own fault, separately from the page bound, so the message says which happened. Also finishes #191's vocabulary work: `FactValue` → `AttributeValue` and `maskedFact` → `maskedValue`, the last two symbols in the persona surface carrying the word that PR took off the screen. English uses of "fact" — a fact about the clock, a fact about how the agent is deployed — are left alone; they are the word doing its own job. 12 new tests (573 core, 359 extension), `tsc -b` clean, `npm run build` clean. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
🛡️ AI Agentic Security Code Review🔎 A manual security review is recommended before merging. Please contact the Security team for specifics and remediation guidance.
|
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.
The loose end from the persona-pane run, plus the last of #191's vocabulary.
A listing was read to the first page
personaAttributeList,personaProfileListandlistBindingsasked for a cursor-paginated listing and returned page one, droppingnextCursor. The specification names that inference directly:and each client's own docstring promised the opposite of what it did: enumerate the pool, every persona bound in this context.
Nothing downstream could notice, which is what makes it worth fixing while it is still latent. A short array is indistinguishable from a complete one, so past the agent's page size (100 by default) the identity map drew a face pointing at attributes that were not in its own list — the chip falls back to printing a raw id where the type should be —
attributeReachunder-reported which contexts a value reaches, and every count on screen agreed with every other because they were all counting the same truncated array. A picture that reads as complete while being partial is the one wrong answer that pane must never give.How
collectPages(core/src/util/pages.ts) follows the cursor, and is where the reasoning lives.utilis the bottom of the layering, soadmin/persona.tsandpersona/bindings.tscan share it without a sideways import.limitis documented as the page size to ask for, never a cap on the result.typePrefixquery that forgot the prefix would answer a question about one attribute by reading every value the holder has —reveal-value.tsmatches by id, so it would not even look wrong.listBindingsreturns a document with nonextCursor, because there is nothing left to fetch. The two console surfaces that ignored that member are correct by construction now rather than by luck.MAX_PAGESis 50 — 25,000 records at the specification's maximum page size, high enough that reaching it means a fault rather than a large pool.The last two
factsymbolsFactValue→AttributeValue,maskedFact→maskedValue: the last symbols in the persona surface carrying the word #191 took off the screen. English uses of "fact" — a fact about the clock, a fact about how the agent is deployed — are left alone; there the word is doing its own job.The other two tidy-ups from that list are already gone: #196's rewrite of
claim-sensitivity.tstook the stale "there is no prefix walk" comment and the unused type-onlyisSensitivewith it.Checks
tsc -bclean;npm run buildclean,manager.jsa single chunk.util.pages.mjs: pages are concatenated in order, a short page is not the end, an empty page carrying a cursor is followed, a repeated cursor is named as a loop, and an endless listing throws instead of returning a short answer. Pinned inadmin.persona.mjs: each of the three clients reads to the end, the cursor goes back on the next request, and a narrowed listing stays narrowed.