feat: document merge precedence; display list in profile load order (#201) - #202
Merged
Conversation
…201) lmm list showed mods in DB install order (installed_at), which has no relationship to what actually decides merge precedence for a deploy_mode: compile game (Icarus). Switched to core.OrderByProfile - the same seam the TUI's mod list already uses (Overview, service_core.go) - rather than the deploy-only GetInstalledModsInProfileOrder: that helper deliberately OMITS a mod absent from the profile's load order (correct for deploy, where an untracked mod must never silently deploy), which would have made such a mod vanish from a listing instead of just showing up first (lowest priority, since it has no claim to "final say"). Using OrderByProfile gives list.go the exact same order the TUI already shows, so CLI and TUI now genuinely agree - true parity, confirmed with the coordinator - rather than inventing a third, list-only convention. Added a "Merge precedence" paragraph to README and docs/configuration.md, verified against the actual merge implementation (internal/source/icarus/merge.go): later-in-load-order mods win conflicting table-row fields via a per-field upsert (untouched fields from earlier mods survive); bundled assets are whole-file last-wins with a warning (install/update surface it; reorder itself regenerates the pak silently); the bottom of the load order has final say; lmm profile reorder regenerates the merged pak immediately. list's help text now names the load-order behavior explicitly (ran `make man` to regenerate the stale committed man page the genman test caught).
There was a problem hiding this comment.
Pull request overview
This PR documents merge precedence for deploy_mode: compile (Icarus) and aligns lmm list output ordering with the profile load order used by the TUI and by merge precedence semantics.
Changes:
- Documented compile-mode merge precedence (field-level upserts; asset collisions are whole-file last-wins with warnings) in README and configuration docs.
- Updated
lmm listto order installed mods usingcore.OrderByProfile(profile load order; never omits installed-but-untracked mods). - Added/updated CLI tests to lock in load-order listing behavior and help-text wording; regenerated the
lmm listman page; added a changelog entry.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a merge precedence explanation for compile-mode mods. |
| docs/configuration.md | Adds merge precedence details under deploy mode configuration. |
| cmd/lmm/list.go | Orders lmm list output by profile load order via core.OrderByProfile; updates help text. |
| cmd/lmm/list_test.go | Adds a help-text regression test ensuring “load order” is mentioned. |
| cmd/lmm/list_order_test.go | Adds ordering regression tests (text + JSON) including “missing from load order still listed”. |
| docs/man/man1/lmm-list.1 | Regenerates/updates man page description for the new list ordering semantics. |
| CHANGELOG.md | Records the user-visible change to lmm list ordering and related docs. |
Suppressed comments (1)
cmd/lmm/list_order_test.go:59
- Same as above: the assertion message says "Mod B (later in load order)" but Mod B is first in the configured profile load order [B, A]. Update the message to match the test setup.
t.Run("verbose", func(t *testing.T) {
out := listVerbose(t, svc, game, false)
idx := modOrder(t, out, "Mod B", "Mod A")
assert.Less(t, idx[0], idx[1], "Mod B (later in load order) must print before Mod A")
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+55
| Mods are printed in the profile's load order (see 'lmm profile reorder') | ||
| - the same order that decides merge precedence for a compiled/merged pak: | ||
| a mod later in the load order is merged later and wins conflicting rows. | ||
| A mod installed but missing from the load order is still shown (never | ||
| silently dropped), placed first since it has no claim to the final say. |
Collaborator
Author
There was a problem hiding this comment.
Fixed in a27cc48 — wording now says field-level last-wins on a shared row (help text + regenerated man page), and the test assertion messages match the test's actual [B, A] ordering.
Comment on lines
+16
to
+20
| Mods are printed in the profile's load order (see 'lmm profile reorder') | ||
| - the same order that decides merge precedence for a compiled/merged pak: | ||
| a mod later in the load order is merged later and wins conflicting rows. | ||
| A mod installed but missing from the load order is still shown (never | ||
| silently dropped), placed first since it has no claim to the final say. |
Collaborator
Author
There was a problem hiding this comment.
Fixed in a27cc48 — wording now says field-level last-wins on a shared row (help text + regenerated man page), and the test assertion messages match the test's actual [B, A] ordering.
Comment on lines
+49
to
+53
| t.Run("non-verbose", func(t *testing.T) { | ||
| out := listNonVerbose(t, svc, game) | ||
| idx := modOrder(t, out, "Mod B", "Mod A") | ||
| assert.Less(t, idx[0], idx[1], "Mod B (later in load order) must print before Mod A") | ||
| }) |
Collaborator
Author
There was a problem hiding this comment.
Fixed in a27cc48 — wording now says field-level last-wins on a shared row (help text + regenerated man page), and the test assertion messages match the test's actual [B, A] ordering.
- cmd/lmm/list.go help text said a later mod "wins conflicting rows" - the actual merge semantics are field-level (a per-field upsert on a shared row, not a whole-row overwrite), matching the README/ configuration.md wording already written for #201. Ran `make man` to regenerate the now-stale lmm-list.1 page. - list_order_test.go's assertion messages labeled Mod B as "later in load order" when the test's own ReorderMods call ([B, A]) actually makes Mod A last (final say) and Mod B first (lowest priority) - a failure would have pointed at the wrong mod. Reworded to state the actual array order and which mod has final say.
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
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.
Summary
Two-part pre-release polish:
docs/configuration.md(Deploy Mode section), verified against the actual merge implementation (internal/source/icarus/merge.go): later-in-load-order mods win conflicting table-row fields via a per-field upsert (untouched fields from earlier mods survive); bundled assets are whole-file last-wins with a warning; the bottom of the load order has final say;lmm profile reorderregenerates the merged pak immediately.lmm listdisplayed DB install order (installed_at), which has no relationship to what actually decides merge precedence. Switched tocore.OrderByProfile— the same seam the TUI's mod list already uses (Overview,service_core.go) — instead of the deploy-onlyGetInstalledModsInProfileOrder, which deliberately omits a mod absent from the load order (correct for deploy; wrong for a listing, where it would make the mod vanish instead of just appearing first at lowest priority). This gives the CLI and TUI genuine ordering parity — confirmed with the coordinator before implementing, since it's a real fork from the issue's literal "append at the end" wording (see the report for the full trace).lmm list's help text now names the load-order behavior explicitly;make manregenerated the stale committed man page.Test plan
go build ./...,go vet ./...,gofmt -l .— all cleango test ./...(fresh,go clean -testcache) — full suite green, 17 packagestrunk check— clean (0 new issues)load ordermentioned,install ordernever claimed)Full report:
.superpowers/sdd/2026-08-01-prerelease-batch/report-201.md🤖 Generated with Claude Code