Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0351828726
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| gatewayManager: Pick<GatewayManager, 'rpc'>, | ||
| limit?: number, | ||
| ): Promise<TokenUsageHistoryEntry[]> { | ||
| const maxEntries = limit ?? 500; |
There was a problem hiding this comment.
When the Models page calls recentTokenHistory() without a limit, this defaults to only 500 entries, while the previous file-backed implementation treated an omitted limit as unbounded. Users with more than 500 usage-bearing messages therefore get incomplete 7-day, 30-day, and all-time totals; the additional 500-session and 500-message RPC caps compound the truncation. Keep the default unbounded or paginate the Gateway data before applying an explicit caller limit.
AGENTS.md reference: AGENTS.md:L41-L41
Useful? React with 👍 / 👎.
| const history = await dependencies.gatewayManager.rpc('chat.history', { | ||
| sessionKey: selected.sessionKey, | ||
| limit: 1_000, | ||
| }) as { messages?: unknown }; |
There was a problem hiding this comment.
For conversations longer than 1,000 messages, chat.history returns only the bounded tail, but the ZIP manifest still records the conversation as fully included. This regresses the previous path, which read the entire transcript, and can omit the earlier context needed to diagnose a long-running failure. Paginate the storage-neutral history or use a full-export Gateway contract before creating the JSONL.
Useful? React with 👍 / 👎.
| checksums, | ||
| }, null, 2)}\n`, { encoding: 'utf8', mode: SNAPSHOT_FILE_MODE }); | ||
|
|
||
| await rm(snapshotDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
If a backed-up file or snapshot.json fails verification after an interrupted migration has already changed the active config or databases, the next launch rebuilds from that partially migrated active state and then deletes the original checkpoint here. Even when only the marker is malformed and the recovery files remain usable, this destroys the sole pre-migration rollback material. Fail closed or quarantine the damaged checkpoint instead of replacing it in place.
Useful? React with 👍 / 👎.
su8su
changed the title
1 participant
No description provided.