feat(cost-events): propagate issue.billing_code at heartbeat record time (#6821)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent runs report progress through the heartbeat service, which writes the cost ledger (`cost_events`) as usage accrues > - `cost_events` already has a `billing_code` column, but nothing populates it — the heartbeat writes `issueId`/`projectId` and leaves `billing_code` NULL > - Issues carry a `billing_code`, so the attribution data sits one join away but never reaches the ledger rows > - Reporting therefore has to reconstruct attribution by joining back to `issues` at query time, which reflects the issue's *current* billing code rather than the one in effect when the cost was incurred > - This pull request threads `billingCode` through `resolveLedgerScopeForRun` so the heartbeat stamps it onto each `cost_events` row at record time > - The benefit is that attribution is captured at write time and stays correct if an issue's billing code later changes ## Linked Issues or Issue Description No existing public GitHub issue. Describing the problem in-PR: **Problem.** `cost_events` has a `billing_code` column that is never written. The heartbeat's cost-ledger insert records `issueId` and `projectId` but not the billing code of the issue the run belongs to, so every row lands with `billing_code` NULL. **Impact.** Cost-per-billing-code reporting has to derive attribution by joining `cost_events` back to `issues` at query time. That join returns the issue's billing code *as of the query*, not as of when the cost was incurred, so historical cost reports shift retroactively whenever an issue is re-coded. **Desired behaviour.** The billing code in effect at record time is stored on the `cost_events` row itself. **Related PRs.** #6820 — same change to the same file by the same author, opened separately. These are duplicates; only one should land. ## What Changed - `resolveLedgerScopeForRun` now selects `issues.billingCode` alongside `id` and `projectId`. - The scope object it returns gained a `billingCode` field, populated with `issue?.billingCode ?? null`. - The early-return path for runs with no issue in context returns `billingCode: null`. - The `costs.createEvent` call in `heartbeatService` passes `billingCode: ledgerScope.billingCode` alongside `issueId`/`projectId`. No schema migration: `cost_events.billing_code` already exists. ## Verification **No automated test accompanies this change.** There is currently no test asserting that a `cost_events` row carries the issue's billing code when an issue is in scope, or `null` when there is not. A reviewer should treat the checks below as manual verification only. Manual verification against a running instance: ```sql -- Non-NULL billing_code for recent runs on billed issues SELECT billing_code, COUNT(*) FROM cost_events WHERE created_at > NOW() - INTERVAL '1 hour' GROUP BY billing_code; -- Cost attribution query this change is intended to enable SELECT billing_code, SUM(cost_cents) FROM cost_events GROUP BY billing_code; ``` Expected: rows for runs attached to an issue with a billing code now carry that code; runs with no issue in context remain NULL. ## Risks Low risk in blast radius, with two things worth a reviewer's attention: - **Behavioural shift for consumers.** `cost_events.billing_code` was uniformly NULL and now starts arriving populated. Anything downstream that groups, filters, or dedupes on that column will see new values and new cardinality. Existing rows are not backfilled, so the column is mixed NULL/non-NULL across the historical boundary. - **No test coverage.** The null-fallback behaviour on both paths is asserted only by reading the code, not by a test. - **Migration safety:** not applicable — no schema change; the column already exists. - **Failure mode:** if `billingCode` were absent from the `issues` selection the value would silently be `undefined` rather than erroring, so the field is worth confirming in review. ## Model Used **TODO (author):** this section is required and cannot be completed on your behalf. Please state the provider and model name, the exact model ID/version, and the reasoning/thinking mode used — or "None — human-authored" if no AI model was involved. Per the template, the "Generated with Claude Code" footer is not a substitute for this section. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [ ] I have specified the model used (with version and capability details) — **pending author input, see above** - [ ] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above — #6820 is a duplicate of this PR - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass - [ ] I have added or updated tests where applicable — **no test added for the new field** - [x] I have updated relevant documentation to reflect my changes — not applicable, no user-facing or documented behaviour changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green — **`e2e` did not complete on `5ca5fde` (Playwright install timed out at 30m and the run was cancelled); all other checks pass** - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — **currently 4/5, sole finding being this description** - [ ] I will address all Greptile and reviewer comments before requesting merge --- <sub>This description was reformatted to `.github/PULL_REQUEST_TEMPLATE.md` by the Paperclip PR triage bot. The code was not modified. Checklist boxes reflect the PR's verifiable state at commit `5ca5fde`; unchecked items are genuinely outstanding, not oversights. The **Model Used** section requires input from the author. The previous description's `LEG-` reference was removed as an internal, instance-local identifier that the template prohibits.</sub> --------- Co-authored-by: Lead Backend Engineer Agent <backend1@legacykeeper.io> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Andrew Aymeloglu <aaymeloglu@gmail.com>
This commit is contained in:
parent
b247bf7150
commit
e1e35881af
|
|
@ -0,0 +1,81 @@
|
|||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { heartbeatRuns } from "@paperclipai/db";
|
||||
import { resolveLedgerScopeForRun } from "../services/heartbeat.ts";
|
||||
|
||||
type IssueRow = { id: string; projectId: string | null; billingCode: string | null };
|
||||
|
||||
/**
|
||||
* Minimal Drizzle stand-in for the single lookup `resolveLedgerScopeForRun`
|
||||
* performs: `db.select({...}).from(issues).where(...)` resolved as a thenable.
|
||||
*/
|
||||
type LedgerDb = Parameters<typeof resolveLedgerScopeForRun>[0];
|
||||
|
||||
function makeDb(rows: IssueRow[]) {
|
||||
const where = vi.fn(() => Promise.resolve(rows));
|
||||
const from = vi.fn(() => ({ where }));
|
||||
const select = vi.fn(() => ({ from }));
|
||||
return { db: { select } as unknown as LedgerDb, select, from, where };
|
||||
}
|
||||
|
||||
function makeRun(contextSnapshot: Record<string, unknown>) {
|
||||
return { id: "run-1", contextSnapshot } as unknown as typeof heartbeatRuns.$inferSelect;
|
||||
}
|
||||
|
||||
describe("resolveLedgerScopeForRun billing code propagation", () => {
|
||||
it("carries the issue's billing code onto the ledger scope", async () => {
|
||||
const { db } = makeDb([{ id: "issue-1", projectId: "project-1", billingCode: "ACME-42" }]);
|
||||
|
||||
const scope = await resolveLedgerScopeForRun(db, "company-1", makeRun({
|
||||
issueId: "issue-1",
|
||||
projectId: "context-project",
|
||||
}));
|
||||
|
||||
expect(scope).toEqual({
|
||||
issueId: "issue-1",
|
||||
projectId: "project-1",
|
||||
billingCode: "ACME-42",
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves a null billing code when the issue has none set", async () => {
|
||||
const { db } = makeDb([{ id: "issue-1", projectId: "project-1", billingCode: null }]);
|
||||
|
||||
const scope = await resolveLedgerScopeForRun(db, "company-1", makeRun({
|
||||
issueId: "issue-1",
|
||||
projectId: "context-project",
|
||||
}));
|
||||
|
||||
expect(scope.billingCode).toBeNull();
|
||||
expect(scope.issueId).toBe("issue-1");
|
||||
});
|
||||
|
||||
it("resolves a null billing code without querying when the run has no issue in context", async () => {
|
||||
const { db, select } = makeDb([]);
|
||||
|
||||
const scope = await resolveLedgerScopeForRun(db, "company-1", makeRun({
|
||||
projectId: "context-project",
|
||||
}));
|
||||
|
||||
expect(scope).toEqual({
|
||||
issueId: null,
|
||||
projectId: "context-project",
|
||||
billingCode: null,
|
||||
});
|
||||
expect(select).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves a null billing code when the issue is not visible to the company", async () => {
|
||||
const { db } = makeDb([]);
|
||||
|
||||
const scope = await resolveLedgerScopeForRun(db, "other-company", makeRun({
|
||||
issueId: "issue-1",
|
||||
projectId: "context-project",
|
||||
}));
|
||||
|
||||
expect(scope).toEqual({
|
||||
issueId: null,
|
||||
projectId: "context-project",
|
||||
billingCode: null,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -2667,7 +2667,7 @@ export function resolveLedgerCostStatus(input: {
|
|||
return input.costUsd == null && hasTokenUsage ? "unpriced" : "reported";
|
||||
}
|
||||
|
||||
async function resolveLedgerScopeForRun(
|
||||
export async function resolveLedgerScopeForRun(
|
||||
db: Db,
|
||||
companyId: string,
|
||||
run: typeof heartbeatRuns.$inferSelect,
|
||||
|
|
@ -2680,6 +2680,7 @@ async function resolveLedgerScopeForRun(
|
|||
return {
|
||||
issueId: null,
|
||||
projectId: contextProjectId,
|
||||
billingCode: null,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -2687,6 +2688,7 @@ async function resolveLedgerScopeForRun(
|
|||
.select({
|
||||
id: issues.id,
|
||||
projectId: issues.projectId,
|
||||
billingCode: issues.billingCode,
|
||||
})
|
||||
.from(issues)
|
||||
.where(and(eq(issues.id, contextIssueId), eq(issues.companyId, companyId)))
|
||||
|
|
@ -2695,6 +2697,7 @@ async function resolveLedgerScopeForRun(
|
|||
return {
|
||||
issueId: issue?.id ?? null,
|
||||
projectId: issue?.projectId ?? contextProjectId,
|
||||
billingCode: issue?.billingCode ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -11653,6 +11656,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
|||
agentId: agent.id,
|
||||
issueId: ledgerScope.issueId,
|
||||
projectId: ledgerScope.projectId,
|
||||
billingCode: ledgerScope.billingCode,
|
||||
provider,
|
||||
biller,
|
||||
billingType,
|
||||
|
|
|
|||
Loading…
Reference in New Issue