Skip to content

ORB-91: Reserve AI quota atomically - #460

Open
thomasluizon wants to merge 4 commits into
mainfrom
fix/orb-91-atomic-ai-quota
Open

ORB-91: Reserve AI quota atomically#460
thomasluizon wants to merge 4 commits into
mainfrom
fix/orb-91-atomic-ai-quota

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Aug 7, 2026

Copy link
Copy Markdown
Owner

DEGRADED: same-vendor review

Closes ORB-91.

What changed

All ORB-91 gated AI provider paths now use TryConsumeAiMessage to reserve quota atomically at the provider boundary. Habit setup checks its response cache first. Tag suggestions load existing tags first. Chat finishes its non-provider context and prompt work first. Each path reserves immediately before its provider call.

Chat FAQ cache hits return without reserving quota. Cache misses reserve exactly once. A reservation is retained when a provider call fails because provider work may already have incurred cost.

The old post-provider counter writes in habit setup and tag suggestions were removed, along with their unused repository and logging dependencies.

Concurrency proof

The chat race regression now creates ten request-local repository and unit-of-work scopes over ten distinct stale User snapshots. One request persists the twentieth free-plan message. The other nine saves throw DbUpdateConcurrencyException; the real pay-gate retry discards pending changes, reloads the persisted count, and rejects them at the quota guard. The result is one successful response, nine pay-gate failures, one model call, and a persisted count of 20.

Verification

  • Focused ORB-91 regressions: 12 passed
  • dotnet build Orbit.slnx --no-restore: 0 errors
  • dotnet test tests/Orbit.Application.Tests --no-build --no-restore: 2,978 passed
  • dotnet test --no-build --no-restore: 5,597 passed
  • node tools/arch-map.mjs: regenerated both architecture artifacts with 0 orphan endpoints and 0 untested feature folders

No external response field, flag, exit code, or API response shape is read by this change.

@thomasluizon

Copy link
Copy Markdown
Owner Author

Implementation approach

  1. Replace the read only AI message gate with a consuming reservation operation in IPayGateService and PayGateService. The service will calculate the effective limit, increment through the existing optimistic concurrency retry path, and complete its save before returning success. This keeps quota ownership inside the request path and before any model call.
  2. Call that reservation from ProcessUserChatCommand.Context.cs, then remove quota accounting from RunBackgroundPostResponseWork in ProcessUserChatCommand.Persistence.cs. Background work will retain fact extraction only.
  3. Audit SuggestHabitSetupCommand and SuggestTagsQuery. These setup helpers will remain deliberately free, with a URL linked reason at each read only gate, rather than silently consuming chat quota.
  4. Add the concurrent one remaining reservation case, the existing at limit behavior, normal Pro consumption, and the failure accounting decision to the Application tests. A started model call will retain its reservation because provider work may already have incurred cost; there will be no refund after an external failure.

Expected files are the pay gate interface and service, the two chat partials, the two suggestion handlers, and the existing chat and pay gate test files. I am using the repository's existing optimistic concurrency retry support instead of an application lock or a detached increment because the database concurrency token provides cross instance serialization without a process local critical section.

@thomasluizon

Copy link
Copy Markdown
Owner Author

Approach correction after the required call site audit

Current main no longer matches the ticket's assertion that the habit setup and tag suggestion paths never increment. SuggestHabitSetupCommand already increments through ConcurrencyRetry after a successful uncached suggestion, and SuggestTagsQuery increments and saves after a successful suggestion. They are metered paths, not deliberately free, so I will preserve their current behavior and call that audit result out in the PR body.

The atomic reservation change remains scoped to chat, where the count is still detached into post response work and the acceptance test requires a ten request burst. I will add a consuming pay gate operation for chat, reserve through the existing User concurrency token before the model call, and remove only chat's deferred increment. This avoids changing the accounting point of the two already metered setup helpers beyond what the current ticket can prove and test.

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44d9440ad1

ℹ️ 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".

Comment thread src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.Context.cs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

DEGRADED: same-vendor review

Independent xhigh review found two blocking items on the current implementation:

  1. High — incomplete AI-quota audit. Only the chat handler uses the new atomic reservation. SuggestHabitSetupCommand and SuggestTagsQuery still follow the old check/model-call/meter sequence, so concurrent requests through those gated AI paths can still exceed quota.
  2. High — concurrency regression does not model stale reads. The test returns one shared in-memory User instance to all requests, which lets even non-atomic mutation appear serialized and does not exercise the database concurrency retry path.

These findings are frozen for one focused fixer round. No merge was performed.

@thomasluizon

Copy link
Copy Markdown
Owner Author

I will close the three blocking findings in three narrow slices:

  1. Update SuggestHabitSetupCommandHandler and SuggestTagsQueryHandler so all cache and context reads happen first, then TryConsumeAiMessage reserves quota immediately before the external suggestion call. Remove their post-provider counter writes and cover allowed, blocked, and cached behavior at the handler seam.
  2. Replace the shared-user chat concurrency test with request-local stale User snapshots backed by a shared persisted counter/version. Each request will use its own repository and unit of work, stale saves will throw DbUpdateConcurrencyException, and the real pay-gate retry will reload before rechecking quota. This proves one remaining slot permits one provider call and rejects the other requests.
  3. Move chat reservation out of context loading to the provider boundary after the FAQ cache return and remaining context reads. Add cache hit and cache miss regressions proving zero and one reservations respectively.

The implementation will stay in the existing pay-gate contract and handler files. I am not adding process locks, a second quota abstraction, or post-provider metering because those shapes either do not coordinate across instances or reopen the original race.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 6ef920a0f0

ℹ️ 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".

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