# Additional Public API Review

Reviewed revision: `3ebb848eadf6e61275ecec6e35838ad6576cdd3a`.
Status: N1–N2 accepted; N3–N6 remain discussion findings. Implementation has not begun.

## Scope And Assessment

This review accepts the inventory in [review.md](review.md) and independently
assesses the public interface through consumer workflows, declaration comments,
the [configuration contract](../config.md), and targeted implementation tracing.
It covers loading/discovery, source and resource ownership, paths and collection
registration, callbacks, inspection/diffs, secrets, and diagnostics. It does not
repeat the symbol inventory or conduct a correctness/performance audit.

[api.md](api.md) owns the accepted D1–D8, A1, and N1–N2 changes. Those decisions remain
settled; public path comparison (A2) remains deferred. The findings here are
additional opportunities, including consequences of the accepted cleanup.

The principal API boundaries remain useful. No broader consolidation of loading
options, filesystem roles, callbacks, or the two history projections is warranted
by this review. The remaining opportunities concern vocabulary, removal of an
obsolete input-error category, and a missing convenience on failed loads.

| Finding | Recommendation | Compatibility impact if accepted |
| --- | --- | --- |
| N1: discovery `Paths` (accepted) | Rename to `SearchPaths`; decision recorded in [api.md](api.md). | Go function rename. |
| N2: main-tier provenance (accepted) | Rename `LayerBase` to `LayerMain`, with value `"main"`; decision recorded in [api.md](api.md). | Go constant rename and observable string change. |
| N3: shared-history quota | Rename `CodeExplainLimitExceeded` to `CodeHistoryLimitExceeded`, with value `"history.limit_exceeded"`. | Go constant rename and machine-readable code change. |
| N4: recording-kind diagnostic | Retire `CodeResolveInvalidKind` if D6 removes its public trigger. | Conditional constant removal. |
| N5: failed-load source lookup | Add `LoadError.Source(id)` alongside accepted A1. | Compatible addition. |
| N6: import-default field names | Consider `FollowSymlinks` and `IncludeHidden`; lower priority. | Go field renames; YAML keys unchanged. |

These are design recommendations, not defect severities or additional release
gates. N2's string-value change is accepted. N3's code-string change still requires
deliberate approval; accepting other naming cleanup does not authorize it.

## N1. Distinguish Discovery Paths From Semantic Paths

Status: accepted. [api.md](api.md) owns the selected change; the discussion below
preserves its rationale and alternatives.

Evidence: [path.go](../../path.go) defines `Path`, `Keys`, and `ParsePath` for
configuration-value coordinates. [discovery.go](../../discovery.go) defines
`Paths(paths ...string) Search` for literal host-file candidates. In the same
package, the plural of `Path` therefore constructs a different concept rather
than a collection of semantic paths. The preliminary catalog recommends retaining
the name and explaining the distinction.

Recommendation: rename the discovery constructor to
`SearchPaths(paths ...string) Search`. This aligns it with `SearchDirectory` and
makes a call such as `SearchPaths("./app.yml", "/etc/app.yml")` recognizable
without inspecting its return type. Keep literal-path interpretation, candidate
order, deferred acquisition, and all search behavior unchanged. No rename of
`Path`, new path type, or discovery redesign is needed.

Alternative: retain `Paths`. It is concise inside `SearchConfig` and its comment
is accurate, but package completion and standalone calls still mix filesystem
and configuration-path vocabulary. `FileCandidates` is another viable name,
although it is less consistent with `SearchDirectory`.

## N2. Use Main Consistently In Provenance

Status: accepted, including the string-value change. [api.md](api.md) owns the
selected change; the discussion below preserves its rationale and alternatives.

Evidence: `LoadLayered(main, ...)`, `SourceInfo.UnderlayIndex`, the configuration
contract, and the accepted `SourceVersionModeMainOnly` name distinguish main input
from lower-priority underlays. [records.go](../../records.go) instead exposes
`LayerBase = "base"` for the main tier. A consumer presenting provenance must
learn that this base is above underlays and is distinct from application defaults.
The preliminary catalog treats this as a documentation concern.

Recommendation: use `LayerMain = "main"` so API calls, provenance records, and
rendered layer labels use the same term. Preserve the existing layer category and
all precedence rules; imported contributions retain their owning tier. This is
a vocabulary change, not a new layer or a change to source acquisition order.

Changing the literal affects applications that compare, serialize, or display
layer strings as well as those referring to the Go constant. If selected, migrate
those uses and document the change together before v1.

Alternative: rename only the Go constant and keep `"base"`. That reduces string
compatibility impact but preserves two names for the same concept in application
code and rendered evidence. Keeping both the existing symbol and value is also
reasonable if the modest clarity gain does not justify migration; the preferred
pre-v1 outcome is one consistent term.

## N3. Name The Retained-History Limit For Its Actual Scope

Evidence: [records.go](../../records.go) exposes
`CodeExplainLimitExceeded = "explain.limit_exceeded"`. Its production sites include
`executeLoadRequest` in [load.go](../../internal/engine/load.go) and
`inheritanceHistoryBudgetDiagnostic` in
[inheritance_walk.go](../../internal/engine/inheritance_walk.go). It can fail a load
without an application calling `Explain`. The completed shared model supplies
metadata, explanation, historical membership, and internal attribution readers.

Recommendation: rename the constant to `CodeHistoryLimitExceeded` and its value
to `"history.limit_exceeded"`. This describes the resource being bounded and
avoids suggesting that skipping explanation would avoid the load failure.
Preserve the quota, retained-node accounting, diagnostic locations, and admission
timing. This does not add a limit setting or change which configurations load.

Alternative: rename the Go constant but retain the existing code string, or keep
both names unchanged and clarify the diagnostic documentation. Either preserves
more consumer compatibility, but a retained `"explain.limit_exceeded"` remains
misleading to applications that work directly with diagnostic codes. A coordinated
pre-v1 change is preferable if the new terminology is accepted.

## N4. Review The Diagnostic Made Obsolete By D6

Evidence: the production use of `CodeResolveInvalidKind` is the kind guard in
`resolutionState.addRecord` in
[resolution.go](../../internal/engine/resolution.go). Today, public `Record` can
pass arbitrary `ChangeKind` values into it. Accepted D6 replaces that method with
two methods that select supported intents; `Default` and `Normalize` also supply
fixed supported kinds. This is distinct from D8's already unproduced
`CodeOptionsUnavailable`.

Recommendation: as part of D6 implementation, confirm that no supported public
input can produce `resolve.invalid_kind`, then remove its exported constant.
Retain the other recording diagnostics and the inspection `ChangeKind` values.
Internal defensive checks can remain where useful; their representation does
not require an unreachable public configuration-error category.

Alternative: retain the constant for consistency with previous diagnostics or
a concrete future use. That costs little code but leaves an unexplained public
error category after its input mechanism has disappeared. Do not add a public
trigger simply to justify keeping it. The removal remains conditional on the
final D6 implementation, rather than assuming a guard must be deleted now.

## N5. Offer Direct Source Lookup On Failed Loads

Evidence: [diagnostic.go](../../diagnostic.go) exposes `LoadError.Diagnostics()`
and `LoadError.Sources()`. Diagnostic primary and related locations refer to
source IDs. Accepted A1 adds direct lookup on successful results, while
`Resolution.Source` already supports it during callbacks. A custom error reporter
still has to scan or index the failed load's source slice to follow the same IDs.

Recommendation: add this companion to A1:

```go
func (e *LoadError) Source(id string) (SourceInfo, bool)
```

It should return a detached record from the same partial inventory as
`LoadError.Sources()`, with false for empty/unknown IDs and nil/zero errors.
Preserve the existing failed-load redaction and available evidence; a lookup
does not complete acquisition or upgrade a partial destination/version record.
This is passive lookup, with no filesystem access or new source-completeness
guarantee. Existing source lookup/conversion helpers can be shared privately;
no public registry, interface hierarchy, or constant-time guarantee is needed.

Alternative: keep only `Sources()`. That is sufficient for applications using
the standard diagnostic renderer or building an index once. The addition can
also wait until after v1, since it is compatible. Adding it with A1 is a small,
coherent way to remove the same consumer bookkeeping on both outcomes of a load.

## N6. Make The Go Import Defaults More Self-Describing

Evidence: [lifecycle.go](../../lifecycle.go) exposes `ImportDefaults.Follow` and
`ImportDefaults.Hidden`. Their comments explain symbolic-link traversal and
inclusion of hidden files/directories, but an options literal alone does not
name the object being followed or say whether hidden entries are included.

Recommendation, at lower priority: consider `FollowSymlinks` and `IncludeHidden`
for those Go fields. Preserve `*bool`, nil defaulting, explicit false, the current
search scope, and the YAML keys `follow` and `hidden`. This would make the public
Go configuration more descriptive without changing the import language. Leave
`Required`, `Recursive`, and `Pattern` alone; they do not have the same ambiguity
in context.

Alternative: retain the current Go names to mirror YAML exactly. That is a real
benefit for readers moving between authored imports and application defaults,
and existing comments already resolve the ambiguity. This is an optional naming
refinement, weaker than N1's collision between two different path concepts.

## Discussion And Validation

Discuss the renames and string-value changes first, then N4 alongside D6 and N5
alongside A1. N6 can be decided independently. Accepted additions should be moved
into [api.md](api.md); this audit is not an implementation plan or a permanent
second API reference.

The review used the trusted catalog, root declarations and comments, current
contracts and consumer guidance, and graph-assisted production-reference checks.
No production code, tests, accepted decisions, or deferred features were changed.
No independent behavioral audit or performance benchmarks were run. Validation
for this document consists of the local documentation-link check and whitespace
review; it is not evidence that the proposed API changes are implemented.