diff --git a/docs/roadmap/audit/02-efficiency.md b/docs/roadmap/audit/02-efficiency.md new file mode 100644 index 0000000..a65ef75 --- /dev/null +++ b/docs/roadmap/audit/02-efficiency.md @@ -0,0 +1,206 @@ +# Algorithmic correctness and efficiency audit + +Status: four open P3 efficiency findings. Reviewed source: +`d8da761a213c82837276266faeebf93beda8eb08`. + +## Scope and architecture + +The [internal overview](../../internal/overview.md) inventories the implemented +owners. Core captures declarations, validates installation batches, resolves +routes and bindings, coordinates invocations, and supplies completion and shared +reporting. Its neutral metadata and internal Cobra/pflag adapter separate public +contracts from backend state. ckconfig prepares scopes and comparison inputs and +projects ConfigKit evidence. ckprompt assembles PromptKit operations and adapts +native requests, catalogs, reports, and smoke results. Applications own loaders, +builders, and execution dependencies; upstream libraries own domain algorithms. +Release launchers delegate maintainer operations to ReleaseKit. + +This pass reviewed declaration/registry traversal, alias compilation and help +discovery, parsing/completion, operand allocation, captured binding replay, +report conversion and delivery, integration selection/filtering/projection, and +maintained consumer/tooling boundaries. Findings describe source-level scaling, +not measured bottlenecks or new performance guarantees. No benchmarks or +real-world performance measurements were gathered. + +Explicitly selected approaches were excluded from reconsideration: + +- Fresh mutable backend state per invocation, immutable metadata reuse, and + report buffering, as selected in the retired performance records and current + [internal overview](../../internal/overview.md). +- The finite pushdown termination analysis, longest-prefix routing semantics, + canonical flag endpoints, scoped precedence, and intermediate-route discovery + selected in the retired alias implementation plan at `f42aefe`. +- Completion's explicit routing check for each proposed child, selected in the + retired completion implementation plan at `10a799a`. +- JSON report normalization, whole-report aggregation, and the composed example's + invocation-local list cache using reflect.DeepEqual, selected in the retired + adoption implementation plan at `2f43f78`. +- The [deferred scope/selector and backend work](../future.md#possible-further-efficiency-work), + upstream domain algorithms, and the completed dense occurrence reconstruction, + heap merge, validation-filter indexing, and subtree-prefix preparation in + [remediation.md](../remediation.md). + +The findings preserve those decisions. The plans prescribe endpoint, precedence, +discovery, and validation behavior, but do not require independent repeated +chain walks, repeated processing of identical prefixes, full-catalog scans per +rewrite, or eager diagnostic path formatting at every valid node. + +## Validation and interpretation + +All three independent module default suites passed with Go 1.27.0 using +`GOWORK=off GOTOOLCHAIN=local go test -mod=readonly ./...`, run separately from +each module directory. The aligned workspace suite also passed using +`GOTOOLCHAIN=local go test -mod=readonly ./... ./integrations/ckconfig/... ./integrations/ckprompt/...`. +The unchanged source reused successful cached results. Dependency downloads were +disabled. No production files or repository tests were modified. + +A temporary external consumer confirmed that the workload shapes below are +accepted: a flag-alias chain with an active command endpoint and inactive group +contexts; a single-token group-alias chain whose help has no descendant aliases; +distinct aliases sharing a discovery-only prefix; and a valid nested declaration +chain. These are behavioral checks, not timing experiments. The complexity +findings follow from the implementation's loops and copies. + +Complexities below use expected constant-time map operations and count tokens +or records separately from their string-byte costs. They describe the identified +phase, not the entire construction or execution pipeline. Nested loops over +disjoint child collections, required independent input copies, sorted output, +and work proportional to actual serialized output are not findings by themselves. + +No additional ordinary correctness defect was established during this pass. +The six open findings in [01-correctness.md](01-correctness.md) remain separate; +passing default suites do not resolve those findings. + +## E1 — P3: Flag-alias compilation repeatedly resolves shared chain suffixes + +**Code:** [flag_aliases.go](../../../flag_aliases.go), resolveFlagAliases lines +226–251 and resolveFlagTarget lines 254–279; compileFlagAliases invokes this work +independently for each canonical and discovery context. + +**Circumstances and complexity:** consider A effective mappings forming +`--a1 → --a2 → … → --aA → --value`. Each source starts a fresh visited map and +chain walk. The suffix traversed for `--a2` was already traversed for `--a1`, and +so on. There are Θ(A²) total target examinations and corresponding cumulative +chain/visited storage operations per context, despite only A mappings. The same +cost occurs when the final flag is unavailable and all these global aliases are +inactive. Across C contexts with the same chain shape, this part can cost +Θ(CA²), apart from sorting, canonical metadata, and string processing. Peak +temporary storage for a single walk is O(A). + +**Potential refactoring:** resolve the effective functional graph once per +context using visitation states and memoized terminal results. Cache both an +available canonical endpoint and an unavailable endpoint, and detect a cycle +using the active traversal stack. This reduces graph traversal to expected O(A) +work and O(A) scratch storage per context. Keeping the existing source sort for +deterministic validation leaves O(A log A) total per-context work for these +steps. A context-local cache suffices; no cross-invocation provider cache is +needed. + +Canonical spellings must still terminate target resolution before following a +mapping of the same spelling. Resolve each source's target before separately +checking active source collisions. Preserve scoped overrides, inactive globals, +scoped missing-target failures, useful cycle diagnostics, and revalidation after +registration. Results cannot be shared indiscriminately between contexts. + +## E2 — P3: Alias help scans every source for every intermediate route + +**Code:** [help_view.go](../../../help_view.go), routeHelpAliases lines 38–51; +[resolver.go](../../../internal/alias/resolver.go), ResolutionPaths lines 139–150. + +**Circumstances and complexity:** let P be the number of paths visited while +rewriting the entered help path and A the number of declared command aliases. +For every visited path, the function scans all A aliases and copies each source +through FromTokens before checking its length and prefix. This performs PA +source examinations, plus repeated source-token copying and prefix comparisons, +even when no source can contribute an entry. + +For a valid chain of A single-token aliases ending at a canonical group, +`help a1` visits Θ(A) paths. Every source is one token long, so every candidate +is discarded by the length check. Discovery of an empty alias section still +takes Θ(A²) examinations/copies. This cost recurs when requesting that help +again even if the routing snapshot is already prepared; it is separate from +registration, termination analysis, and backend construction. + +**Potential refactoring:** query an immutable source-prefix index for strict +descendants of each visited prefix. The existing internal alias trie is a +possible owner; alternatively prepare a source index with the routing snapshot. +Building such an index is proportional to total source tokens. Prefix lookup +then visits matching descendants rather than the entire catalog. For the +single-token-chain example, the identified discovery work becomes O(A), with no +descendant entries to enumerate. General cases still pay for prefix lengths, +matching candidates, authoritative target resolution, and sorting the result. + +Keep intermediate-route discovery and resolve each surviving composed entered +path through the existing resolver. Preserve longer overrides, canonical-target +validation, suffix deduplication, and deterministic presentation. A prefix index +does not authorize enumerating additional composed routes or introducing the +deferred alias-completion feature. + +## E3 — P3: Discovery-context preparation rebuilds shared prefixes repeatedly + +**Code:** [flag_aliases.go](../../../flag_aliases.go), discoverFlagContexts lines +181–200, particularly the prefix cloning/key construction at 185–187 and +resolution at 194–195. + +**Circumstances and complexity:** R distinct alias sources can share a prefix +of depth D, for example `legacy nested … action1`, `legacy nested … action2`, +and so on, all targeting an ordinary canonical group. Every source independently +enumerates the same D proper prefixes. For each prefix length k, it copies k +tokens, builds a key, and may resolve and classify the prefix again. The +prefixes map deduplicates only the eventual result; it does not prevent this +work, including for prefixes previously classified as ineligible. + +With bounded component lengths and no rewrite within those proper prefixes, +the shared-prefix portion costs Θ(RD²), although the source declarations contain +only Θ(RD) tokens and there are only D distinct proper prefixes. When R and D +both grow, that is cubic work in their common scale for quadratic input size. +Additional rewrite work can increase the classification cost further. + +**Potential refactoring:** enumerate distinct proper source prefixes once, +preferably by traversing the already compiled source trie, then run the existing +classification once per unique prefix. Retaining full paths and keys once per +prefix is sufficient; no change to command routing is required. In the example, +source traversal plus prefix materialization/classification can be +O(RD + D²), instead of O(RD²). More generally, account for total source tokens, +each distinct prefix's representation, and one resolution per distinct prefix. + +A seen set consulted only after rebuilding each full prefix key would remove +duplicate resolutions but retain the Θ(RD²) cloning/key cost. Use a traversal +that actually shares prefix processing. Preserve exclusion of canonical paths, +exact alias sources, and prefixes intercepted by valid canonical operand space; +cache negative classifications too if using an alternative memoized traversal. +The source index proposed in E2 could support this work as well. + +## E4 — P3: Successful declaration validation eagerly constructs every full path + +**Code:** [command.go](../../../command.go), collectDeclarationIssues lines +444–470, especially path copying and formatting at 451–453. + +**Circumstances and complexity:** every visited declaration copies its entire +ancestral path and clones/joins it again to make a diagnostic label before +checking whether any issue needs that label. For N valid commands arranged in +a depth-N chain, this performs Θ(1 + 2 + … + N) = Θ(N²) token-copying and label +construction work, with quadratic cumulative allocation, while producing no +diagnostic. A valid tree can be checked for retained issues and root-only +placement in one traversal. + +**Potential refactoring:** maintain one scratch path for the validation walk using +an append/truncate stack and format it only when emitting an actual issue or a +root-only placement error. With ordinary amortized slice growth, the successful +validation walk becomes O(N) work and O(D) path storage for depth D. Error cases +also pay for the path strings actually included in their diagnostics. + +Preserve child order, retained error identity, root-only placement checks, and +invalid/absent declaration handling. This changes scratch diagnostic preparation, +not captured declaration ownership. Other installation phases retain their own +metadata/path costs; this finding does not claim that complete App construction +would consequently become linear for every tree shape. + +## Recommendation + +Consider E1 first: memoizing a context's flag graph removes repeated suffix +work with a small, local change. E2 and E3 can share a private source-prefix +traversal/index while retaining the selected resolver and discovery semantics. +E4 is an independent simplification of diagnostic traversal. All four are +efficiency opportunities for consideration, not evidence of measured operational +impact or a reason to reopen the selected algorithms listed above.