Skip to content

fix(schema): create per-workspace token_usage table (#712) - #784

Merged
frankbria merged 2 commits into
mainfrom
fix/p0.1-token-usage-table
Jul 3, 2026
Merged

fix(schema): create per-workspace token_usage table (#712)#784
frankbria merged 2 commits into
mainfrom
fix/p0.1-token-usage-table

Conversation

@frankbria

Copy link
Copy Markdown
Owner

What & why

token_usage was never created by any production code path — the only CREATE TABLE token_usage lived in test fixtures. So every save_token_usage() raised OperationalError: no such table (swallowed at debug in react_agent._persist_token_usage), silently dropping all cost/token data and crashing cf stats tokens / zeroing the /costs page. This contradicts shipped Phase 5.2 cost tracking.

Fixes #712 [P0.1] — first launch-blocker from the SaaS launch-readiness audit.

Changes

  • core/workspace.py: new _create_token_usage_schema(cursor) (columns match the repository INSERT; task_id/agent_id/project_id TEXT for v2 UUID task IDs) with indexes on timestamp, task_id, agent_id. Wired into:
    • _init_database → fresh workspaces get the table.
    • _ensure_schema_upgrades → existing workspaces are upgraded idempotently.
  • core/react_agent.py: _persist_token_usage now logs at WARNING (not debug) on failure, so a future persistence break is visible.
  • tests/ui/test_costs_v2.py: the _ensure_token_usage_table fixture delegates to the real schema builder instead of hand-rolling DDL (can't drift).

Tests (TDD, RED→GREEN)

New tests/core/test_token_usage_schema.py:

  • fresh workspace has the table with all INSERT columns
  • indexes on timestamp/task_id/agent_id exist
  • save → read roundtrip through Database (the exact react_agent path) with a UUID task_id
  • _ensure_schema_upgrades adds the table to a pre-existing DB that lacked it

42 passed across test_workspace.py, test_token_usage_schema.py, test_costs_v2.py. ruff clean.

Demo (acceptance criteria)

Fresh cf inittoken_usage present → cf stats tokens renders a zero-state table (exit 0) instead of crashing on no such table.

Acceptance criteria

  • token_usage created by create_or_load_workspace() with expected columns + indexes on timestamp/task_id/agent_id
  • a fresh workspace can save_token_usage() and read it back with no error
  • _persist_token_usage logs at WARNING on failure
  • test fixtures invoke the real schema builder

Scope / not in this PR

The react_agent persist path still routes through the control-plane Database.initialize(), which pollutes the workspace DB with auth tables — that's the separate #713 [P0.2] (this PR does not touch it). Fixing the missing table here is sufficient to make writes succeed and reads/cf stats work.

No production path created token_usage — only test fixtures did — so every
save_token_usage() raised 'no such table' (swallowed at debug in react_agent),
dropping all cost/token data and making cf stats tokens crash.

- add _create_token_usage_schema() and wire it into _init_database (fresh
  workspaces) and _ensure_schema_upgrades (existing DBs), with indexes on
  timestamp/task_id/agent_id
- react_agent._persist_token_usage: log at WARNING, not debug, on failure
- test_costs_v2 fixture delegates to the real schema builder (no hand-rolled DDL)

Closes #712
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 32 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3514abfe-e5ad-4b5e-9777-192f0a483e98

📥 Commits

Reviewing files that changed from the base of the PR and between ecb43f2 and f2855f2.

📒 Files selected for processing (4)
  • codeframe/core/react_agent.py
  • codeframe/core/workspace.py
  • tests/core/test_token_usage_schema.py
  • tests/ui/test_costs_v2.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/p0.1-token-usage-table

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code Review — fix(schema): create per-workspace token_usage table (#712)

This is a clean, well-scoped P0.1 fix. The root cause analysis is accurate, the implementation is minimal, and the TDD approach (new test_token_usage_schema.py) locks in the regression correctly.


What works well

  • Single source of truth for DDL: Extracting _create_token_usage_schema(cursor) and calling it from both _init_database and _ensure_schema_upgrades is the right design. Schema drift between the two paths was the original failure mode, so this eliminates it structurally.
  • Test fixture correction: The old _ensure_token_usage_table in test_costs_v2.py had multiple divergences from production (task_id INTEGER, NOT NULL constraints that don't match the INSERT, a spurious session_id column). Delegating to the real builder is a meaningful correctness improvement.
  • Warning elevation: Promoting _persist_token_usage failure from debug to warning is the right call — a persistence failure that silently drops cost data should be visible in logs.
  • Schema columns match the INSERT exactly: Cross-checked _create_token_usage_schema against the actual INSERT INTO token_usage in platform_store/repositories/token_repository.py — columns align correctly.

Issues / nits

1. NOT NULL constraints intentionally absent — worth a comment

The old fixture had NOT NULL on agent_id, project_id, model_name, input_tokens, output_tokens, and estimated_cost_usd. The new schema drops all of them. Since actual_cost_usd in the INSERT can be None, relaxing constraints is probably correct — but a short inline comment would make the decision explicit and prevent a future reviewer from adding them back.

2. project_id=0 (int) in the roundtrip test, but the column is TEXT

In test_save_and_read_roundtrip, project_id=0 is an int but the schema column is TEXT. SQLite accepts this via type affinity, but the test is misleading — production passes UUID strings. Using project_id="ws-001" would better document the TEXT intent and match what react_agent actually sends.

3. session_id column dropped — confirm no active reader

The old fixture had session_id TEXT DEFAULT NULL. The new schema correctly omits it (the INSERT does not write it). A quick grep to confirm no router SELECT references it by name would be a good pre-merge check.


Test coverage

The four tests in test_token_usage_schema.py cover the key scenarios well: fresh workspace schema presence and column set, index presence on timestamp/task_id/agent_id, save/read roundtrip through the exact Database path react_agent uses, and upgrade for pre-existing DBs that lacked the table.

The roundtrip test calling db.initialize() does create platform-store tables alongside workspace tables (the known #713 issue), but that reflects the current production behavior and does not affect this correctness test.


Summary

Approve with minor nits. The core fix is correct and the test coverage is solid. The most actionable item is changing project_id=0 to a string in the roundtrip test to better reflect the TEXT column and UUID production path. Everything else is either a confirm-and-comment or pre-existing tech debt not introduced by this PR.

@frankbria

Copy link
Copy Markdown
Owner Author

Thanks for the review. Addressed the nits:

@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Follow-up review

All three nits from my prior review are resolved. Here's the close-out:

#1 (nullability comment) — confirmed addressed.
The _create_token_usage_schema docstring now reads: "Columns are intentionally nullable: the INSERT omits some (e.g. actual_cost_usd) — do not add NOT NULL constraints back." — exactly what was needed.

#2 (project_id=0 as int) — confirmed correct.
Checked codeframe/core/models.py:687 and react_agent.py:376 directly:

  • TokenUsage.project_id is typed int (not Optional[int], not str).
  • Production path passes project_id=0 as a bare int.
  • The test using project_id=0 is the correct mirror. No change needed.

#3 (session_id dropped) — confirmed safe.
Checked platform_store/repositories/token_repository.py: both INSERT INTO token_usage branches explicitly enumerate columns and session_id is absent from both. The field exists on the TokenUsage model but is intentionally not persisted. Omitting it from the schema is correct.

One minor docstring observation (non-blocking):
The _create_token_usage_schema docstring says "task_id/agent_id/project_id are TEXT because v2 task IDs are UUID strings" — the grouping slightly conflates three different reasons (task_id: UUID strings; agent_id: names; project_id: int stored in a flexible TEXT column). Not worth a revision now, but worth untangling if the schema docstring gets touched again.

Verdict: ready to merge. Core fix is correct, schema/INSERT alignment verified, test coverage is solid, and all prior nits are closed.

@frankbria
frankbria merged commit 35f4744 into main Jul 3, 2026
11 checks passed
@frankbria
frankbria deleted the fix/p0.1-token-usage-table branch July 3, 2026 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant