feat(project-engine-client): meter prompt units as texts × models in the mock (LLMO-5616)#1795
feat(project-engine-client): meter prompt units as texts × models in the mock (LLMO-5616)#1795byteclimber wants to merge 2 commits into
Conversation
…the mock (LLMO-5616) The mock counted raw prompt rows with no model factor, so the `texts × models` prompt-unit identity (serenity-docs#22 §2) — the fact the dynamic-allocation design rests on — was untestable. Per Rainer's follow-up on #22: - quota.js promptsUsed is now Σ_project (texts × models); adds modelsUsed(ws, project) and canAddModel(ws, project). canCreatePrompts takes projectId and sizes the batch by that project's model count. Usage is derived live, so a model add/remove (or prompt delete) re-meters automatically — a model-less project consumes zero units. - The v2 model-attach route 405s when the attach would re-meter existing texts past the allocation; both prompt-create routes pass projectId so the batch is sized texts × models. - quota unit tests (100% branches) cover the multiplier, zero-models-free, and canAddModel; e2e adds the texts × models prompt-create and the model-attach 405 cases. Enables the api-service swap (net 0 → zero transfers) / pure-removal (net < 0 → one release) integration tests (spacecat-api-service#2764), which pair with this mock. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
There was a problem hiding this comment.
Hey @byteclimber,
Verdict: Request changes - two documentation files are now stale after this change.
Complexity: HIGH - medium diff; API surface signal.
Changes: Teaches the Project Engine mock to meter prompt units as texts x models, adding model-attach quota enforcement and updating unit/e2e tests (6 files).
Note: CI checks are currently pending - verify before merge.
Must fix before merge
-
[Important] Package CLAUDE.md is now stale - quota enforcement list incomplete -
packages/spacecat-shared-project-engine-client/CLAUDE.mdThe CLAUDE.md states: "AI-unit quota (the disguised-405 the live API returns for an over-allocation) is in
mock/quota.js, set via thePOST /__quotacontrol route orbuildSeed({ quota }), enforced on project create / prompt write / publish."After this PR, quota is also enforced on model attach (
POST /v2/.../ai_modelsreturns 405 when re-metering existing texts would exceed the allocation). Update the enforcement list to include "model attach" so the doc remains accurate for the next reader. -
[Important]
docs/mock-usage.mdis now stale - ai_models POST route description incomplete -packages/spacecat-shared-project-engine-client/docs/mock-usage.mdThe usage manual documents the AI models POST route as returning only
201. After this PR, the route also returns405when attaching a model would exceed the prompts allocation. Update the route documentation to note the metered behavior, consistent with how other metered routes (project create, prompt write, publish) are documented there.
Non-blocking (3): minor issues and suggestions
- nit:
ai_models.jsquota guard lacks the explanatory comment about 405 being undeclared in the OpenAPI spec -prompts.jshas this comment and maintaining the pattern helps future readers -mock/counterfact/routes/v2/workspaces/{id}/projects/{project_id}/ai_models.js:33 - suggestion: E2E model-attach test could verify the model was NOT persisted after the 405 rejection (a follow-up GET asserting model count == 1 would catch persist-before-check regressions) -
test/e2e/project-engine-mock.e2e.js:1749 - suggestion: Add a cross-project unit test for
canAddModelthat seeds two projects with usage and verifies that project A's consumption counts against project B's ability to add a model -test/mock/quota.test.js
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 45s | Cost: $4.11 | Commit: b06adc0f0be5c660b4ed54592212e5d35e6e152d
If this code review was useful, please react with 👍. Otherwise, react with 👎.
…review nits (LLMO-5616) MysticatBot review on #1795: - CLAUDE.md: quota enforcement list now includes "model attach" + notes the texts × models unit. - docs/mock-usage.md: §6 metered-ops list + the ai_models POST route row document the model-attach 405 (re-metering existing texts past the prompts allocation), consistent with the other routes. Nits: matched prompts.js's raw-405 explanatory comment on the ai_models route; e2e now asserts the rejected model is NOT persisted after a 405; added a cross-project canAddModel unit test (the add cost is the target project's text count). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey @byteclimber,
Verdict: Approve - all prior findings addressed; metering logic is correct.
Complexity: HIGH - medium diff; API surface signal.
Changes: Teaches the Project Engine mock to meter prompt units as texts x models, adding model-attach quota enforcement and corresponding unit/e2e tests (8 files).
Note: Recommend a human read before merge - this change modifies package-level documentation (CLAUDE.md) that anchors the mock's quota contract. The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks are currently pending - verify before merge.
Non-blocking (2): minor issues and suggestions
- nit:
canAddModelusesstore.size(...)directly for the project's text count rather than routing through a named accessor likemodelsUsed()does for models - minor abstraction inconsistency -mock/quota.js:170 - suggestion: Add a symmetric unit test for
canAddModelon a project with zero texts (model attach is free when there are no texts to re-meter) - mirrors the existing "zero models = free" test -test/mock/quota.test.js
Previously flagged, now resolved
- CLAUDE.md updated with "model attach" in enforcement list and texts x models unit explanation
- mock-usage.md updated with metered 405 behavior and multiplier formula
- Explanatory 405 comment added to ai_models.js quota guard
- E2E test verifies rejected model NOT persisted (GET after 405 asserts count == 1)
- Cross-project unit test for canAddModel added (target-project isolation)
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 25s | Cost: $4.10 | Commit: 306e43d1413d61ae2e98203d23204b84461c5fdc
If this code review was useful, please react with 👍. Otherwise, react with 👎.
What & why
The Project Engine mock metered prompts as a raw row count with no model factor, so the
texts × modelsprompt-unit identity that the dynamic-allocation design rests on (serenity-docs#22 §2, live-verified 2026-07-02) was untestable. Per Rainer's follow-up on serenity-docs#22 (comment), teach the mock the multiplier so a regression in the net-delta sizing can be caught.Changes (mock only; nothing published ships)
mock/quota.js:promptsUsedis nowΣ_project (texts × models)— derived live from theprompts:{ws}:{proj}andai_models:{ws}:{proj}collection sizes, so a model add/remove (or prompt/project delete) re-meters automatically. AddsmodelsUsed(ws, projectId)andcanAddModel(ws, projectId);canCreatePrompts(ws, projectId, count)now sizes the batch by the project's model count. A model-less project consumes zero units.aio/prompts,aio/prompts/tagged) passproject_idso the batch is sizedtexts × models.quota.test.js(100% branches — the package gate) covers the multiplier, zero-models-free, andcanAddModel; the e2e adds thetexts × modelsprompt-create 405 and the model-attach 405 cases.Pairs with
Gates
test:typesclean, lint clean, unit tests 100% coverage (lines/branches/statements), e2e green (70 passing).🤖 Generated with Claude Code