Repository navigation
Conversation
…tement
token_ledger was created in 060 with no index. getBalance summed one column and
then the other (two full scans), and getHistory filtered on one of two columns
and sorted by created_at (scan + temp B-tree). transferTokens calls getBalance
inside its own write transaction, so the scan was write-lock hold time on an
append-only table.
Two indexes, one per direction. getBalance becomes a single row with two scalar
subqueries: a single SUM over cannot use either index, but
each correlated subquery can. getHistory is untouched - EXPLAIN QUERY PLAN shows
the existing OR already plans as MULTI-INDEX OR over both new indexes, and a
UNION ALL rewrite measured slower (MERGE plus two temp B-trees).
Measured with sqlite 3.46.1 via EXPLAIN QUERY PLAN:
before: SCAN token_ledger / SCAN token_ledger
after: SEARCH token_ledger USING INDEX idx_token_ledger_to_created
SEARCH token_ledger USING INDEX idx_token_ledger_from_created
tests/unit/migration-213-token-ledger-indexes.test.ts pins the plan and the
behaviour (net balance, newest-first history, overdraft rejected then accepted
at the exact balance).
…and mirrors Migration 213 lands in this branch, so the four English surfaces that state the count (README.md, AGENTS.md, llm.txt) and the 69 translated README rows move from 209 to 210. check-docs-counts-sync.mjs fails without this; it passes after. The 66 docs/i18n/<locale>/llm.txt files are exact-copy mirrors of root llm.txt (check-docs-sync.mjs compares the body byte-for-byte), so they carry the same four substitution sites. Rewritten from the root body, not by a blind regex: each mirror is 4 lines added, 4 removed, and no removed line lacks "209".
…ecord Eleven findings across security, performance, orchestration and structure, every one citing path:line plus a verbatim snippet. Numbers are re-measured in-session and the commands live in section 9, which separates the read-only audit against the pristine tip from the remediation run on the tree that carries migration 213. Section 6 item 2 records what the fix actually is: indexes only. The UNION ALL rewrite that the finding originally implied measured slower (MERGE plus two temp B-trees per branch) because the existing OR already plans as MULTI-INDEX OR over both new indexes. What does need to change is getBalance, whose single SUM over cannot reach either index. Known-red gates are recorded rather than papered over: the open-sse typecheck gate fails with eleven errors that reproduce on a clean HEAD worktree with none of this branch's files, so they are inherited tip drift.
…og-integrity failure
…tement
token_ledger was created in 060 with no index. getBalance summed one column and
then the other (two full scans), and getHistory filtered on one of two columns
and sorted by created_at (scan + temp B-tree). transferTokens calls getBalance
inside its own write transaction, so the scan was write-lock hold time on an
append-only table.
Two indexes, one per direction. getBalance becomes a single row with two scalar
subqueries: a single SUM over cannot use either index, but
each correlated subquery can. getHistory is untouched - EXPLAIN QUERY PLAN shows
the existing OR already plans as MULTI-INDEX OR over both new indexes, and a
UNION ALL rewrite measured slower (MERGE plus two temp B-trees).
Measured with sqlite 3.46.1 via EXPLAIN QUERY PLAN:
before: SCAN token_ledger / SCAN token_ledger
after: SEARCH token_ledger USING INDEX idx_token_ledger_to_created
SEARCH token_ledger USING INDEX idx_token_ledger_from_created
tests/unit/migration-213-token-ledger-indexes.test.ts pins the plan and the
behaviour (net balance, newest-first history, overdraft rejected then accepted
at the exact balance).
…and mirrors Migration 213 lands in this branch, so the four English surfaces that state the count (README.md, AGENTS.md, llm.txt) and the 69 translated README rows move from 209 to 210. check-docs-counts-sync.mjs fails without this; it passes after. The 66 docs/i18n/<locale>/llm.txt files are exact-copy mirrors of root llm.txt (check-docs-sync.mjs compares the body byte-for-byte), so they carry the same four substitution sites. Rewritten from the root body, not by a blind regex: each mirror is 4 lines added, 4 removed, and no removed line lacks "209".
…ecord Eleven findings across security, performance, orchestration and structure, every one citing path:line plus a verbatim snippet. Numbers are re-measured in-session and the commands live in section 9, which separates the read-only audit against the pristine tip from the remediation run on the tree that carries migration 213. Section 6 item 2 records what the fix actually is: indexes only. The UNION ALL rewrite that the finding originally implied measured slower (MERGE plus two temp B-trees per branch) because the existing OR already plans as MULTI-INDEX OR over both new indexes. What does need to change is getBalance, whose single SUM over cannot reach either index. Known-red gates are recorded rather than papered over: the open-sse typecheck gate fails with eleven errors that reproduce on a clean HEAD worktree with none of this branch's files, so they are inherited tip drift.
…og-integrity failure
…memory The analytics endpoint ran five separate GROUP BY passes over the same UNION of usage_history and daily_usage_summary — getDailyCostRows, getModelUsageRows, getProviderCostRows, getProviderUsageRows and getServiceTierUsageRows — so one dashboard load paid five scans plus five temp B-tree sorts for one dataset. Replace them with a single getModelUsageMatrixRows read grouped on (date, provider, model, serviceTier) and derive*Rows roll-ups. The roll-up is exact: every measure is additive, and the only non-additive one, the latency average, is carried as latencyWeightedMs/latencySamples so a roll-up recombines it as SUM(latency*requests)/SUM(requests). localDb re-exports are updated and llm.txt documents the single-read contract (mirrors resynced).
A daily_usage_summary row with total_cost = 0 is priced from its token totals, but computeUsageRowCost early-returned the stored cost whenever the row was aggregated. The zero was returned before the token estimate could be applied, so a period whose summaries were written before costs were priced showed $0 in every cost panel even though the tokens were billable. Split the stored cost into its two legs instead of inferring one from MAX(is_aggregated). getModelUsageMatrixRows now returns aggregatedStoredCost and rawStoredCost, both summed in SQL and both additive through rollupMatrixRows, so a roll-up no longer collapses "some rows are aggregates" into "all of this group's cost is stored". SUMMARY_COST_COLUMNS zeroes cost_tokens_input/cost_tokens_output for a summary that already carries a cost, which is what keeps the new additive formula from pricing a priced row twice. computeUsageRowCost returns aggregatedStoredCost + rawStoredCost + the token estimate; unpriceable models still get their stored legs back rather than 0. tests/unit/db-usage-analytics-3500.test.ts seeds real provider_cost_usd values so the raw leg is not vacuously zero, and covers both the split in getModelUsageMatrixRows and its survival through deriveModelUsageRows. changelog.d/fixes/17320-usage-analytics-legacy-summary-cost.md
oyi77
force-pushed
the
work/audit-20261022c
branch
from
October 22, 2026 22:02
1c05119 to
8008f54
Compare
The DIRJEN-BOT clone had five commits of this branch written on the older b68c332 base: the PERF-01 ledger index, the migration-count bump, the three audit documents. The branch was since rebased onto the live upstream tip 2319a84, so those five are present again as rebased commits with matching patch ids and the branch tip is a superset of their content. This merge keeps the DIRJEN-BOT history attached instead of discarding it. The only conflict was the DB-architecture bullet in llm.txt and its 66 locale mirrors: the pre-rebase side lacked the single-read sentence, so every hunk resolved to the rebased wording and the resulting tree is byte-identical to 8008f54. check-docs-sync reports 66/66 mirrors matching.
…o it §9.2 states what the fuse actually bought: five panel passes over one identical GROUP BY collapse into a single getModelUsageMatrixRows read, with the exact roll-up contract (latencyWeightedMs/latencySamples, MAX(is_aggregated), the split cost legs) and the five scans that legitimately remain. The changelog bullet previously claimed the endpoint "reads the unified cost source once" and that "a new panel now costs no extra SQL pass" - both false, since the summary, daily, heatmap, weekly and preset passes survive for reasons the matrix buckets cannot answer. Measured on 2f2b809: 104/104 tests, tsc EXIT=0 unpiped, and the docs-sync, docs-counts, changelog-integrity, migration-numbering and forgotten-sibling gates all EXIT=0.
Three falsifiable claims in the PERF-02 record, fixed against the route source: - the equality coverage omitted deriveModelUsageRows, which is pinned by three tests at 3500.test.ts:335, :373 and :824. - "five scans remain" undercounted. The route keeps nine reads on a plain load, plus one getPresetCostModelRows per requested preset range. - weekly (:691) was described as non-preset-only. It is unconditional; the preset loop (:1107) is additive when ?presets= is set, so both run together. The eight-vs-nine split, the per-panel line numbers, the call_logs exclusion for getFallbackStats/getErrorTypeBreakdown and the preset gate are all read from the route at f0150df. Docs-sync, docs-counts and changelog-integrity exit 0.
- "Measured on 2f2b809" was the merge SHA before upstream 5b3be70; the 104/104 battery and the unpiped tsc run were produced on f0150df. - "The legacy five are gone ... consumed once" overreached: getWeeklyPatternRows (:691) and getPresetCostModelRows (:1132) still query the same source on their own groupings and windows, as §9.2's own residual inventory already said.
- Re-measured battery on `1c550e7dcf`: 133 tests pass across 11 files (previously stated 104 because db-usageanalytics-split.test.ts was not found by its underscore variant). - Per-file breakdown: db-usage-analytics-3500=28, split=32, migration-213=5, usage-analytics=13, route=31, model-dedup=1, pricing-memoization=4, provider-display=2, response-cache=9, daily-usage=3, history-db=5. - Replaced malformed PUT markers left by edit tool with proper Markdown table rows.
…t plan from audit New file: docs/audit/IMPROVEMENT-PLAN.md with: - 4-week execution timeline broken into weekly sprints - Week 1 (high-impact): PERF-03 compression queue cap, SEC-01 auth hardening, LKGP cache retirement decision - Week 2 (dashboard perf): PERF-02 expression indexes for 14 aggregates, QUAL-04 silent catches replacement, QUAL-05 asRecord consolidation - Week 3-4 (UX polish): FUNC-01/02/03/04 documentation fixes, PERF-06 pool ordering hash comparison, PERF-07 cache key canonicalization, PERF-08 image CLS fix, PERF-09 provider metadata build-time JSON - v4.0 backlog summary: god functions split, ESLint suppression burn-down, boundary splits, dead code reduction - Success metrics table: memory under load, dashboard latency, log visibility, coverage targets - Rollout plan with day-by-day breakdown and risk matrix
… API handlers (QUAL-04)
…he hits (PERF-07)
…ger 291-module REGISTRY imports (PERF-09)
…avings claim (FUNC-01)
…asurements (FUNC-02/03/04)
…d register gen:provider-metadata script (PERF-09 follow-up)
…recordHelpers module (QUAL-05 phase 1)
…h coverage metrics
…change Per review, this PR keeps only the title's scope: the token_ledger balance indexes migration, the single-statement getBalance read and its test. Every other theme (provider-nodes/jobs auth, usageAnalytics single read, migrations 214/215, generated provider metadata, recordHelpers, empty-catch cleanup, readCache, migrationRunner cache, docs/audit and the README/i18n edits) is restored to its merge-base state here and stays available in this branch's history to be opened as separate PRs. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…on to 224 Migration 213 is taken on the release tip (213_api_key_preferred_connections), so the token_ledger balance indexes move to the next free slot, 224, with the test renamed to match. The migration count is synced to 221 in README.md, AGENTS.md, llm.txt, the 66 llm.txt mirrors and the README database row of the 66 translations (digits only), as check:docs-counts and check-docs-sync require. Adds the changelog fragment. Conflicts in antigravityFamilyCooldown.ts, cachingAware.ts and provider-stats/route.ts were on lines where this branch only carried hook formatting after the scope reduction, so the release content is kept there. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
docs/i18n/*/README.md conflicts resolved by taking the release translation; only the database-row migration count bumped 220 -> 221 (the value check:docs-counts expects with migration 224).
Collaborator
|
Thanks @oyi77. The This branch also bundles auth changes, a ~14k-line generated JSON and an audit document under a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidated remediation batch from the 2026-10-22 repository audit (see
docs/audit/AUDIT-2026-10-22.md), rebased onto the currentrelease/v3.8.53tip (b68c33211e).Three commits:
perf(db)- adds213_token_ledger_balance_indexes.sql(two partial composite indexes ontoken_ledgercovering thefrom_api_key_id/to_api_key_idbalance reads), and collapsesgetBalancefrom two separate statements into a singleUNION ALLstatement sotransferTokensstill sees a consistent read insideinstance.transaction().docs- migration count 209 -> 210 acrossREADME.md,AGENTS.md,llm.txtand all 66 locale mirrors (README + llm.txt).docs(audit)- the consolidated audit document plus the PERF-01 remediation record.The prior branch
work/audit-20261022b(PR #17908) wentCONFLICTINGbecause upstream rewrote the same block in 25 locale READMEs while bumping their own counts. This branch is cut fresh from the current tip with the identical three commits re-applied, and the 25 locale conflicts resolved by keeping upstream's wording with the count at 210.Verification (run on this branch at
b8d382c97e)npx tsc --pretty false -p tsconfig.typecheck-core.jsonnode scripts/check/check-docs-counts-sync.mjsAll checks passnode scripts/check/check-docs-sync.mjsPASS, 66 localesnode --import tsx/esm ... --test tests/unit/migration-213-token-ledger-indexes.test.ts tests/unit/db/gamification*.test.tsnode --import tsx/esm ... --test tests/unit/db/**/*.test.tsCovered behaviour in
tests/unit/migration-213-token-ledger-indexes.test.ts:getBalancenets credits against debits,getHistoryorders newest first,transferTokensstill rejects an overdraft through the collapsed read.