GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — feat(aws-marketplace): bill a listing's LLM usage to AWS through a per-period metering tally by waleedlatif1 · Pull Request #9921 · simstudioai/sim Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
[Critical impact] The PR sends metered usage into AWS Marketplace billing for configured listings. Fix the two unbilled-usage paths before merging. FindingsSummaryAdds AWS Marketplace usage billing through per-period tallies and hourly reports.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 13 files
Confidence score: 2/5
route.ts is not scheduled by the Helm chart, so Marketplace usage metering never runs in standard deployments. Add a cron job for this endpoint at the intended 15-minute cadence.0456_aws_marketplace_usage_report.sql leaves existing organization-linked licenses with a NULL start time, so the metering query excludes their usage. Backfill those rows to the migration time.metering.ts can drop usage during a mid-hour license handoff: the old license ends at expiry, but the new one starts at the next hour boundary. Adjust the handoff reporting window so usage between those times is included.
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/billing/aws-marketplace/metering.ts">
<violation number="1" location="apps/sim/lib/billing/aws-marketplace/metering.ts:184">
P2: A mid-hour license handoff drops usage. When the lapsed license's final row covers only up to its expiry, the next license starts at the following hour boundary, so usage between expiry and that boundary is never billed. Start the successor's chain at the lapsed license's expiry instead of `last.hourStart + HOUR_MS`, or carry the exact boundary forward.</violation>
</file>
<file name="apps/sim/app/api/cron/aws-marketplace-meter/route.ts">
<violation number="1" location="apps/sim/app/api/cron/aws-marketplace-meter/route.ts:25">
P1: This handler is not scheduled by the Helm chart, so Marketplace usage metering never runs in the standard deployment. Add a `cronjobs.jobs` entry for `/api/cron/aws-marketplace-meter` with the intended 15-minute schedule.</violation>
</file>
<file name="packages/db/migrations/0456_aws_marketplace_usage_report.sql">
<violation number="1" location="packages/db/migrations/0456_aws_marketplace_usage_report.sql:19">
P1: Existing organization-linked licenses receive `NULL` here and are then excluded by the metering query, so their organization’s model usage is never billed. Backfill linked rows to the migration time while leaving unlinked rows `NULL`.</violation>
</file>
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@cubic-dev-ai review this PR |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…gh a metering ledger
…iod, so in-place cost top-ups, renewals and cancellations bill exactly
…and keep out-of-window records from failing a batch
baa1879 to
5dd3ea9
Compare
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@cubic-dev-ai review this PR |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 13 files
Confidence score: 3/5
metering.ts, reporting an ended license as soon as it becomes inactive can miss usage that arrives during the one-hour post-end window, leaving it unbilled. Wait until that window closes before sending the final rows.packages/db/schema.ts, deleting an organization cascades to its unsent usage rows, so that usage can never be billed. Preserve those rows or prevent the cascade.
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/billing/aws-marketplace/metering.ts">
<violation number="1" location="apps/sim/lib/billing/aws-marketplace/metering.ts:511">
P2: Sending an ended license's rows as soon as its status flips to `inactive` closes its record before the one-hour post-end window ends. Usage that arrives after that send is skipped, not billed, because `chargeOrganization` returns 0 once `sentAt` is set. Hold the ended license's record until the post-end window closes, or document that usage after the send is intentionally dropped.</violation>
</file>
<file name="packages/db/schema.ts">
<violation number="1" location="packages/db/schema.ts:2023">
P2: The `organizationId` FK on `aws_marketplace_usage_report` cascades on organization delete, so deleting an org drops its unsent usage rows. The license keeps its ARN but loses its link, so that owed usage is never billed to AWS. Consider a non-cascading FK, or flushing pending rows before the org is removed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| eq(awsMarketplaceUsageReport.status, 'pending'), | ||
| or( | ||
| lt(awsMarketplaceUsageReport.hourStart, new Date(floorHour(now))), | ||
| eq(awsMarketplaceLicense.status, 'inactive') |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
P2: Sending an ended license's rows as soon as its status flips to inactive closes its record before the one-hour post-end window ends. Usage that arrives after that send is skipped, not billed, because chargeOrganization returns 0 once sentAt is set. Hold the ended license's record until the post-end window closes, or document that usage after the send is intentionally dropped.
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/billing/aws-marketplace/metering.ts, line 511:
<comment>Sending an ended license's rows as soon as its status flips to `inactive` closes its record before the one-hour post-end window ends. Usage that arrives after that send is skipped, not billed, because `chargeOrganization` returns 0 once `sentAt` is set. Hold the ended license's record until the post-end window closes, or document that usage after the send is intentionally dropped.</comment>
<file context>
@@ -0,0 +1,560 @@
+ eq(awsMarketplaceUsageReport.status, 'pending'),
+ or(
+ lt(awsMarketplaceUsageReport.hourStart, new Date(floorHour(now))),
+ eq(awsMarketplaceLicense.status, 'inactive')
+ )
+ )
</file context>
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| { | ||
| organizationId: text('organization_id') | ||
| .notNull() | ||
| .references(() => organization.id, { onDelete: 'cascade' }), |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
P2: The organizationId FK on aws_marketplace_usage_report cascades on organization delete, so deleting an org drops its unsent usage rows. The license keeps its ARN but loses its link, so that owed usage is never billed to AWS. Consider a non-cascading FK, or flushing pending rows before the org is removed.
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/db/schema.ts, line 2023:
<comment>The `organizationId` FK on `aws_marketplace_usage_report` cascades on organization delete, so deleting an org drops its unsent usage rows. The license keeps its ARN but loses its link, so that owed usage is never billed to AWS. Consider a non-cascading FK, or flushing pending rows before the org is removed.</comment>
<file context>
@@ -1993,6 +2008,71 @@ export const awsMarketplaceLicense = pgTable(
+ {
+ organizationId: text('organization_id')
+ .notNull()
+ .references(() => organization.id, { onDelete: 'cascade' }),
+ dimension: text('dimension').notNull(),
+ periodStart: timestamp('period_start').notNull(),
</file context>
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| and( | ||
| eq(usageLog.billingEntityType, 'organization'), | ||
| eq(usageLog.billingEntityId, organizationId), | ||
| gt(usageLog.billingPeriodEnd, new Date(now - PERIOD_SETTLE_MS)) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
countNewUnits stops reading a period 24 hours after its end, but reporting-window payers can still add cost to it. AWS customers are provisioned as Enterprise customers with reporting windows. For those payers, /api/billing/update-cost omits payerSubscriptionId, so recordCumulativeUsage keeps updating the original row instead of moving later cost into a new period.
A callback or retry arriving after the cutoff adds billable cost that no future meter run reads. Keep those changes discoverable, or ensure reporting-window writes roll forward before applying this cutoff.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| const inForce = licenses.find( | ||
| (license) => | ||
| license.status === 'active' && (!license.expiresAt || license.expiresAt.getTime() > now) | ||
| ) | ||
| if (inForce) return { license: inForce, hourStart: floorHour(now) } | ||
| const ended = licenses | ||
| .filter((license) => license.endedAt && license.endedAt.getTime() >= now - AFTER_END_MS) | ||
| .sort((left, right) => (right.endedAt?.getTime() ?? 0) - (left.endedAt?.getTime() ?? 0))[0] | ||
| return ended?.endedAt ? { license: ended, hourStart: floorHour(ended.endedAt.getTime()) } : null |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Expired licenses miss final charges
chargeTarget cannot flush an expired license while its stored status is still active. After expiresAt, the active branch rejects it, but the fallback requires endedAt. That field is only stamped after verification marks the license inactive.
A license expiring at 10:20 can gain usage after the 10:15 meter run, then have no charge target at 10:30. If no prompt event arrives, routine verification can wait a day—past AWS's one-hour final-report window. The final usage then goes unbilled. Use the known expiry as an end time when no replacement is in force, and send the final record without waiting for the status change.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
5dd3ea93
Deployed
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.