Repository navigation
Add Analytics Chats and preserve usage history - #1016
GraciousGazelles wants to merge 19 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces account-scoped Analytics Chats to the TUI /usage menu, allowing users to view chat usage and estimates. It adds an AnalyticsSession client for authentication and plan discovery, implements backend queries for task and thread usage, integrates these into the TUI with pagination and local lineage details, and updates the relevant documentation and tests. The review feedback focuses on improving efficiency and backward compatibility, suggesting the replacement of is_none_or with map_or for older Rust toolchains, checking collection limits early to abort iterations, and optimizing checks to avoid unnecessary string allocations and cloning.
| if current.is_none_or(|auth| { | ||
| auth.get_account_id() != self.auth.get_account_id() | ||
| || auth.get_chatgpt_user_id() != self.auth.get_chatgpt_user_id() | ||
| }) { |
There was a problem hiding this comment.
Using is_none_or requires Rust 1.82.0 or newer. To ensure better backward compatibility with older Rust toolchains (MSRV), consider using map_or(true, ...) instead.
| if current.is_none_or(|auth| { | |
| auth.get_account_id() != self.auth.get_account_id() | |
| || auth.get_chatgpt_user_id() != self.auth.get_chatgpt_user_id() | |
| }) { | |
| if current.map_or(true, |auth| { | |
| auth.get_account_id() != self.auth.get_account_id() | |
| || auth.get_chatgpt_user_id() != self.auth.get_chatgpt_user_id() | |
| }) { |
| let mut ids = HashSet::new(); | ||
| if threads.is_empty() | ||
| || threads.len() > 100 | ||
| || threads.iter().any(|thread| { | ||
| std::iter::once(&thread.thread_id) | ||
| .chain(&thread.descendant_thread_ids) | ||
| .any(|id| id.trim().is_empty() || id.len() > 512 || !ids.insert(id.as_str())) | ||
| }) | ||
| || ids.len() > 1_000 | ||
| { |
There was a problem hiding this comment.
Evaluating ids.len() > 1_000 only after the entire any iteration finishes can lead to excessive memory allocation and CPU usage if an input contains a very large number of descendant thread IDs. Checking the limit inside the closure allows the iteration to abort early.
let mut ids = HashSet::new();
if threads.is_empty()
|| threads.len() > 100
|| threads.iter().any(|thread| {
std::iter::once(&thread.thread_id)
.chain(&thread.descendant_thread_ids)
.any(|id| {
id.trim().is_empty()
|| id.len() > 512
|| !ids.insert(id.as_str())
|| ids.len() > 1_000
})
})
{| let requested: std::collections::HashSet<_> = thread_ids.iter().copied().collect(); | ||
| if thread_ids.is_empty() || thread_ids.len() > 100 || requested.len() != thread_ids.len() { | ||
| return Err(RequestError::from(anyhow!( | ||
| "expected 1–100 distinct thread IDs" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
Checking the bounds of thread_ids after collecting them into a HashSet is inefficient. Performing the length and emptiness checks first avoids unnecessary memory allocation when the input is invalid.
| let requested: std::collections::HashSet<_> = thread_ids.iter().copied().collect(); | |
| if thread_ids.is_empty() || thread_ids.len() > 100 || requested.len() != thread_ids.len() { | |
| return Err(RequestError::from(anyhow!( | |
| "expected 1–100 distinct thread IDs" | |
| ))); | |
| } | |
| if thread_ids.is_empty() || thread_ids.len() > 100 { | |
| return Err(RequestError::from(anyhow!( | |
| "expected 1–100 distinct thread IDs" | |
| ))); | |
| } | |
| let requested: std::collections::HashSet<_> = thread_ids.iter().copied().collect(); | |
| if requested.len() != thread_ids.len() { | |
| return Err(RequestError::from(anyhow!( | |
| "expected 1–100 distinct thread IDs" | |
| ))); | |
| } |
| let mut seen = HashSet::new(); | ||
| let mut batches = Vec::new(); | ||
| let mut batch = Vec::new(); | ||
| let mut count = 0; | ||
| for query in queries { | ||
| let ids = std::iter::once(&query.thread_id).chain(&query.descendant_thread_ids); | ||
| if ids.clone().any(|id| !seen.insert(id.clone())) { | ||
| return Err("Task descendant groups overlap.".to_string()); | ||
| } | ||
| let size = 1 + query.descendant_thread_ids.len(); | ||
| if count + size > 1_000 || batch.len() == 100 { | ||
| batches.push(std::mem::take(&mut batch)); | ||
| count = 0; | ||
| } | ||
| count += size; | ||
| batch.push(query); | ||
| } | ||
| if !batch.is_empty() { | ||
| batches.push(batch); | ||
| } |
There was a problem hiding this comment.
Cloning all thread IDs during the overlap check is inefficient. By splitting this into a two-pass approach, we can perform a read-only overlap check using HashSet<&str> to completely avoid string allocations, and then perform the batching in a separate pass.
let mut seen = HashSet::new();
for query in &queries {
let ids = std::iter::once(query.thread_id.as_str())
.chain(query.descendant_thread_ids.iter().map(|id| id.as_str()));
if ids.clone().any(|id| !seen.insert(id)) {
return Err("Task descendant groups overlap.".to_string());
}
}
let mut batches = Vec::new();
let mut batch = Vec::new();
let mut count = 0;
for query in queries {
let size = 1 + query.descendant_thread_ids.len();
if count + size > 1_000 || batch.len() == 100 {
batches.push(std::mem::take(&mut batch));
count = 0;
}
count += size;
batch.push(query);
}
if !batch.is_empty() {
batches.push(batch);
}| let partial_usage = rows.iter().any(|row| { | ||
| row.usage | ||
| .as_ref() | ||
| .is_none_or(|usage| usage.data_status != TaskUsageStatus::Available) | ||
| }); |
There was a problem hiding this comment.
Using is_none_or requires Rust 1.82.0 or newer. To ensure better backward compatibility with older Rust toolchains (MSRV), consider using map_or(true, ...) instead.
| let partial_usage = rows.iter().any(|row| { | |
| row.usage | |
| .as_ref() | |
| .is_none_or(|usage| usage.data_status != TaskUsageStatus::Available) | |
| }); | |
| let partial_usage = rows.iter().any(|row| { | |
| row.usage | |
| .as_ref() | |
| .map_or(true, |usage| usage.data_status != TaskUsageStatus::Available) | |
| }); |
GraciousGazelles
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| let legacy_usage_path = codex_home.join(format!("{unversioned_usage_name}{suffix}")); | ||
| assert_eq!( | ||
| tokio::fs::try_exists(&legacy_usage_path) | ||
| tokio::fs::read(&legacy_usage_path) |
| let old_usage_path = codex_home.join(format!("usage_0.sqlite{suffix}")); | ||
| assert_eq!( | ||
| tokio::fs::try_exists(&old_usage_path) | ||
| tokio::fs::read(&old_usage_path) |
The current
/usagemenu lacks the operator's Analytics Chats view. Add a bounded, account-scoped Chats view for supported workspace and consumer plans, with page-local ranking, explicit partial-history and unavailable-data handling, and labelled backend credit estimates. Read the existing local usage ledger for per-thread root, subagent and fork detail; do not add a writer or PostgreSQL ingestion. Preserve old usage database files and SQLite sidecars at startup; leave current-database opening and migrations unchanged.The 30-day indexed, page-local scope and reduced Analytics controls were accepted in w14939#112827. This draft is a source component for the existing w14939 owner, not a replacement integration programme or a qualified binary: reliable single-writer recording (PR #1015), restoration of the installed durable mailbox, the existing native-wait work (PR #1000), historical-source reconciliation, final composition and owner acceptance remain open. Existing PR #995 and rollback paths remain intact.
Validation so far:
d5218e4fe5d10145a802647c30fa075cd974c6b4preserves older usage database files and sidecars and labels local usage as potentially incomplete as well as overcounted. Predecessor9eed19ce1b80709837bb55da02cf2575ffe664b4passed hosted full state and TUI library runs (42032502585 and 42032502606), including preservation, lineage and the seven requested Chats/usage selectors; exact-head hosted full state and TUI library runs passed (42033428873 and 42033428877); Linux full Clippy, Rust CI gate and Linux/macOS Bazel Clippy passed; cargo-deny required gate failed and other checks remain in progress. Combined-source qualification remains open.a293d38acb70087261831fcf73850d0e72f09072: hosted full TUI library command passed, including all seven requested Chats and usage selectors (run 42021009106); usage-picker tests 35/35 and maintained full argument-comment lint also passed. Protected Clippy failed on the now-corrected clone and cargo-deny failed; the exact hosted cargo-deny log was unavailable. Local cargo-deny 0.19.0 reported the same five Rust advisories on protected main and predecessor.a8427f2d2e3c8262553a0cadd007a5dbe7948442: nine Chats seam tests, provider-auth library checks, MCP safety and docs lanes passed; the successor adds reviewed snapshots only.8c31447111d9e81669177af5b49fa0565bc86ea6: mailbox orchestration, blocking/capacity waits, MCP device login, weekly pacing and state-migration lanes passed.