Algorithmic correctness and efficiency audit

Status: four open P3 efficiency findings. Reviewed source: d8da761a213c82837276266faeebf93beda8eb08.

Scope and architecture

The internal overview 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.
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, upstream domain algorithms, and the completed dense occurrence reconstruction, heap merge, validation-filter indexing, and subtree-prefix preparation in 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 remain separate; passing default suites do not resolve those findings.

E1 — P3: Flag-alias compilation repeatedly resolves shared chain suffixes

Code: 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, routeHelpAliases lines 38–51; 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, 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, 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.

12 KiB Raw Blame History Unescape Escape