Absorb config-layer precedence into a typed layer chain (links-va-001-config-layer-vhf.1)#204
Merged
brandon-fryslie merged 1 commit intoJun 10, 2026
Conversation
…-config-layer-vhf.1)
The global-vs-project precedence lived in statement order, an inline
root guard, a [LAW:single-enforcer] comment, and a variadic-faked
optional parameter. It is now one value: an ordered layers slice built
once by configLayers and merged unconditionally in slice order.
- [LAW:one-source-of-truth] the slice ordering is the only encoding of
precedence; the justification comment is deleted, not restated.
- [LAW:dataflow-not-control-flow] the 'if root != ""' guard dissolves:
absence flows as PathSpec data into mergeConfigFile, which already
treats an absent layer as contributing nothing.
- [LAW:types-are-the-program] Load(workspaceRoot ...string) permitted
the illegal Load("a","b"); it now takes one pathspec.PathSpec whose
zero value is 'no workspace'. first() deleted.
Deviations from the epic sketch (per va-001 family precedent):
- ordered slice instead of map[ConfigLayer][]string — a map loses
ordering, and ordering IS the precedence; the map is a false theorem.
- the chain stays package-internal; Load remains the public boundary
returning Config (callers want settings, not provenance).
Behavior note: with LIT_CONFIG_PROJECT_PATH set and no workspace root,
the project layer now merges instead of being silently dropped (env
vars are test-only knobs; production behavior unchanged — verified
byte-identical old-vs-new binary output on the live store). New test
pins the honest env-override semantics.
There was a problem hiding this comment.
Z.ai Coding Agent Review
No must-change items. The PR cleanly absorbs config-layer precedence into a typed layers chain, which is a strict improvement on multiple fronts:
- [LAW:dataflow-not-control-flow]: The old
if root := …; !root.IsEmpty()guard that conditionally skipped the project layer is replaced by an unconditionallayers.mergeloop where absent layers contribute nothing as data. - [LAW:types-are-the-program]:
Load(...string)→Load(pathspec.PathSpec)makes the "absent root" case the zero value rather than a missing variadic arg. - [LAW:one-source-of-truth]: Layer ordering lives in exactly one place (
configLayers). - [LAW:single-enforcer]: One merge loop replaces the duplicated
mergeConfigFilecalls with separate error handling blocks.
Pre-existing / minor notes for the author's attention (not blocking):
- The
Loaddocstring (line 92–93) says "when a workspace root is present," but the new test confirms the project layer is also consulted viaLIT_CONFIG_PROJECT_PATHwithout a workspace root. The comment is now slightly inaccurate — worth updating to match the new behavior. - The new test at line 270 uses
Load()in itsFatalfmessage while all other updated tests useLoad(pathspec.PathSpec{})— minor inconsistency. - The
mergemethod does I/O (mergeConfigFile) inside its loop, which is a mild [LAW:effects-at-boundaries] concern, but config loading is itself a boundary operation and this was pre-existing.
✅ Approved
brandon-fryslie
added a commit
that referenced
this pull request
Jun 10, 2026
…recedence.First enforcer (links-va-001-precedence-jg6.1) (#207) Three surviving helper variants (templates.firstNonEmpty, cli.firstNonEmptySyncBranch, cli.firstNonEmptySyncRemote — the latter two byte-identical) collapse into internal/precedence.First. The epic's proposed trim bool is deliberately not implemented: every sync candidate is already trimmed where it is produced, and template content must never be trimmed — so trim is a property of the value at its production boundary (the PathSpec precedent), not a mode of resolution. [LAW:no-mode-explosion] [LAW:single-enforcer] Helper-specific sync test becomes the precedence package test matrix. The other three filed sites (config.first, workspace.firstNonEmptyTrimmed, error_output.firstNonEmptyString) were already absorbed by #202-#204.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements links-va-001-config-layer-vhf.1 — F-16, config layering precedence types (Tier 2).
What changed
The global-vs-project config precedence lived in four pellets: statement order in
Load, an inlineif root != ""guard, a[LAW:single-enforcer]justification comment, and a variadic-faked optional parameter (Load(workspaceRoot ...string)+first()). It is now one value:merged unconditionally in slice order by
layers.merge.mergeConfigFile, which already treats an absent layer as a no-op (PR refactor(paths): absorb trimmed-path guards into pathspec.PathSpec (links-va-001-pathspec-drq.1) #203 foundation).Load("a", "b");Loadnow takes a singlepathspec.PathSpecwhose zero value is 'no workspace'.first()deleted.Deviations from the epic sketch (family precedent: sketch is a starting point)
map[ConfigLayer][]string— a map loses ordering, and ordering is the precedence; the map is a false theorem about the domain.MergeRequired()became the return value oflayers.merge— no separate accessor needed.Loadremains the public boundary returningConfig; callers want settings, not provenance ([LAW:composability]).Behavior note
With
LIT_CONFIG_PROJECT_PATHset and no workspace root, the project layer now merges instead of being silently dropped. The env vars are test-only knobs (grep: no non-test consumers), so production behavior is unchanged; the new semantics are the honest ones — an explicit override is never silently ignored. Pinned byTestLoadHonorsProjectEnvOverrideWithoutWorkspaceRoot.Verification
go test ./...clean.46034b5) vs new binary output byte-identical on the live store:lit ready,lit backlog,lit queue,lit show.