2026-11-07 12:43:07 +00:00
|
|
|
|
# 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
|
2026-11-07 15:20:25 +00:00
|
|
|
|
[remediation.md](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/264b82bd5759f052a3646d57828945a87b7586d3/docs/roadmap/remediation.md).
|
2026-11-07 12:43:07 +00:00
|
|
|
|
|
|
|
|
|
|
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.
|