feat(comptroller): bridge ChittyComptroller cost into the books#133
feat(comptroller): bridge ChittyComptroller cost into the books#133chitcommit wants to merge 1 commit into
Conversation
ChittyComptroller (comptroller.chitty.cc) is the source of truth for
AI/infra cost. This bridge has ChittyFinance CONSUME its HTTP cost API and
mirror the daily per-service total into transactions as an expense — it does
NOT re-ingest gateway logs ("fed back, no duplication").
- storage.upsertComptrollerCost: idempotent by external_id
`comptroller:{YYYY-MM-DD}:{service}`, enforced by a partial unique index
on (tenant_id, external_id). amount is decimal(12,2) cents; the exact
cost_usd + tokens + per-tier breakdown are preserved in metadata so no
precision is lost. onConflictDoUpdate uses targetWhere to match the partial
index arbiter.
- lib/comptroller-sync: fetch metrics, aggregate per-(service,tier) rows to a
per-service total (multi-tier services would otherwise overwrite each other),
upsert one expense/service. COA 6010 "Software Subscriptions".
- POST /api/comptroller/sync (tenant via X-Tenant-ID middleware).
- Daily cron (worker.ts scheduled) resolves the infra tenant from the seeded
"ChittyOS Infrastructure" account (external_id 'chittyos-infra').
Validated against the live API + real Neon (solitary-rice-14149088): 3 rows
recorded (chittycounsel 0.04, chittyclaw 0.03, chittygateway 0.00) matching
the Comptroller today totals; re-ran twice, no duplicates. npm run check clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
📝 WalkthroughWalkthroughThis PR implements a complete Comptroller cost sync bridge that fetches external AI/infra costs from an API, aggregates them by service, and idempotently upserts expense transactions into the ledger. It adds database schema constraints for idempotency, configuration, type contracts, API integration, storage persistence, a tenant-scoped HTTP endpoint, and scheduled cron execution. ChangesComptroller Cost Sync Bridge
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler as Route Handler
participant ComptrollerAPI as Comptroller API
participant Database
Client->>RouteHandler: POST /api/comptroller/sync
RouteHandler->>Database: Lookup infra account by external_id
Database-->>RouteHandler: infraAccount
RouteHandler->>ComptrollerAPI: GET /api/v1/metrics
ComptrollerAPI-->>RouteHandler: ComptrollerMetrics (today)
RouteHandler->>RouteHandler: Aggregate metrics by service
loop For each service
RouteHandler->>Database: upsertComptrollerCost (INSERT ... ON CONFLICT)
Database-->>RouteHandler: SyncedRow
end
RouteHandler->>Database: ledgerLog audit entry
RouteHandler-->>Client: { synced, date, rows }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 852075a272
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| set: { | ||
| amount, | ||
| coaCode: input.coaCode, | ||
| classifiedBy: 'comptroller', | ||
| classifiedAt: now, | ||
| metadata, | ||
| updatedAt: now, |
There was a problem hiding this comment.
When a Comptroller transaction for the same tenant/day/service has already been reconciled, any later sync for that day hits this conflict path and updates amount, coaCode, metadata, and updatedAt anyway. That bypasses the reconciled-row lock enforced elsewhere in classifyTransaction and can change already-reconciled books whenever the daily total changes or the sync is rerun; the conflict update should exclude reconciled rows or require an L3/L4 unlock path.
Useful? React with 👍 / 👎.
| app.route('/', allocationRoutes); | ||
| app.route('/', classificationRoutes); | ||
| app.route('/', emailRoutes); | ||
| app.route('/', comptrollerRoutes); |
There was a problem hiding this comment.
This mounts /api/comptroller/sync with the authenticated routes, but I checked the protectedPrefixes list above and /api/comptroller is not included. As a result requests to this new endpoint do not run storageMiddleware or tenantMiddleware; c.get('storage')/c.get('tenantId') are unset, so the route fails before it can perform the documented tenant-scoped manual sync.
Useful? React with 👍 / 👎.
| // ChittyComptroller cost bridge — record today's per-service AI/infra cost | ||
| ctx.waitUntil( | ||
| runComptrollerSync(env).catch((err) => { | ||
| console.error('[cron:comptroller] sync failed:', err instanceof Error ? err.message : String(err)); | ||
| }), |
There was a problem hiding this comment.
The production cron I checked in wrangler.jsonc/deploy/system-wrangler.jsonc runs this scheduled handler once at 09:00 UTC, but this call records Comptroller's today data using the current date. For services that incur cost after 09:00 UTC, the transaction for that day is booked with only a partial daily total and will not be corrected automatically until a manual rerun; the cron should sync a completed previous-day window or run after the reporting day closes.
Useful? React with 👍 / 👎.
| accountId: account.id, | ||
| metrics, | ||
| }); | ||
| console.log(`[cron:comptroller] synced ${rows.length} services:`, JSON.stringify(rows.map((r) => `${r.service}=${r.amount}`))); |
There was a problem hiding this comment.
For the scheduled path that creates or updates the daily Comptroller transactions, the only audit emitted after the write is this console log, while the manual route records a comptroller.sync ledger entry. In production cron runs, those automated financial-state mutations therefore have no immutable ledger/audit event to tie the inserted expense rows back to the sync, so add the same ledger logging (or equivalent) to the scheduled path.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/app.ts (1)
101-106:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
/api/comptrolleris missing fromprotectedPrefixes— route will fail at runtime.The
comptrollerRouteshandler readsc.get('storage')andc.get('tenantId')from context, but/api/comptrolleris not inprotectedPrefixes. Without the middleware chain, both values will beundefined, causing the route to throw when callingstorage.getAccountByExternalId().Proposed fix
const protectedPrefixes = [ '/api/accounts', '/api/transactions', '/api/properties', '/api/integrations', '/api/tasks', '/api/ai-messages', '/api/ai', '/api/summary', '/api/mercury', '/api/github', '/api/charges', '/api/forensics', '/api/portfolio', '/api/import', '/api/reports', - '/api/google', '/api/comms', '/api/workflows', '/api/leases', '/api/coa', '/api/classification', '/mcp', + '/api/google', '/api/comms', '/api/workflows', '/api/leases', '/api/coa', '/api/classification', '/api/comptroller', '/mcp', ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/app.ts` around lines 101 - 106, protectedPrefixes is missing '/api/comptroller', causing comptrollerRoutes to run without middleware that sets context values; update the protectedPrefixes array to include '/api/comptroller' so the middleware chain that populates c.get('storage') and c.get('tenantId') runs before comptrollerRoutes and prevents storage.getAccountByExternalId() from receiving undefined values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/lib/comptroller-sync.ts`:
- Around line 48-54: fetchComptrollerMetrics performs the fetch to
`${base}/api/v1/metrics` with no timeout; add an AbortController and pass its
signal to fetch, start a timer (configurable, e.g., 5s) that calls
controller.abort() on timeout, and clear the timer after fetch resolves; ensure
the function still throws on non-ok responses and returns the parsed JSON as
ComptrollerMetrics.
In `@server/storage/system.ts`:
- Around line 188-205: The UPDATE block that sets
coaCode/classifiedBy/classifiedAt on schema.transactions currently skips writing
a corresponding row to classification_audit, which violates the COA audit
requirement; modify the same transaction that performs the .onConflictDoUpdate
(the code updating schema.transactions) to also INSERT into classification_audit
a new audit row containing tenantId, transaction id or externalId, coaCode,
classifiedBy, classifiedAt, metadata and createdAt (and agent/trust fields per
AGENTS.md L0-L4) so every classification write creates an audit entry; ensure
the insert runs in the same DB transaction as the update and handle
conflict/duplicates appropriately (e.g., always insert a new audit row rather
than updating existing audit rows).
---
Outside diff comments:
In `@server/app.ts`:
- Around line 101-106: protectedPrefixes is missing '/api/comptroller', causing
comptrollerRoutes to run without middleware that sets context values; update the
protectedPrefixes array to include '/api/comptroller' so the middleware chain
that populates c.get('storage') and c.get('tenantId') runs before
comptrollerRoutes and prevents storage.getAccountByExternalId() from receiving
undefined values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 784fba43-1c6b-49df-bf5e-a8ccff35a2ef
📒 Files selected for processing (7)
database/system.schema.tsserver/app.tsserver/env.tsserver/lib/comptroller-sync.tsserver/routes/comptroller.tsserver/storage/system.tsserver/worker.ts
| export async function fetchComptrollerMetrics(base: string): Promise<ComptrollerMetrics> { | ||
| const res = await fetch(`${base}/api/v1/metrics`, { headers: { accept: 'application/json' } }); | ||
| if (!res.ok) { | ||
| throw new Error(`Comptroller returned ${res.status}`); | ||
| } | ||
| return (await res.json()) as ComptrollerMetrics; | ||
| } |
There was a problem hiding this comment.
Add an explicit timeout for the Comptroller fetch call.
Line 49 performs an external call without a timeout. If Comptroller hangs, this can stall request handling and cron execution.
Suggested fix
export async function fetchComptrollerMetrics(base: string): Promise<ComptrollerMetrics> {
- const res = await fetch(`${base}/api/v1/metrics`, { headers: { accept: 'application/json' } });
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), 10_000);
+ const res = await fetch(`${base}/api/v1/metrics`, {
+ headers: { accept: 'application/json' },
+ signal: controller.signal,
+ }).finally(() => clearTimeout(timeout));
if (!res.ok) {
throw new Error(`Comptroller returned ${res.status}`);
}
return (await res.json()) as ComptrollerMetrics;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/comptroller-sync.ts` around lines 48 - 54, fetchComptrollerMetrics
performs the fetch to `${base}/api/v1/metrics` with no timeout; add an
AbortController and pass its signal to fetch, start a timer (configurable, e.g.,
5s) that calls controller.abort() on timeout, and clear the timer after fetch
resolves; ensure the function still throws on non-ok responses and returns the
parsed JSON as ComptrollerMetrics.
| coaCode: input.coaCode, | ||
| classifiedBy: 'comptroller', | ||
| classifiedAt: now, | ||
| metadata, | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: [schema.transactions.tenantId, schema.transactions.externalId], | ||
| // The arbiter index is partial (WHERE external_id IS NOT NULL); Postgres | ||
| // requires the predicate in the conflict target to infer a partial index. | ||
| targetWhere: sql`${schema.transactions.externalId} IS NOT NULL`, | ||
| set: { | ||
| amount, | ||
| coaCode: input.coaCode, | ||
| classifiedBy: 'comptroller', | ||
| classifiedAt: now, | ||
| metadata, | ||
| updatedAt: now, | ||
| }, |
There was a problem hiding this comment.
COA classification fields are being written without a corresponding audit row.
Lines 188-203 update coaCode/classifiedBy/classifiedAt directly, but this method does not insert into classification_audit. That bypasses the trust-path audit requirement and makes classification history incomplete for these synced transactions.
As per coding guidelines, "COA classification writes must write audit rows. Trust levels L0-L4 enforced per AGENTS.md."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/storage/system.ts` around lines 188 - 205, The UPDATE block that sets
coaCode/classifiedBy/classifiedAt on schema.transactions currently skips writing
a corresponding row to classification_audit, which violates the COA audit
requirement; modify the same transaction that performs the .onConflictDoUpdate
(the code updating schema.transactions) to also INSERT into classification_audit
a new audit row containing tenantId, transaction id or externalId, coaCode,
classifiedBy, classifiedAt, metadata and createdAt (and agent/trust fields per
AGENTS.md L0-L4) so every classification write creates an audit entry; ensure
the insert runs in the same DB transaction as the update and handle
conflict/duplicates appropriately (e.g., always insert a new audit row rather
than updating existing audit rows).
Source: Coding guidelines
|
placeholder |
Code Review — feat(comptroller): bridge ChittyComptroller cost into the booksOverall this is a clean, well-scoped integration. The idempotency approach (partial unique index + Critical — Route is unprotected
Fix: add Bug — Decimal precision comment is wrongThe comment in Minor — Float accumulation for financial totalsIn Minor — Stale cron comment in wrangler.jsoncThe wrangler cron comment still says Minor — Redundant
|
Canonical "ChittyScrape feeds the cost flow" wiring. ChittyScrape (scrape.chitty.cc) extracts vendor charges from portals with no API (registered-agent fees, utilities, mortgage). This adds a real ChittyFinance ingest that turns a ChittyScrape result envelope + resolved charge into an idempotent expense transaction, reusing the PR #133 comptroller pattern. - storage.upsertVendorCharge: idempotent by external_id `scrape:{portalId}:{period}:{vendor}`, type='expense', enforced by the partial unique index transactions_tenant_external_idx (external_id NOT NULL). amount is REQUIRED (> 0) and decimal(12,2); exact USD + paymentStatus + category preserved in metadata. onConflictDoUpdate targetWhere matches the partial arbiter. - POST /api/vendor-charge/ingest: validates the envelope + charge (zod), rejects success=false (422) and a missing/zero amount (400, never defaults to 0), maps vendor category -> real COA code, books against the account by external_id (default chittyos-infra), audit-logs via ledgerLog. - COA mapping (verified against seeded COA on Neon solitary-rice-14149088, no invented codes): registered-agent -> 5050 Legal & Professional Fees; utilities -> 5100/5110/5120/5130/5140; mortgage -> 5300 Mortgage Interest. nw-registered-agent mapping: scraper annualFeeUsd -> charge.amountUsd, paymentStatus ('ok'|'failed') -> charge.paymentStatus, portal 'nw-registered-agent' -> category 'registered-agent' -> COA 5050, period = filing year. Verified end-to-end on real Neon (solitary-rice-14149088): upserted a real Northwest Registered Agent $125/yr charge via the exact storage SQL, selected it back (COA 5050, type expense), re-ran the upsert — same row id, 1 row, no duplicate (idempotency proven). npm run check clean. NOTE: the live portal scrape is credential-gated (NW login via ChittyConnect); the registration + ingest wiring + mapping are real and verified, but the live scrape -> ingest hop is the chico/ChittyConnect follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Bridge that feeds ChittyComptroller cost data INTO ChittyFinance books — fed back, no duplication. ChittyComptroller stays source of truth; ChittyFinance consumes its /api/v1/metrics and records a daily per-service infra expense (COA 6010 Software Subscriptions), idempotent by external_id comptroller:{date}:{service} via a partial unique index on (tenant_id, external_id). amount=cents (decimal 12,2), exact cost_usd + tokens + per-tier breakdown in metadata. Per-(service,tier) rows aggregated to per-service totals. POST /api/comptroller/sync (tenant via X-Tenant-ID) + daily cron in worker.ts scheduled (infra tenant resolved from seeded chittyos-infra account). Validated on live API + real Neon solitary-rice-14149088: chittycounsel 0.04 (0.040361), chittyclaw 0.03 (0.034849), chittygateway 0.00 (0.000252) — match Comptroller; re-ran 3x, no duplicates. npm run check clean. Seed (applied, idempotent): tenant IT CAN BE LLC, account ChittyOS Infrastructure, partial unique index. chico follow-up: npm run deploy needs DATABASE_URL; reuses existing 0 9 * * * cron.
Co-Authored-By: Claude Opus 4.8 (1M context)
Summary by CodeRabbit