Repository navigation
feat(audit-summary): CLI summarizer over voice/call/task/cost ledgers (§5 Next) - #750
Conversation
sonichi
left a comment
There was a problem hiding this comment.
LGTM — clean separation (summarize / renderMarkdown / CLI entry), 6 tests including the cost-accounting flake fix (real ms-collision bug, nice catch), and graceful missing-source handling. The realpathSync on import.meta.url + process.argv[1] for the isMain check is a nice touch for /tmp → /private/tmp macOS test runs.
Two non-blocking observations for follow-up:
-
Source-regex stops at first hyphen.
match(/^(task|result|…)(?:-([a-z]+))?-?\d*/i)captures only the first lowercase segment after the prefix, so a future filename liketask-discord-bridge-…txtrolls up asdiscord, andtask-react-native-…txtwould roll up asreact. Probably fine right now — every current source channel is one word — but worth a comment near line 116 so a future contributor adding a multi-word source doesn't silently lose the suffix in by_source totals. -
Voice errors capped at 10 with no overflow signal.
voiceErrors.slice(0, 10)truncates without telling the owner the total. For a real incident-triage rollup over--since 7dthat could mask "is this 11 errors or 11,000?". Small fix: adderrors_total: voiceErrors.lengthnext toerrors:in the schema so the markdown renderer can show "Errors: 10 of 47 shown" when truncated. Not a blocker — current behavior is bounded output, which is the right default.
Ship 🚀
…ale TODO Three fixups per sonichi's #749 review: 1. mkdirSync(target, recursive=true) is cheap but non-zero; record() fires per tool-call + per session-end, so an _ensuredDirs Set saves N-1 syscalls per ledger over the process lifetime. Pure perf, no correctness risk. 2. Add 'ms' to by_model. Previously by_model only carried in_tokens, out_tokens, requests — when audit-summary (#750) eventually adds a per-model average-latency view, it needs ms broken out by model. Free roll-up: emit shape unchanged; rollup() picks up ms when the event has both .model and unit='ms'. New test verifies. 3. TODO(scale) comment in rollup() docstring: 'reads whole file into memory' — fine near-term, but flagged to switch to createReadStream + readline when long-running tenants hit ~50MB+ ledgers. Also pulled in the time-range test's setTimeout(2) fix from #750 so #749's CI stays deterministic in isolation (was flaky due to ms- precision adjacent record() calls colliding on the same timestamp). Tests: 9/9 pass (was 8 — added 'by_model now breaks out ms' test). tsc clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Heads-up — Should be a clean rebase: No action needed from me — flagging so you see it before reviewing. Round-1 LGTM still stands once rebased. |
7503451 to
a5fe768
Compare
liususan091219
left a comment
There was a problem hiding this comment.
Reviewed: new audit-summary CLI rolls up voice-metrics + call-metrics + cost-accounting + task/result archives into markdown (or --json). Composable: audit-summary | gh issue create --body-file - is one-line plumbing. CLI-first per existing diagnostic surface convention. — Lucy (Mac Studio bot)
liususan091219
left a comment
There was a problem hiding this comment.
LGTM — first §5 Next "audit UI" deliverable, with the right shape for an MVP: CLI that composes (audit-summary | gh issue create --body-file -), markdown by default + --json for machine consumers, and aggregation across all the existing JSONL sources without forcing a schema migration. Picks up #749's cost ledger as a fifth source so the cost surface lights up the moment that lands.
Things I like:
-
CLI-first instead of web UI (top docstring) — explicit reasoning: matches existing diagnostic tooling convention (self-diagnose, regression-search), output composes with shell pipes, no server to deploy. This is the "ship the smallest useful thing" call and the comment names it so future-me doesn't re-litigate the decision.
-
Multi-source aggregation with skip-on-missing (
audit-summary.ts:154-203) — voice-metrics.jsonl, call-metrics.jsonl, cost-accounting/usage-.jsonl, tasks/archive, results/archive. Each source is independent and the CLI degrades cleanly when one is absent. A fresh install with only voice data still produces a useful summary instead of an empty file. -
findFirst(...paths)for legacy-location handling (audit-summary.ts:96-99+ usages 161, 184) —call-metrics.jsonlcan live underdata/,workspace/data/, ORtasks/archive/data/depending on when the install was provisioned. The fallback list captures the fleet-history of where the file moved without bloating the call site. -
Date filter via ISO string comparison (
audit-summary.ts:165, 188) —m.timestamp >= fromIso && m.timestamp <= toIso. AvoidsDate.parse()on every record (which would be 10K+ parses for a busy week's voice-metrics). Works because ISO 8601 strings sort lexicographically the same as dates. -
parseDuration("7d"|"24h"|"30m"|<ISO>)(audit-summary.ts:135-145) — natural-language relative ranges + ISO escape hatch. The shape is intuitive (matchesgh pr list --search's relative-date syntax). The regex is tight enough that--since 7dayserrors loudly instead of silently parsing as ISO 0. -
renderMarkdowntopNhelper (audit-summary.ts:240-244) —topN(o, 5)formats{name: count}asname (count), ...sorted desc. Reused for voice tools, call tools, task sources, cost-by-tool, cost-by-device, cost-by-model. Six call sites, one definition — exactly the right level of abstraction. -
Cost rollup only renders when
event_count > 0(audit-summary.ts:215-216) — empty cost section is suppressed cleanly. So pre-#749-deploy summaries don't have an awkward "0 events" block; post-#749 it appears organically. -
isMaincheck usesrealpathSyncon both sides (audit-summary.ts:298-303) — handles macOS's/tmp→/private/tmpsymlink sotsx /tmp/foo/audit-summary.tscorrectly identifies as direct invocation. The comment names why this is non-trivial, which is the kind of "save the next debugger" note that pays off. -
AuditSummaryexported as a type (audit-summary.ts:60-89) — downstream consumers (future web UI, programmatic callers) get a typed contract. The JSON output shape and the type stay in sync because the JSON IS the type. -
Realistic test coverage (
audit-summary.test.ts:81-148) — empty workspace, voice-metrics aggregation with date filter, call-metrics owner/meeting flags, archive counts with mtime filter, cost ledger emission viarecord()then assert in rollup, markdown structure assertions. Each test exercises one source independently, so a regression in one source-reader doesn't poison the others.
Non-blocking observations:
-
readJsonlreads the full file into memory (audit-summary.ts:101-109) — fine for today's ~MB-scale files, but voice-metrics.jsonl on a heavy user could grow to 10s of MB. A streaming readline would scale linearly without holding everything; for now, a TODO comment naming the threshold (~50MB) would let future-me know when to refactor. -
voiceErrors.slice(0, 10)caps but doesn't summarize (audit-summary.ts:220) — the first 10 error events show verbatim; the 11th+ are silently dropped. Atotal: N, showing first 10line would prevent under-reporting in a worst-case bad week. -
listArchiverecurses but doesn't bound depth (audit-summary.ts:112-138) —<YYYY-MM>/<file>.txtis the documented layout; if someone nests further (debug archives, manual reorgs), the recursion is unbounded. Cheap fix: cap at 2 levels (the documented depth). -
Filename regex for source classification is best-effort (
audit-summary.ts:127-128) — handlestask-,result-,proactive-,question-,briefing-,insight-,friction-prefixes but anything else collapses tounknown. If new prefixes ship (e.g.health-,digest-), the by_source breakdown silently undercounts. Worth a centralized prefix list shared with the writer side. -
Cost rollup uses
tenantId(ws)(audit-summary.ts:206) — current process's tenant. If someone wants to audit another tenant's ledger from the same shell (SUTANDO_TENANT_ID=acme tsx audit-summary.ts --tenant acme), the env var is set but the cached tenant_id from a prior tenant might be stale. The__resetTenantForTestshelper exists; production calls might want aresetTenantId()public method. -
--tenantarg doesn't filter voice/call metrics — only cost rollup. Soaudit-summary --tenant acmewould mix tenant-acme's costs with the current-process's voice/call metrics. Once voice-metrics + call-metrics gain atenant_idfield (likely follow-up from #751 / #752), the filter should extend. Tagged in the type union as an obvious future-extension. -
No
--sincevalidation —parseDurationthrows on bad input but the error stack doesn't include the CLI-usage hint. A try/catch around the parse withconsole.error(usage); process.exit(2)would be a friendlier UX. -
Tests use
utimesSyncfor archive mtime (audit-summary.test.ts:107-108) — clean trick but worth a comment ("simulate aged file") for the next test author.
Cross-PR linkages:
- Direct consumer of #749 cost-accounting —
costRollup({tenant_id, from, to})is the new dependency. Per my #749 review observation about rollup scaling at ~50MB, this CLI is the first place that pain shows up. - Direct consumer of #754 tenant.py twin (sibling) and #748 tenant scaffold —
tenantId(ws)is the tenant-resolver entry point. - Pairs with #751 #752 cost emitters — voice-agent + phone-conversation emit cost events via
record(); this CLI reads them viarollup(). End-to-end test path: emit → file → rollup → render. - §5 Next "Audit UI" — this is the milestone. Web UI is the named follow-up.
- Composes with
gh issue create(docstring example) — useful for end-of-week roll-ups or "what did the agent do during the merge freeze?" introspection.
Tests: 6 tests, all green. Empty-workspace / voice-aggregation / call-aggregation / archive-counts / cost-emit / markdown-structure. The cost-emit test exercises the full source→record→rollup→render chain in one assertion path, which is the highest-value integration test in the file.
Ready to merge after #749 lands (the only hard dep). #751 + #752 + #754 are soft deps — without them, the summary works but has fewer sources lit up.
— Lucy (Mac Studio bot)
…ale TODO Three fixups per sonichi's #749 review: 1. mkdirSync(target, recursive=true) is cheap but non-zero; record() fires per tool-call + per session-end, so an _ensuredDirs Set saves N-1 syscalls per ledger over the process lifetime. Pure perf, no correctness risk. 2. Add 'ms' to by_model. Previously by_model only carried in_tokens, out_tokens, requests — when audit-summary (#750) eventually adds a per-model average-latency view, it needs ms broken out by model. Free roll-up: emit shape unchanged; rollup() picks up ms when the event has both .model and unit='ms'. New test verifies. 3. TODO(scale) comment in rollup() docstring: 'reads whole file into memory' — fine near-term, but flagged to switch to createReadStream + readline when long-running tenants hit ~50MB+ ledgers. Also pulled in the time-range test's setTimeout(2) fix from #750 so #749's CI stays deterministic in isolation (was flaky due to ms- precision adjacent record() calls colliding on the same timestamp). Tests: 9/9 pass (was 8 — added 'by_model now breaks out ms' test). tsc clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
90fcf16 to
8a36cab
Compare
…dger Second consumer of PR #749's per-tenant cost ledger. Mirrors voice-agent's emit shape from PR #751: 1 request + total duration in ms + 1 tool_call per session.toolCalls entry. Token counts deferred until bodhi exposes in/out totals — same shape lights them up when ready. device_id='phone-call:<callSid>' distinguishes phone-originated usage from laptop/glasses in audit-summary (#750)'s by_device breakdown. Non-owner calls emit with source='phone-conversation:non-owner' so spam-attribution is preserved without leaking the caller number into device_id keys. Dynamic import + try/catch keeps cost-accounting an optional dep — if the module is absent (older worktree, partial deploy), phone session finalization silently skips emit rather than crash the call-end path. Mirrors #751's defensive shape. Test plan: - [x] npx tsc --noEmit clean. - [ ] Manual: complete a phone call → verify state/cost-accounting/usage-default.jsonl gains api_call(request=1) + api_call(ms=N) + per-tool entries with device_id='phone-call:<callSid>'. - [ ] Manual: audit-summary --since 24h shows the Cost ledger populated with phone-call.<callSid> in by_device. Depends on PR #749 (cost-accounting). Targets feat/cost-accounting- scaffold as base. Independent of PR #751 — both emit into the same ledger but live at separate call sites. Roadmap impact: - §5 Next 'Cost accounting' second consumer landed. Remaining emit sites: skill execution (gws-gmail-voice, image-generation, etc.) and the Mentra bridge (PR #740). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Activates PR #749's per-tenant cost-accounting ledger from voice-agent's session-end path. Without this, the ledger stays empty and the cost section of audit-summary (PR #750) reports 'No events recorded yet (cost-accounting emitters not wired)'. What ships: - writeVoiceMetrics() now also emits to cost-accounting after the existing voice-metrics.jsonl write. Records: * 1 'request' event tagged with the voice model (gemini-2.5-flash today). * Total session duration in ms. * 1 'tool_call' per voiceToolCalls entry, with the tool name. - Token in/out counts are intentionally NOT emitted yet — the realtime transport doesn't expose them today. When bodhi adds them (or we add a thin counter around sendFile/sendContent), the same emit point lights up the token totals; no schema change needed. - Dynamic import of './cost-accounting.js' so the module is optional — older worktrees / partial deploys silently skip emit rather than crash the session-end path. Device-aware shape: 'laptop' deviceId is hardcoded here because the voice-agent's local session IS the laptop. Per-device emission for Mentra glasses / phone calls / future devices lives in the bridge that owns the relevant DeviceSession (PR #740 for Mentra; phone- conversation's conversation-server.ts is the next consumer). Test plan: - [x] npx tsc --noEmit clean. - [ ] Manual: start voice-agent, complete a session, verify `state/cost-accounting/usage-default.jsonl` exists with api_call(request=1, ms=N) + per-tool entries. - [ ] Manual: `tsx src/audit-summary.ts --since 24h` shows the Cost ledger section populated. Depends on PR #749 (cost-accounting scaffold). Targets feat/cost- accounting-scaffold as base. Roadmap impact: - §5 Next 'Cost accounting' consumer landed for voice. Phone- conversation + skill execution + Mentra bridge are follow-ups (one call site per PR keeps review focused). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (§5 Next) Implements §5 Next "Audit UI built on existing audit.jsonl files. 'What did the agent do, on which device, with which methodology, why?' A trust feature for productization, not just a debugging artifact." First-cut: CLI summarizer that aggregates every audit source already in the repo. Web UI is a future iteration; CLI lands first because (a) it's the canonical owner-introspection interface (matches self-diagnose, regression-search), (b) the output composes into shell pipelines. Sources consumed (any subset; missing files skipped): - data/voice-metrics.jsonl — voice-agent sessions - data/call-metrics.jsonl — phone-conversation metrics (also looks under tasks/archive/data/ and workspace/tasks/data/) - tasks/archive/<YYYY-MM>/ — task lifecycle counts by source - results/archive/<YYYY-MM>/ — result lifecycle counts - state/cost-accounting/usage-<tenant>.jsonl — from PR #749 Output: markdown by default, --json for machine-readable. Usage: tsx src/audit-summary.ts # today tsx src/audit-summary.ts --since 7d # last 7 days tsx src/audit-summary.ts --since 2026-05-01 --until 2026-05-15 tsx src/audit-summary.ts --tenant acme # cost rollup scoped tsx src/audit-summary.ts --json What ships: - src/audit-summary.ts (~230 LOC): summarize(opts) returns an AuditSummary object; renderMarkdown(s) produces the report; CLI entry point parses --since/--until/--tenant/--json. - tests/audit-summary.test.ts — 6 unit tests covering empty workspace, voice-metrics date filtering + aggregation, call-metrics owner/ meeting flags, archive counts by mtime, cost-ledger surfacing, renderMarkdown shape. All pass. - tests/cost-accounting.test.ts: fixed flake in the time-range test (adjacent record() calls collide on ms; setTimeout(2) between them gives a stable cut point). Now deterministic. Real-workspace smoke (last 24h on my install): - 19 voice sessions, 664 min, 4 tool calls - 1 phone call, 0.1 min - 32 archived tasks, 37 archived results - Cost ledger empty (emitters not wired — wiring is the follow-up PR noted in #749's body). What this PR does NOT change: - No web UI. CLI markdown is the first cut. - No structured emit to audit.jsonl. The summarizer reads existing scattered sources; a unified audit.jsonl is a future refactor. - No per-device or per-methodology breakdown beyond what the cost ledger already exposes — owner can drill via --tenant + the by_device and by_tool maps in --json output. Test plan: - [x] npx tsx --test tests/audit-summary.test.ts tests/cost-accounting.test.ts tests/tenant.test.ts — 28/28 pass. - [x] npx tsc --noEmit clean. - [x] Real-workspace smoke: tsx src/audit-summary.ts --since 24h outputs structured markdown. Depends on PR #749 (cost-accounting) for the rollup() consumer. Targets feat/cost-accounting-scaffold as base. Roadmap impact: - §5 Next "Audit UI": CLI first cut shipped. With this + #748 + #749 + the existing voice/call/archive sources, every "what did the agent do" question has a structured answer. Web UI is the next iter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
a5fe768 to
1ea9e68
Compare
Implements §5 Next "Audit UI" — first cut as a CLI summarizer that aggregates every audit source already in the repo. Web UI lands later; CLI first because (a) matches self-diagnose / regression-search pattern, (b) composes into shell pipelines.
Base: feat/cost-accounting-scaffold (#749). Depends on #748 + #749.
Sources consumed (skips missing)
Usage
Real-workspace smoke
Last 24h on my install:
What ships
Test plan
Roadmap impact
🤖 Generated with Claude Code