From 805e48c873eec32d7e7fd5f5ebb17156c27430e8 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 16:51:28 +0000 Subject: [PATCH] Document capacity scheduling audit results --- audit.md | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/audit.md b/audit.md index f2e1441..b678478 100644 --- a/audit.md +++ b/audit.md @@ -3700,3 +3700,156 @@ that: - Stage 17 may reconsider extra-parameter validation marshaling only if a representative benchmark or payload model establishes material cost; this stage found no standalone efficiency defect there. + +## Stage 15: Capacity, Admission, And Generation Scheduling + +### Scope Reviewed + +The review covered all production and test code in `internal/capacity`, the +capacity assembly in `NewEngine`, ordinary and prepared admission in +`internal/usecase`, the root capacity contract suite, and the prepared-capacity +contract case. The internal capacity document supplied the durable ownership, +bounded-admission, FIFO, cancellation, and release expectations. Backend +normalization, ordinary and prepared execution semantics, provider transport, +and public error mapping were treated as established inputs from their owning +stages except where they integrate directly with capacity. + +The refreshed code graph bounded the scheduler to one engine-local `Manager`, +one immutable map of limited-backend pools, one immediate admission operation, +and one model-client wrapper. Source synchronization then followed every +admission and active-permit state transition under its pool mutex, including +the grant/cancel race and panic unwinding, and traced the exact manager instance +through engine construction, both runner entry points, initial generation, and +the internal repair integration. + +### Accepted Findings + +None. The implementation matched the documented capacity invariants, and the +focused ordinary, race-enabled, repeated, and coverage-guided checks did not +establish a correctness, security, maintainability, or material efficiency +defect. + +### Unresolved Observations + +None. The only uncovered statement branch in the focused package coverage run +was `pool.acquire` receiving an already-canceled context before it can reserve +or enqueue. Source proves that branch returns the context error while holding +the same lock used for all state changes, and assembled limited-backend runs +normally reject that state at the preceding admission check. Adding a root +test for the internal defensive branch would duplicate the owning package +boundary without protecting a distinct consumer contract. + +### Coverage Ledger + +- **Construction and ownership:** `NewEngine` obtains a copied policy snapshot + from its immutable backend registry, constructs one new manager, and passes + that same instance to the capacity client and runner. Each manager creates + fresh pools and owns no global state, goroutine, timer, worker, shutdown + hook, or persistence. Concurrent map access is read-only after construction, + registered unlimited backends and endpoint-only profiles have no pool, and + separately constructed engines cannot share counters or waiter lists. +- **Policy arithmetic and admission:** Construction rejects blank policy IDs, + nonpositive concurrency, negative queue capacity, and addition overflow + before storing immutable limits. A limited admission locks its pool, checks + context cancellation, compares `admitted` with + `concurrencyLimit + queueCapacity`, and increments at that linearization + point or returns only `ErrCapacityExceeded`. The returned closure uses + `sync.Once`, so concurrent or repeated release cannot underflow the count. + Missing pools take the unrestricted path without allocating coordination + state. +- **Run boundaries and lease lifetime:** Ordinary execution resolves the prompt, + profile, backend, effective target, credential requirements, and output + contract before admission, then installs the release defer before schema, + artifact, render, generation, validation, or repair work. Preparation alone + does not admit. `RunPrepared` first atomically claims the handle and rechecks + credentials, then admits the frozen backend and installs the same defer + before execution. Thus every successful, failing, canceled, or panicking + exit unwinds one whole-run lease, while an internal repair remains part of + its original admission. +- **Active permits and backend selection:** The wrapper selects exactly one + pool from the request's resolved backend ID. Unlimited requests pass the + original context and request directly to the collaborator. Limited requests + increment `active` only below the immutable limit and defer release before + invoking the collaborator, preserving the exact response and error on + ordinary returns and restoring the permit during panic unwinding. Endpoint + overrides retain their selected backend ID. The owning internal repair + integration gives the runner and default repairer the same wrapper, so its + initial and repair calls use one pool while releasing the active permit + between calls. +- **FIFO and cancellation:** A contended call appends one waiter to a + mutex-protected `container/list`. New arrivals cannot bypass existing + waiters. Release removes the front waiter, marks the grant while still under + the lock, and transfers the existing active count directly before closing + the ready channel outside the lock. Cancellation uses that same lock either + to unlink its still-ungranted element and return the context error or to + observe that the grant won and invoke the collaborator with the original, + now-canceled context. These mutually exclusive transitions prevent lost or + double permits and leave no canceled waiter retained by a pool. +- **Lock scope and resource behavior:** Admission and scheduling critical + sections contain only context inspection, integer comparisons and updates, + and constant-time list operations. Collaborator calls, channel waits, + channel close, payload preparation, validation, and repair occur outside + pool locks. Capacity creates no internal goroutines or timers; one waiter + object and channel are allocated only for an actually contended limited + call. Direct FIFO handoff plus cancellation removal prevents later work from + starving behind abandoned entries, and per-backend locks avoid unrelated + backend contention. +- **Test ownership:** Manager tests own policy validation, immutable snapshot + behavior, admission bounds, backend independence, idempotent release, + context handling, and unrestricted paths. Client tests own exact peak + limits, FIFO order, first and middle waiter cancellation, grant/cancel races, + pool independence, pass-through identity, and panic release. Use-case tests + appropriately own admission ordering and lifetime plus shared + initial/repair scheduling; root tests own assembled queue rejection, + endpoint identity, injected-client limits, unlimited concurrency, public + capacity identity, and engine independence. Prepared tests own deferred + admission and execution-time release. These are relational assertions at + distinct boundaries rather than duplicate scheduler implementations. + +### Verification Performed + +The code knowledge graph was refreshed and used first to inventory every +capacity symbol, identify the manager and wrapper as the only stateful +hotspots, and trace their callers through engine construction and ordinary and +prepared execution. All graph conclusions were checked against source, the +capacity document, architecture and testing policies, and the Stage 12 through +14 handoffs. + +The focused package suite reported 97.3% statement coverage; coverage was used +only to locate the defensive pre-canceled-acquire branch discussed above: + +```sh +capacity_audit_cover=/tmp/promptkit-capacity-audit.cover +go test -coverprofile="$capacity_audit_cover" ./internal/capacity +go tool cover -func="$capacity_audit_cover" +``` + +The following focused ordinary, race-enabled, and repeated checks also passed: + +```sh +go test ./internal/capacity ./internal/usecase . -run 'Capacity|Admission|SchedulesInitialAndRepairGenerationThroughOneBackendPool|ReleasesAdmission|UnlimitedBackends|PreparedExecutionCredentialCapacity' +go test -race ./internal/capacity ./internal/usecase . -run 'Capacity|Admission|SchedulesInitialAndRepairGenerationThroughOneBackendPool|ReleasesAdmission|UnlimitedBackends|PreparedExecutionCredentialCapacity' +go test ./internal/capacity ./internal/usecase . -count=50 -run 'Capacity|Admission|SchedulesInitialAndRepairGenerationThroughOneBackendPool|ReleasesAdmission|UnlimitedBackends|PreparedExecutionCredentialCapacity' +``` + +The repository-wide ordinary and race-enabled suites and the maintained +offline consumer workflow passed as well: + +```sh +go test ./... +go test -race ./... +go run ./examples/go-library/prepare +``` + +### Handoff + +- The Stage 0 baseline remains absent and was not backfilled during this + concurrency review. +- Stage 16 can treat admission, backend selection, and generation scheduling + as one engine-local boundary with no accepted defect. Consumer tests should + assert only visible capacity behavior and should not reproduce mutex or + waiter-list mechanics. +- Stage 17 has no capacity-specific optimization candidate from this review. + Any future change to fairness or allocation strategy should first preserve + the current linearization points and be justified by representative + contention measurements.