-
Notifications
You must be signed in to change notification settings - Fork 710
fix(usage): disclose the window a truncated read actually covers #1532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,29 @@ function refreshedUsageSummary<T extends UsageSummary & { historyTruncated: bool | |
| return { ...summary, since, generatedAt: now }; | ||
| } | ||
|
|
||
| /** | ||
| * Timestamp bounds of the rows the bounded reader actually loaded. | ||
| * | ||
| * Deliberately computed over the whole snapshot, BEFORE `summarizeUsage` applies the range | ||
| * and surface predicates: truncation is a property of the read, not of the query, so the | ||
| * window that matters to a client is the one the reader could see. It is not a completeness | ||
| * claim and must never be presented as one. `usage.jsonl` is appended when a request | ||
| * COMPLETES while each row carries the request START time, so a long-running request can be | ||
| * appended after shorter ones that started later — meaning the oldest loaded timestamp does | ||
| * not bound what the dropped prefix contains (#1497). | ||
| */ | ||
| function snapshotWindow(entries: PersistedUsageEntry[]): { start: number | null; end: number | null } { | ||
| let start: number | null = null; | ||
| let end: number | null = null; | ||
| for (const entry of entries) { | ||
| const at = entry.timestamp; | ||
| if (typeof at !== "number" || !Number.isFinite(at)) continue; | ||
| if (start === null || at < start) start = at; | ||
| if (end === null || at > end) end = at; | ||
| } | ||
| return { start, end }; | ||
| } | ||
|
|
||
| export async function handleLogsUsageRoutes(ctx: ManagementContext): Promise<Response | null> { | ||
| const { req, url, config, deps, syncClaudeAgentDefsBestEffort } = ctx; | ||
|
|
||
|
|
@@ -209,12 +232,15 @@ export async function handleLogsUsageRoutes(ctx: ManagementContext): Promise<Res | |
| const overlayVersion = userCostOverlayVersion(); | ||
| const snapshot = await readUsageSnapshotForManagement(effectiveReadLimit); | ||
| const revisionReadAt = Date.now(); | ||
| const window = snapshotWindow(snapshot.entries); | ||
| const summary = { | ||
| ...summarizeUsage(snapshot.entries, range, now, surface), | ||
| historyTruncated: snapshot.truncatedPrefixBytes > 0 || snapshot.entriesTruncated, | ||
| truncatedPrefixBytes: snapshot.truncatedPrefixBytes, | ||
| entriesTruncated: snapshot.entriesTruncated, | ||
| entriesDropped: snapshot.entriesDropped, | ||
| snapshotWindowStart: window.start, | ||
| snapshotWindowEnd: window.end, | ||
|
Comment on lines
+242
to
+243
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These fields add a user-visible management API contract whose pre-filter semantics and nullable bounds are not documented in AGENTS.md reference: AGENTS.md:L234-L235 Useful? React with 👍 / 👎. |
||
| }; | ||
| if (userCostOverlayVersion() !== overlayVersion) { | ||
| // The overlay changed while the summary was being computed, so this | ||
|
|
@@ -266,6 +292,8 @@ export async function handleLogsUsageRoutes(ctx: ManagementContext): Promise<Res | |
| truncatedPrefixBytes: 0, | ||
| entriesTruncated: false, | ||
| entriesDropped: 0, | ||
| snapshotWindowStart: null, | ||
| snapshotWindowEnd: null, | ||
| error: "read_failed", | ||
| }); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Restore the original global-property shape.
Lines 164-165 save only values. Lines 231-233 then define every global key, even when that key did not exist before this test. This leaks own
window,document,navigator,localStorage, orIS_REACT_ACT_ENVIRONMENTproperties into later tests.Save each original property descriptor. Restore the descriptor when it exists. Delete the property when it did not exist.
Proposed fix
Also applies to: 231-233
🤖 Prompt for AI Agents