5.0 KiB
Complexity audit
Reviewed baseline: 620c832 on main. The review used the local codebase
knowledge graph's cognitive and cyclomatic complexity estimates to identify
candidates, then inspected their code, callers, package boundaries, and
documented contracts. Scores are heuristics, not defect or refactoring
thresholds. No empirical performance measurements or application tests were
run for this documentation-only review. Existing correctness, concurrency,
security, and efficiency findings are recorded in the neighboring audits.
1. Shared validation combines several kinds of orchestration
validation.Run has a graph cognitive score of 78 and cyclomatic score of 37. It validates settings, acquires the Go tool, runs tests, vet and compilation, decides whether to load a committed tree, chooses an error identity for a failed tree load, runs file checks, and attributes a deferred reader-close error. The tree-load condition, the reverse-priority error selection, and the ordered check blocks each separately enumerate selections. Adding a check therefore requires a maintainer to update several distant parts while preserving ordering and close-error attribution.
Consider separating the Go-command sequence and the tree-check sequence into small private functions, with the tree owner responsible for closure and its failure attribution. Keep the selected checks and their order visible in one place; a generic plugin system is unnecessary. Preserve nil versus empty shared file selections, the current first-failure identities and precedence, and the fact that a close failure replaces success only after a content read.
2. Git source inspection is repeated across package boundaries
build.BuildResolved uses its own
gitCommit and cleanSource subprocesses before and after building.
candidate separately reads branch,
commit and status, while git has
CaptureHead and another tagged-source inspection path. These overlap in
commands and interpretation but use different execution and error handling.
Understanding a source-state change or a subprocess fix requires tracing all
three owners. The existing correctness audit identifies
replacement-object and cancellation concerns that make this duplication
particularly relevant.
Consider moving the common commit and worktree-status primitives behind the existing internal Git boundary, with callers composing only the observations they need. Candidate checking still needs branch and tag rules; building still records dirty source rather than rejecting it; delivery still checks a tag. Preserve these different contracts, error context, cancellation behavior, and the timing of before/after observations. Coordinate any such refactor with the existing correctness work instead of treating code consolidation as its fix.
3. Internal build results retain duplicate settings
internal/build.Result stores normalized
Settings alongside separate MainPackage, BinaryName, and Targets
fields containing the same information. BuildResolved initializes both,
and facade conversion reconstructs both on input while
reading Settings on output. Delivery uses Settings for identity and asset
validation. This creates two sources of truth inside the implementation and
additional work when a build setting is added; a future caller could populate
them inconsistently.
Consider retaining Settings as the single internal representation and
updating internal build execution to use it. Keep the existing public
BuildResult fields and the CLI's private receipt format at their respective
boundaries; their distinct types and conversions serve deliberate ownership
and serialization requirements. Verify partial build results and prebuilt
delivery remain unchanged. This is a simplification opportunity, not an
observed mismatch in current callers.
4. CLI command handling interleaves routing and command-specific workflows
run has a graph cognitive score of 85 and combines routing with profile loading, publication and check reporting, build execution, receipt invalidation and verification, and exit-code decisions. The same file also contains delivery preparation and receipt conversions. The build branch alone has several separate failure paths that must be read alongside its success and partial-result output; changes to one command add noise to the central dispatcher.
Consider keeping run as a small dispatcher and moving the check/publish and
build command workflows into named private handlers, grouped by responsibility.
Keep receipt invalidation before build-error reporting, existing messages and
exit statuses, and the CLI's use of the public orchestration facade. Existing
delivery handlers provide a local example of this separation. This does not
require a new command framework or public interfaces.