diff --git a/docs/roadmap/audit/12-deadcode.md b/docs/roadmap/audit/12-deadcode.md new file mode 100644 index 0000000..e6eb9d9 --- /dev/null +++ b/docs/roadmap/audit/12-deadcode.md @@ -0,0 +1,233 @@ +# Dead Code And Obsolete Functionality Audit + +Reviewed 2026-11-04 against commit `3b76031` and the current worktree. Existing +audit-document moves were preserved. No production code, tests, dependencies, +or functionality were changed. + +## Scope And Assessment + +Reviewed repository-owned Go packages, public entry points, configuration and +runtime values, collaborator interfaces, default/optional paths, embedded +resources, build constraints, and the release launchers and their wiring test. +The architecture remains a public facade over captured configuration, use-case +orchestration, generation/capacity, and focused internal adapters. The former +standalone configuration repositories have already been retired; this review +found smaller residues rather than another abandoned subsystem. + +The review combined Rapid Type Analysis, Staticcheck unused/data-flow checks, +graph caller discovery, source inspection, and the format, internal, policy, +and roadmap documents. Tests count as real callers, but a helper's self-test +does not by itself establish a production need. Conversely, absence from the +examples or default execution path is not evidence that an optional public +feature is obsolete. + +Four cleanup areas warrant consideration. These are low-priority reductions in +state and maintenance surface, not newly demonstrated runtime bugs. None calls +for deleting a supported public feature or imposing a new release gate. + +## 1. Remove Backend Presence Bookkeeping With No Runtime Consumer + +**Relevant code:** [loaded.backends](../../../internal/configuration/load.go:122), +[backendFields and presence capture](../../../internal/configuration/load.go:215), +[effective backend construction](../../../internal/configuration/targets.go:14), +and [Backend.QueueCapacitySet](../../../internal/domain/domain.go:187). + +**Evidence:** Construction allocates a per-backend field-state map and fills it +through `captureFields(..., state.backends)`. No production code reads the +recorded backend states. The only lookup of the captured queue state is in +[TestLoadDefaultsAbsentQueueCapacity](../../../internal/configuration/load_test.go:755). +Unlike profile presence, backend presence is not consumed while constructing +effective execution targets. + +`QueueCapacitySet` is also assigned during target construction and asserted in +tests, but never read to select a production behavior. The current assignment +is simply whether concurrency is positive; it no longer distinguishes an +explicit queue setting from a derived default. The registry's capacity-policy +projection uses `ConcurrencyLimit` and `QueueCapacity`, not this flag. + +**Why obsolete:** Queue defaulting and its provenance are already handled by +[resolveBackends](../../../internal/configuration/resolve_targets.go:11) and +ConfigKit's derived-default record. Typed final values drive runtime capacity; +the retained side map and Boolean do not contribute to that behavior or to the +public inspection view. No documented compatibility or planned-feature need for +these extra records was found. + +**Potential remediation:** Remove the backend-only presence capture, its field +list and retained map, and the unused runtime Boolean/assignment. Adjust tests +to assert effective queue behavior and, where relevant, the existing ConfigKit +inspection/provenance result. Keep `captureFields`, profile field-state +tracking, and prompt-input metadata presence: those have actual consumers and +preserve required absent/null/explicit-value distinctions. Retain explicit +zero queue capacity, derived defaulting, and operator history unchanged. + +This removes unnecessary construction work and retained per-backend maps; it +does not justify dropping configuration provenance or changing its ownership +under the [configuration-boundary ADR](../../adr/0001-configuration-boundary.md). + +## 2. Retire Legacy Fields In The Captured Prompt Representation + +**Relevant code:** [PromptDefinition.OutputFormat](../../../internal/domain/domain.go:148), +[PromptMessageTemplate.ContentFile](../../../internal/domain/domain.go:173), +[prompt materialization](../../../internal/configuration/prompts.go:58), +[resolveOutputContract](../../../internal/usecase/runner.go:639), and +[definition hashing](../../../internal/promptdef/hash.go:12). + +**Evidence:** Supported engine construction requires and validates a complete +output contract. Materialization sets `definition.Validation` to that contract +and duplicates its format into `definition.OutputFormat`. The runner reads +the duplicate only when `Validation.Format` is empty, a condition that the +captured definitions supplied by engine construction cannot have. The standard +runner test-definition helper fills both fields with the same value; no +supported public repository-injection path supplies an alternative definition. + +For messages, materialization resolves the real source `content_file` into +captured `Content` and does not populate the domain `ContentFile` field. The +renderer uses `Content`/captured templates, not that path field. Its one +nonempty assignment is a renderer fixture demonstrating that already-loaded +content is rendered without opening a file. The source-format +`promptMessageConfig.ContentFile` remains actively used and is a different +field. + +**Why obsolete:** The runtime representation still carries source-era message +path data and a second output-format source after configuration loading became +the authoritative materialization boundary. The empty-format fallback is +disconnected from supported engine definitions. No current documentation or +roadmap records a compatibility requirement for these internal duplicate fields. + +**Potential remediation:** Use `PromptDefinition.Validation.Format` as the one +captured format and remove the legacy duplicate/fallback. Remove only the +runtime message path field, adjusting its fixture to exercise captured text. +Preserve the actual YAML `content_file` feature, source-relative resource +resolution, ConfigKit origin inspection, and construction-time capture. Also +preserve the separate, supported request-override rule that an empty replacement +`OutputContract.Format` defaults to text. + +**Serialization caveat:** These fields are included indirectly by +`HashDefinition`'s `json.Marshal`, even when a message path is empty. They are +therefore not completely inert bytes: removing them changes current prompt +hash values. Account for that effect explicitly and protect the documented +opaque-equality relationships between inspection and preparation. Do not claim +bit-identical hashes after changing the hash input, or invent a permanent hash +encoding guarantee merely to perform this cleanup. The fields are internal +representation, not part of the public stable DTO shapes. + +## 3. Remove Unused Diagnostic State And A Discarded Helper Result + +**Relevant code:** [reachabilityProbeOutcome.initialEndpoint](../../../internal/usecase/smoke_reachability.go:24), +[HTTP report assembly](../../../internal/usecase/smoke_tester.go:376), +[fileSource](../../../internal/engine/engine.go:211), and +[SetSchemaFile](../../../internal/engine/engine.go:75). + +**Evidence:** The reachability probe assigns its private outcome's +`initialEndpoint`, but the smoke tester never reads that member. It records +the real public attempt's `InitialEndpoint` from the resolved report target. +Only `TestReachabilityProbeRemovesURLCredentials` inspects the private echo. +Actual sanitized request/evidence and report behavior are established through +other values. + +Separately, `fileSource` returns a filesystem, file name, source-directory +string, and error. Its only caller, `SetSchemaFile`, always discards the +directory result. The directory is still needed internally to construct +`os.DirFS`; exposing it as another return value serves no caller. + +**Potential remediation:** Remove the unused probe-outcome member and its +assignment; retain credential-removal assertions against the actual request, +evidence, or public diagnostic report. Reduce `fileSource`'s private signature +to the consumed results. Keep its directory calculation, file checks, selected +filesystem/root, and the public `WithSchemaFile` option intact. + +Neither cleanup removes `SmokeTestAttempt.InitialEndpoint`, destination +evidence, or single-schema-file support. Those public features are live even +though an extra private field/result is unnecessary. + +## 4. Remove Orphan Helpers Whose Only Callers Are Their Own Tests + +**Relevant code and evidence:** + +- [filecatalog.RelativePath](../../../internal/filecatalog/catalog.go:71) is + called only by `TestRelativePathNested`. Current source discovery and + source-relative resource resolution use other helpers; no production or + public-facade caller uses this wrapper around `filepath.Rel`. +- [resourceNamespaceFromID](../../../internal/domain/resource_identity.go:34) + is called only by `TestResourceNamespaceFromID`. Actual namespace inventory + and filtering paths do not call it. The exported identity-validation and + qualification operations remain active and are not candidates for removal. +- [runner_test.go:stringPtr](../../../internal/usecase/runner_test.go:2592) + has no callers, including in tests. Both reachability analysis and + Staticcheck `U1000` report it. + +Caller searches found no reflection, registration, build-constrained caller, +or supported external API exposing these helpers. The first two belong to +internal packages and have only their direct self-tests; no documented future +or compatibility purpose was found. The third is already test-only and unused. + +**Potential remediation:** Remove the orphan helpers and their now-irrelevant +self-tests, keeping behavioral coverage of real discovery, namespace +inventory/filtering, and identity qualification. Remove the unused pointer +helper without replacing it. There is no reason to introduce a new abstraction +or move the self-tests to another layer solely to preserve these unused APIs. + +## Intentional Retention And Related Findings + +The following distinctions prevent removal based on misleading reachability: + +- `WithSchemaFile` and `InlineWithURI` are supported public helpers despite + being unreachable from the current test executables. The former's missing + public wiring coverage is already in [audit 08](08-testing.md); it is not + evidence that the option should be deleted. +- The older capacity client wrapper and no-repair runner constructor are + expressly retained for focused internal callers/tests in the current + [capacity](../../internal/capacity.md#fifo-generation-permits) and + [runner](../../internal/runner.md#collaborators) documentation. This audit + does not classify them as dead functionality or recommend deleting them. + [Audit 11](11-boundaries.md) discusses the wrapper's architectural placement; + altering that deliberate retention is a separate decision. +- Repository capability fallbacks, standalone rendering/validation, injected + clients' nil usage-presence compatibility, nil-validator/admitter behavior, + and optional provider settings have meaningful callers or documented + purposes. Normal engine assembly taking another path does not obsolete them. +- ConfigKit YAML fields and stable JSON DTO fields can be used by reflection; + formatter methods and interfaces can be invoked indirectly. Embedded + OpenRouter and smoke-schema assets, the Linux FIFO test, and release scripts + likewise have consumers outside a simple direct-call list. They were not + treated as dead. No unused public configuration option or abandoned release + launcher was established. + +The domain smoke-request/full-report cloners have a narrow-or-unused consumer +surface already described in [audit 11, finding 3](11-boundaries.md#3-narrow-smoke-value-copy-helpers-to-their-actual-ownership-boundaries). +That ownership/remediation scope is not counted again here. The schema and +JSON-copy consolidations in prior audits likewise remain their existing work, +not an excuse to delete intentional alternative implementations wholesale. + +## Tooling And Validation + +The checks used local Go 1.27.0 and pinned audit-tool versions: + +```sh +go run golang.org/x/tools/cmd/deadcode@v0.52.0 -test -filter='^go.maximumdirect.net/promptkit(/|$)' ./... +go run honnef.co/go/tools/cmd/staticcheck@v0.8.2 -checks 'U1000,SA4*' ./... +go list -f '{{.ImportPath}} {{join .GoFiles " "}} {{join .IgnoredGoFiles " "}}' ./... +``` + +`deadcode -test` reported the two public helpers and unused test pointer helper +listed above. Staticcheck reported only `stringPtr` (`U1000`) and exited with +status 1 because of that finding; it reported no additional selected `SA4*` +diagnostics. These are audit observations, not successful lint sign-offs. +Neither tool establishes that all tested or serialized state is necessary, +which is why the write-only and redundant fields required manual review. + +Rapid Type Analysis starts from executable roots; tests add roots, and example +programs do not exercise every library consumer path. The analysis is local-build +evidence, not proof about all platforms or downstream programs. In particular, +platform-specific schema handling and optional library operations were retained. + +Existing offline race-enabled tests passed: + +```sh +go test -race -count=1 ./internal/configuration ./internal/domain ./internal/filecatalog ./internal/prompt ./internal/engine ./internal/usecase +go test -race -count=1 -run 'Test(ConfigurationInspection|PreparedExecution|ValidateRequestMatchesPrepareContract|AppendedMessagesComposeFrozenEffectivePrompt)' . +``` + +Tool execution left `go.mod` and `go.sum` unchanged. No production changes, +test removals, provider calls, or new validation gates were made during this +review.