Document capacity scheduling audit results
This commit is contained in:
153
audit.md
153
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.
|
||||
|
||||
Reference in New Issue
Block a user