From aac0e11ae67c78a35745ed9a25688aa51c15e793 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 17:00:12 +0000 Subject: [PATCH] Reuse the declaration validation path across siblings --- command.go | 70 ++++++++++++++++++---------------- docs/roadmap/audit/catalog.md | 7 +++- docs/roadmap/implementation.md | 9 +++-- docs/roadmap/remediation.md | 58 +++++++++++++++++++++------- 4 files changed, 91 insertions(+), 53 deletions(-) diff --git a/command.go b/command.go index d48f009..5467d46 100644 --- a/command.go +++ b/command.go @@ -443,42 +443,46 @@ func validateCommandDeclaration(command Command) error { func validateCommandPlacement(command Command) error { var issues []error - collectDeclarationIssues(command, nil, true, &issues) + // Keep the grown slice header across recursive calls so siblings reuse capacity. + var path []string + var collectDeclarationIssues func(Command, bool) + collectDeclarationIssues = func(command Command, root bool) { + if command.record == nil { + issues = append(issues, errors.New("invalid zero command declaration")) + return + } + record := command.record + length := len(path) + path = append(path, record.name) + defer func() { path = path[:length] }() + var label string + if len(record.issues) > 0 || (record.rootOnly && !root) { + label = commandpath.Path(path).String() + } + for _, issue := range record.issues { + if label != "" { + issues = append(issues, fmt.Errorf("command %q: %w", label, issue)) + } else { + issues = append(issues, issue) + } + } + if record.rootOnly && !root { + issues = append(issues, fmt.Errorf("root-only command %q is nested beneath another command", label)) + } + if record.invalid || record.absent { + return + } + for _, child := range record.children { + if child.record == nil { + continue + } + collectDeclarationIssues(child, false) + } + } + collectDeclarationIssues(command, true) return errors.Join(issues...) } -func collectDeclarationIssues(command Command, parents []string, root bool, issues *[]error) { - if command.record == nil { - *issues = append(*issues, errors.New("invalid zero command declaration")) - return - } - record := command.record - path := append(parents, record.name) - var label string - if len(record.issues) > 0 || (record.rootOnly && !root) { - label = commandpath.Path(path).String() - } - for _, issue := range record.issues { - if label != "" { - *issues = append(*issues, fmt.Errorf("command %q: %w", label, issue)) - } else { - *issues = append(*issues, issue) - } - } - if record.rootOnly && !root { - *issues = append(*issues, fmt.Errorf("root-only command %q is nested beneath another command", label)) - } - if record.invalid || record.absent { - return - } - for _, child := range record.children { - if child.record == nil { - continue - } - collectDeclarationIssues(child, path, false, issues) - } -} - func validateCommandName(name string) error { return commandpath.ValidateComponent(name) } diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index a9933c6..503669e 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -491,8 +491,11 @@ Also discussed in the [responsibility assessment](A1-responsibility.md#machinery ## F22 — P3: Successful declaration validation eagerly formats every full path -**Complete.** The walk reuses a scratch path and formats it only for retained -issues or root-only placement errors. Focused declaration and registry tests pass. +**Complete.** The walk retains one shared scratch slice header across siblings, +truncates on every return, and formats only retained issues or root-only placement +errors. The [R1 follow-up](../remediation.md#r1--p3-declaration-validation-can-still-copy-a-shared-deep-prefix-for-every-sibling) +completes the initial path refactor and records source review, passing focused +and full suites, and a deep branching consumer check. `collectDeclarationIssues` copies and formats each ancestral path before knowing whether a diagnostic is needed. A valid depth-N declaration chain incurs Θ(N²) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index caa29c2..97beb18 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -564,10 +564,11 @@ Neither finding closes merely because the new tree methods exist. ## Stage 17 — Format declaration paths only for diagnostics -**Complete.** Declaration traversal uses the recursive scratch path and formats -labels only for emitted issues. Focused declaration, registry and retained-error -tests pass offline with Go 1.27.0 and `-mod=readonly`; source review confirms -valid paths are not eagerly copied and formatted during this walk. +**Complete.** Declaration traversal now captures one shared scratch slice header, +retaining grown capacity across siblings and truncating on every return. Labels +are formatted only for emitted issues. The [R1 follow-up](remediation.md#r1--p3-declaration-validation-can-still-copy-a-shared-deep-prefix-for-every-sibling) +corrects the initial by-value traversal and records source-level complexity +verification, passing focused and full suites, and a deep branching consumer check. Findings: F22. Source: `command.go` collectDeclarationIssues. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 581a233..8ffe37a 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Implementation efficiency remediation -Status: one open P3 efficiency finding. Reviewed source: +Status: R1 complete; no open findings. Audited source before remediation: `1cba65fffe966cb70f48c324baf551502544cbad`. ## Scope and architecture @@ -42,7 +42,7 @@ path, rather than proposing a different algorithm or public contract. | 14 | Shared token consumption advances through short spellings and consumed values; it does not rescan each complete short cluster. | | 15 | Implementation retains the selected context-local memoization and source ordering. No residual independent suffix walks were found. | | 16 | Implementation uses the selected resolver tree and owned-path results for distinct-prefix discovery and descendant lookup. Those selected algorithms were not reopened. | -| 17 | **R1:** a child's grown path slice is not retained for siblings, allowing repeated deep-prefix copying. | +| 17 | **R1 complete:** the traversal now retains grown scratch-path capacity across siblings. | | 18 | Operation-preparation extraction preserves the selected independent inputs and existing preflight rules; no new repeated algorithm was introduced. | | 19 | Capture extraction preserves existing selector/scope processing. The explicitly deferred indexing work remains outside this audit. | | 20 | Operand/error attribution extraction preserves the selected dense reconstruction and replay merge, without introducing general sorting. | @@ -52,7 +52,18 @@ path, rather than proposing a different algorithm or public contract. ## R1 — P3: Declaration validation can still copy a shared deep prefix for every sibling -**Code:** [command.go](../../command.go), collectDeclarationIssues lines +**Complete.** validateCommandPlacement now owns one path slice captured by its +recursive collectDeclarationIssues visitor. Each node appends its name and +defers truncation to its entry length, including on invalid/absent early returns. +Truncation preserves the grown slice header and capacity for subsequent siblings. +Successful traversal therefore takes O(N) amortized path work with O(D) scratch +storage; diagnostic labels are still formatted only when emitted. Child/issue +order, error wrapping, root-only checks and declaration handling are preserved. +See [remediation verification](#remediation-verification) for the checks performed. + +The analysis below records the defect at the audited revision. + +**Audited code:** [command.go](../../command.go), collectDeclarationIssues lines 450–479, particularly append at line 456 and recursive calls at line 478. validateCommandPlacement starts the walk with nil parents; installation reaches it through the candidate validator. @@ -110,19 +121,16 @@ strings. This needs neither a public API change nor restrictions on declaration size or depth. It completes the selected design without promising that all App construction becomes linear. -**Verification and status:** a temporary public consumer installed a valid +**Audit verification:** a temporary public consumer installed a valid 32-node shared prefix with 64 leaf siblings and dispatched a leaf successfully. The prefix depth was selected at an ordinary slice-capacity boundary. This confirms that the workload shape is accepted; it is not a timing or allocation measurement. The copying bound follows from the recursive slice-header ownership and append semantics above. -Use existing declaration/registry behavioral checks to preserve results, and -review that grown capacity survives sibling traversal. Include a branching -shape when checking the remedy; a chain alone misses this case. No timing -threshold or benchmark is necessary. Stage 17 and F22 are currently marked -complete; their completion evidence should be corrected to account for this -remaining case when remediation is performed. +At audit time, Stage 17 and F22 were marked complete without accounting for this +branching case. Their completion evidence now records the shared slice remedy +and links to this follow-up. ## Validation and limits @@ -133,7 +141,29 @@ cached Go 1.27.0 binary with `GOTOOLCHAIN=local`, `GOPROXY=off`, and Cached successful results were reused for unchanged code. Those suites validate behavior, not asymptotic copy costs. -No production code, repository tests, manifests or existing roadmap records were -modified. No benchmarks, performance profiles, real-world latency/memory data, -external services, credentials or publication operations were used. No additional -ordinary correctness defect was established during this algorithmic review. +During the audit, no production code, repository tests, manifests or existing +roadmap records were modified. No benchmarks, performance profiles, real-world +latency/memory data, external services, credentials or publication operations were +used. No additional ordinary correctness defect was established during this +algorithmic review. + +## Remediation verification + +The focused declaration/registry checks passed uncached, including retained +causes, absence, invalid declarations, nested root-only groups and transactional +installation. The independent core suite and aligned three-module suite also +passed offline using cached Go 1.27.0 with `GOTOOLCHAIN=local`, `GOPROXY=off` and +`-mod=readonly`. The aligned command was +`go test -mod=readonly ./... ./integrations/ckconfig/... ./integrations/ckprompt/...`; +the independent core check used `GOWORK=off go test -mod=readonly ./...`. + +The temporary public consumer was rerun successfully with a valid depth-32 prefix +and 64 leaf siblings. This checks installation and dispatch for the branching +shape. Source inspection establishes the complexity improvement: all recursive +calls update the same slice header, growth survives return, and each node's +deferred truncation restores its parent's path on every exit. Owned diagnostic +strings are constructed before sibling traversal can change the scratch path. + +Existing behavioral tests were reused; no timing/allocation threshold or new +benchmark was introduced. These results concern this validation walk; other +construction phases retain their own costs.