From 5adc1c1e6459923dbca0aab7fb6221280dc1be6a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 4 Nov 2026 13:18:32 +0000 Subject: [PATCH] Record algorithmic efficiency audit findings --- docs/roadmap/audit/02-efficiency.md | 147 ++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 docs/roadmap/audit/02-efficiency.md diff --git a/docs/roadmap/audit/02-efficiency.md b/docs/roadmap/audit/02-efficiency.md new file mode 100644 index 0000000..0e2be80 --- /dev/null +++ b/docs/roadmap/audit/02-efficiency.md @@ -0,0 +1,147 @@ +# Algorithmic Efficiency Audit + +Reviewed 2026-11-04 against commit `0d27252`. + +## Scope And Method + +Reviewed PromptKit-owned configuration assembly, provider-value conversion and +copying, rendering and requirement analysis, inventory selection, prepared-run +ownership, validation orchestration, repair formatting, retry coordination, +observations, and capacity management. Findings below follow from implementation +and operation counts; no benchmarks or live-provider calls were needed. + +The public facade delegates to internal components. ConfigKit supplies the final +configuration; PromptKit constructs immutable runtime indexes and captured +templates. Execution uses operation-local validation plans, shared backend +capacity, and a generation coordinator. Inspection reads the captured snapshot. +ConfigKit and ReleaseKit internals are outside this repository audit. + +### Deliberate Choices Preserved + +Checked current internal documentation and relevant retired plans in Git history, +including the performance plan at `8a5c0de`, configuration adoption at `5f7b194`, +and collections at `89af2a1`. This audit does not reopen: + +- lexical inventory ordering, recursive provider-map overlay semantics, or + conservative requirement-discovery semantics; +- captured parsing, per-render artifact-text reuse, operation-local schema + compilation, frozen retry payloads, and linked-list FIFO permits with + constant-time cancellation removal; +- the existing JSON depth/work bounds, output/diagnostic limits, or copies + deliberately retained to protect public and injected-collaborator ownership; +- synchronous filesystem/template/schema operations or the selected retry and + repair budgets. + +The opportunities below concern implementation work that those choices do not +require. Complexity statements use expected constant-time Go map operations and +treat identifier comparison/hashing cost separately. No new public performance +guarantee, cache, caller limit, or release gate is proposed. + +## 1. Condition Evidence Unions Can Require Quadratic Work + +**Priority:** P2. Requirement-inspection path; most relevant to deeply nested +conditions rather than ordinary small templates. + +**Code:** [evaluateCommand](../../../internal/prompt/requirements.go:142) and +[mergeConditionEvaluations](../../../internal/prompt/requirements.go:318). + +Each comparison recursively evaluates its operands, then allocates fresh +`missingVariables` and `contentInputs` maps and copies both operands' accumulated +evidence. A left-heavy expression with distinct missing caller keys, such as +successively nesting `eq (eq .v0 .v1) .v2`, carries an increasingly large missing +key set through each ancestor. Template parsing accepts these expressions; +inspection can encounter them even when their eventual execution would fail or +the inspection result is partial. + +For an expression of size n, successive unions copy approximately +2 + 3 + ... + n entries: **Theta(n²) total set work and cumulative allocation**. +The final unique evidence has only O(n) entries. This is separate from the +previously fixed repeated syntax-subtree scan: visiting each syntax node once +does not eliminate repeated copies of the evidence produced by those nodes. + +**Opportunity:** Transfer ownership of temporary sets and merge the smaller set +into the larger one, rather than copying both at every comparison. For this +expression family that is O(n); small-to-large merging gives O(n log n) aggregate +set-insertion work across general expression trees. Another option is an +operation-local evidence accumulator that records each discovery directly, +while keeping only condition truth/limitation state on recursive return values. +Choose the simpler approach after checking temporary-set ownership. Preserve +current grammar, classifications, deduplication, sorted output, cancellation, +and partial-result behavior; interpreting more expressions is not necessary. + +## 2. Recursive Provider Overlay Repeatedly Clones Descendant Subtrees + +**Priority:** P3. Construction-time improvement with an existing depth bound. + +**Code:** [overlayProviderMap](../../../internal/configuration/targets.go:131). + +The helper deep-copies the entire lower map before considering overrides. When +both values at a key are maps, it recursively invokes itself on a descendant +that was already copied, and copies that whole descendant again. The first copy +is then replaced and discarded. All descendants on an overlapping path can be +copied once per ancestor. + +Let N be the lower tree's node count and D its overlapping map depth. Node-copy +work can reach **O(ND)**. For a narrow chain it is the triangular sum +D + (D-1) + ... + 1. A wide K-entry leaf below D overlapping ancestors is cloned +roughly D times instead of once. `jsonvalue.CopyMap` also sorts map keys, so the +same wide leaf's sorting work is repeated at those ancestors. + +The existing maximum container depth of 100 is important: this is **not an +unbounded quadratic-in-N claim under today's fixed limits**. It is an avoidable +depth-dependent multiplier in traversal and cumulative allocations within those +limits, multiplied again when multiple profiles overlay the same backend. That +bound and the recursive-map/value-replacement semantics remain selected choices. + +**Opportunity:** Clone the lower tree once, then overlay into the owned clone +without re-cloning existing descendant maps. Copy incoming replacement values +where needed to preserve isolation from configuration inputs. Alternatively, +construct the merged output in one traversal. Either removes the repeated +ancestor copies: expected O(N + M) merge traversal for lower/upper trees of sizes +N and M, apart from the retained validation/sorting costs. Preserve null and +array replacement, empty-map behavior, input ownership, and existing JSON +validation/bounds; do not replace ConfigKit's configuration machinery. + +## 3. Filtered Listings Copy And Scan The Complete Inventory + +**Priority:** P3. Scalability opportunity for engines with many namespaces. + +**Code:** [filterIdentityList](../../../internal/engine/engine.go:306), +[ListPrompts](../../../internal/engine/engine.go:270), and the +[configuration inventory methods](../../../internal/configuration/listing.go:14). + +A named-namespace request first obtains a detached copy of all N identities and +then scans it to build its k matching results. Even an absent namespace costs +**Theta(N) time and O(N) temporary storage**, rather than work proportional to +the selected inventory. Listing C namespaces separately costs Theta(CN); when +each has a bounded number of entries, that becomes quadratic in total inventory +size despite only O(N) total returned entries. + +The collection implementation plan at `89af2a1` expressly allowed namespace +buckets or sorted ranges to avoid repeated full scans; it did not require the +present whole-inventory-copy algorithm. The lexical ordering and caller-owned +result contracts do not prevent indexed selection. + +**Opportunity:** Capture per-kind namespace buckets or ranges alongside existing +construction-time inventories, then copy only the selected entries. Buckets +allow expected O(1 + k) selection/copy; binary search over sorted full IDs can +support named-prefix ranges in O(log N + k). Keep unqualified and all-ID modes +correct without assuming unqualified entries form one contiguous lexical range. +Resolve filters before requesting the inventory, and retain exact namespace +matching, full IDs, ordering, context behavior, and independent returned slices. +An all-ID request must still spend O(N) to return N owned entries; that is not a +defect. No public pagination API or mutable cache is needed. + +## Review Outcome + +These three opportunities are supported by direct operation-count analysis; +they are not measured latency claims. The largest asymptotic issue is growing +condition-evidence copying. The overlay finding is explicitly bounded, and the +listing finding matters as independent namespace count grows. + +No additional ordinary correctness defect was established in this pass. The +findings in [the correctness audit](01-correctness.md) remain separate and were +not duplicated here. No production code, tests, dependencies, or established +behavior were changed, so the successful race-suite/vet evidence recorded there +still applies to the unchanged implementation; those checks were not rerun just +for this documentation change.