fix(tools): add timestamp/chain filters and newest-first ordering to ar_query_receipts#119
Conversation
…ar_query_receipts Fixes #118. Two bugs fixed: 1. `timestamp_after` (and `timestamp_before`, `chain_id`) were silently dropped because they weren't in the TypeBox schema or passed to the SDK query. All three are now wired through. 2. Default ordering returned the oldest receipts first (SDK uses ASC). Results are now sorted newest-first in JS after fetching (no SDK limit), then sliced to `limit`. Sequence number is the tiebreaker for receipts within the same millisecond. The tool description documents the polling pattern: pass `timestamp_after` set to the last-seen receipt's timestamp to fetch only new receipts since then.
7f51729 to
dfdb16b
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the ar_query_receipts agent tool so it can filter by chain and timestamp bounds, and so it returns newest receipts first instead of the SDK’s default oldest-first ordering. It fits into the plugin’s audit-trail surface by making receipt queries more useful for session inspection and polling workflows.
Changes:
- Added
chain_id,timestamp_after, andtimestamp_beforeparameters toar_query_receipts, with light timestamp validation. - Changed receipt query behavior to sort newest-first in tool code before applying
limit. - Expanded unit tests for filtering/ordering and updated integration expectations to match descending results.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/tools.ts |
Extends ar_query_receipts parameters and changes result ordering/limit behavior. |
src/tools.test.ts |
Adds focused tests for new filters, ordering, and invalid timestamp handling. |
src/integration.test.ts |
Updates end-to-end assertions to match newest-first receipt ordering. |
| chainId: params.chain_id, | ||
| after, | ||
| before, |
| const limit = params.limit ?? 20; | ||
| const results = all | ||
| .sort((a, b) => { | ||
| const ta = a.credentialSubject.action.timestamp; | ||
| const tb = b.credentialSubject.action.timestamp; | ||
| if (tb < ta) return -1; | ||
| if (tb > ta) return 1; | ||
| // Tiebreak by sequence descending so calls within the same millisecond | ||
| // are still returned newest-first within their chain. | ||
| return b.credentialSubject.chain.sequence - a.credentialSubject.chain.sequence; | ||
| }) | ||
| .slice(0, limit); |
| chain_id: Type.Optional( | ||
| Type.String({ description: "Restrict results to a single receipt chain." }), |
| // Fetch all matching receipts without a limit so we can sort newest-first | ||
| // in JS before slicing. The SDK only supports ASC ordering today. | ||
| const all = deps.store.query({ | ||
| actionType: params.action_type, | ||
| riskLevel, | ||
| status, | ||
| limit: params.limit ?? 20, | ||
| chainId: params.chain_id, | ||
| after, | ||
| before, | ||
| }); | ||
|
|
||
| const limit = params.limit ?? 20; | ||
| const results = all | ||
| .sort((a, b) => { | ||
| const ta = a.credentialSubject.action.timestamp; | ||
| const tb = b.credentialSubject.action.timestamp; | ||
| if (tb < ta) return -1; | ||
| if (tb > ta) return 1; | ||
| // Tiebreak by sequence descending so calls within the same millisecond | ||
| // are still returned newest-first within their chain. | ||
| return b.credentialSubject.chain.sequence - a.credentialSubject.chain.sequence; | ||
| }) | ||
| .slice(0, limit); |
| // Tiebreak by sequence descending so calls within the same millisecond | ||
| // are still returned newest-first within their chain. | ||
| return b.credentialSubject.chain.sequence - a.credentialSubject.chain.sequence; |
| "To poll for new actions since your last check, pass `timestamp_after` set to the timestamp of " + | ||
| "the most recent receipt you've already seen.", |
- Make timestamp_after exclusive (>): SDK is >=, narrow in JS post-filter before sort; update parameter description accordingly - Add chain_id to result shape so callers can identify which chain each receipt belongs to - Clamp limit to non-negative integers; fractional/negative values fall back to default 20 (consistent with how invalid risk_level/status are silently ignored) - Test same-millisecond sequence tiebreaker (seq DESC wins) - Default chain_id to current session's chain when omitted; add all_chains boolean opt-out to query across chains - Update integration.test.ts queries that span sessions to pass all_chains: true; update tools.test.ts to construct tools via factory with matching session context
| const limit = | ||
| typeof params.limit === "number" && Number.isInteger(params.limit) && params.limit >= 0 |
There was a problem hiding this comment.
Misread of the diff: this commit (5dfb9e7) removed those casts. Current code at this line is the typeof-narrow form you're suggesting (typeof params.limit === "number" && Number.isInteger(params.limit) && params.limit >= 0 ? params.limit : 20) — no assertions left. Marking not applicable.
| // drop any receipt whose timestamp exactly equals params.timestamp_after. | ||
| const filtered = after | ||
| ? all.filter((r) => r.credentialSubject.action.timestamp !== after) |
| const results = filtered | ||
| .sort((a, b) => { | ||
| const ta = a.credentialSubject.action.timestamp; | ||
| const tb = b.credentialSubject.action.timestamp; | ||
| if (tb < ta) return -1; | ||
| if (tb > ta) return 1; | ||
| // Tiebreak by sequence descending so calls within the same millisecond | ||
| // are still returned newest-first within their chain. | ||
| return b.credentialSubject.chain.sequence - a.credentialSubject.chain.sequence; | ||
| }) | ||
| .slice(0, limit); | ||
|
|
|
|
||
| // Fetch all matching receipts without a limit so we can sort newest-first | ||
| // in JS before slicing. The SDK only supports ASC ordering today. | ||
| const all = deps.store.query({ | ||
| actionType: params.action_type, | ||
| riskLevel, | ||
| status, | ||
| limit: params.limit ?? 20, | ||
| chainId, | ||
| after, |
Summary
Fixes #118. Two bugs in
ar_query_receipts:timestamp_aftersilently dropped: the parameter wasn't in the TypeBox schema and wasn't passed to the SDK query.timestamp_beforeandchain_idhad the same gap. All three are now wired through.ReceiptStore.queryreturns resultsORDER BY timestamp ASC, so with aLIMIT 20the tool returned the 20 oldest receipts, not the 20 newest. Results are now sorted newest-first in JS after fetching, then sliced tolimit. Sequence number is the tiebreaker for receipts within the same millisecond.What changed
src/tools.ts: three new parameters (chain_id,timestamp_after,timestamp_before) added to the schema andexecutesignature; sort+slice replaces the SDK-side limit; tool description updated with polling guidance; light ISO 8601 validation (consistent with how invalidrisk_level/statusare already handled).timestamp_afteris now exclusive (>): the SDK'safterfilter is>=, so polling with the last-seen timestamp would re-return that receipt. The JS post-filter drops any receipt whose timestamp exactly equalstimestamp_after. Parameter description updated to say "exclusive".chain_idadded to result shape: each receipt inresultsnow includeschain_idso callers can identify which chain it belongs to and use it in follow-up queries.limitclamped to non-negative integers: fractional values (e.g.5.7) and negative values (e.g.-1) fall back to the default of 20, consistent with how invalidrisk_level/statusare already silently ignored.chain_idto current session's chain: whenchain_idis omitted, results are scoped to the current session's chain (resolved via factory context). New booleanall_chains: trueopts out and queries across all chains.src/tools.test.ts: new and updated tests — 8 existing tests migrated to factory-with-context pattern; 10 new tests covering exclusivetimestamp_after,chain_idin result shape, limit clamping (negative and fractional), same-millisecond sequence tiebreaker, session-chain defaulting,all_chains: true, and explicitchain_idoverride.src/integration.test.ts: cross-session queries updated to passall_chains: true(the tool is resolved at registration time with a mock context that doesn't match the session under test).Out of scope (intentional follow-ups)
DEFAULT_QUERY_LIMITplus ASC ordering means stores with more than 10,000 matching receipts will silently lose the newest ones, even with this PR's JS-side sort. The proper fix is at the SDK level (DESC ordering and/or removing the silent cap) and applies equally to all language SDKs (TS, Go, Python). Tracked in ReceiptStore.query: support DESC ordering and remove silent default-limit cap ar#300.ar_query_receiptsis request/response; a truetail -fpattern would require a different mechanism. Out of scope per this issue.chain_idfilter enables per-chain queries but doesn't enforce or detect cross-chain splits. ADR-10 tracks that separately.Test plan
pnpm test— 171 tests, 7 files, all passpnpm typecheck— cleantimestamp_afteris exclusive: polling with last-seen timestamp no longer returns duplicatelimit: 5returns the 5 newest, not the 5 oldestchain_idin result shapeall_chains: truereturns all