Union cascade arrays so child-note tags don't clobber folder tags#701
Merged
Union cascade arrays so child-note tags don't clobber folder tags#701
Conversation
…#697) `Emanote.Model.SData.mergeAeson` previously delegated to `aeson-extra`'s `lodashMerge`, which `alignWith`-merges arrays element-by-element. For list-valued frontmatter fields like `tags`, this meant a parent `folder.yaml` declaring `tags: [team-doc]` plus a child note declaring `tags: [internal-note]` produced `[internal-note]` — the cascaded tag was silently clobbered, both in the per-page chip strip and in the post-#352 global tag index. The render path had carried this behavior forever; #352 surfaced the same boundary on the index path. Replace `lodashMerge` with a direct merger that matches cascade semantics: objects deep-merge by key, arrays concatenate then dedup, scalars right-win. The change is local to `mergeAeson` so every cascade caller (`parseSDataCascading`, `getEffectiveRouteMetaWith`, `withAesonDefault`) inherits the new behavior in one place. Drops the `aeson-extra` dependency since `lodashMerge` was its only user. Adds `Emanote.Model.SDataSpec` (8 hspec cases) plus an `issue-697` fixture and two smoke scenarios that exercise both the chip strip and the tag-index page when a note carries cascaded, frontmatter, and inline-body tags simultaneously. Refreshes the now-obsolete fixture comment on `issue-352/note.md` warning future contributors away from inline `#tags` — the underlying constraint no longer holds. Documents the cascade merge rules in `docs/guide/yaml-config.md` so "arrays extend, scalars override" is discoverable next to the cascade description, instead of folklore.
…de callers Address three lowy findings on the just-landed mergeAeson rewrite: 1. Semantic overload at the boundary: mergeAeson is called from one true-cascade site (Meta.getEffectiveRouteMetaWith) and three non-cascade sites (Note.withAesonDefault, Note.overrideAesonText, View.Template.setErrorPageMeta). Renaming to mergeCascadeAeson would mislead the latter; instead, document that the contract is universal and that non-cascade callers happen to never traffic in arrays — they share the merger by design, not coincidence. 2. Stability note: per-key merge strategies are an item on the issue roadmap. The Haddock now flags the right boundary for that change (a new module that owns cascade folding) so future readers don't reach for a strategy parameter on this function. 3. Cascade-vs-non-cascade asymmetry: the contract section makes the shared dependence on object/scalar clauses explicit and lists every call site, so the invisible coupling lowy flagged is visible in the source. No code change beyond the Haddock block.
Code-police elegance pass flagged the post-lowy Haddock as disproportionate (28 prose lines for a 5-line function). Two of the three sections were over-specified: - The non-cascade caller enumeration named three call sites in a docstring that won't be updated when callers move; grep finds those call sites in seconds and the listing will rot. - The stability note prescribed a future module shape — speculative architecture advice that belongs in an issue or PR discussion, not in a function docstring. The Contract section + lodash-divergence paragraph already make the boundary visible: the merge applies uniformly to every caller, and the array clause is documented as the #697 fix. That's enough.
Code-police elegance pass: the bullet was restating the merge contract that docs/guide/yaml-config.md now owns. Trim to the bug-and-cause story and link out for the contract; the docs page is the canonical reference for "how cascade merges work."
Owner
Author
Hickey/Lowy Analysis
Hickey rationale
Lowy rationale
Disposition notes
Generated by |
Owner
Author
EvidenceThe page chip strip on the
Generated by |
Owner
Author
|
| Step | Status | Duration | Verification |
|---|---|---|---|
| sync | ✓ | 1s | git fetch ok; forge=github; noGit=false |
| research | ✓ | 8m 16s | Identified mergeAeson uses lodashMerge (alignWith for arrays); planned the concat-and-dedup fix |
| branch | ✓ | 14s | Created fix/697-cascade-tags-union from origin/master |
| implement | ✓ | 8m 28s | Rewrote mergeAeson with object-key/array-concat-dedup/scalar-right semantics (drops aeson-extra). 8 hspec cases pass; cabal build all green. New issue-697 fixture + e2e scenarios; refreshed stale issue-352 fixture comment |
| check | ✓ | 21s | cabal build all → up to date |
| docs | ✓ | 36s | Added CHANGELOG bullet and 'Cascade merge semantics' section to docs/guide/yaml-config.md |
| fmt | ✓ | 29s | cabal-fmt, fourmolu, hlint, nixpkgs-fmt all passed |
| commit | ✓ | 54s | Primary fix 575ff9ac pushed to origin |
| hickey+lowy | ✓ | 4m 42s | Hickey: no findings. Lowy: 3 findings folded into Haddock revision 8ad010ec |
| police | ✓ | 5m 39s | Pass 1 (rules): clean. Pass 2 (fact-check): clean. Pass 3 (elegance): 2 fixes — 5cb699f0, 2e9dc07c |
| test | ✓ | 3m 14s | cabal test all (74/74) + e2e-live (37/37) + e2e-static (35/37, 2 skipped) + e2e-morph (37/37) |
| create-pr | ✓ | 1m 10s | Draft PR opened with hickey/lowy analysis comment |
| ci | ✓ | 3m 8s | vira ci pipeline succeeded on HEAD 2e9dc07 |
| evidence | ✓ | 3m 14s | Before/after chip-strip screenshots posted |
| Total | 40m 50s |
Optimization suggestions
- Research could shave ~3 min. Cloning
aeson-extrato confirmlodashMerge'salignWithsemantics took most of the research budget. IfEmanote.Model.SData.hswere pre-read before invoking/do, the lodash detour wouldn't be needed — the issue body already names the symbol. - Implement+research were the two heaviest steps (~16 min combined, 41% of total) but neither dominates individually (≥30% threshold not hit). The parallelism that hickey+lowy bought (4m 42s for both) and the
BuildOnlyshort-circuit onvira ci(3m 8s) kept the back half lean. - Police's elegance pass partially walked back the post-Lowy Haddock. The Lowy reviewer asked for a stability note; the elegance pass cut it as speculative. Future runs could resolve this tension earlier — e.g., by treating Lowy's "future-proofing" recommendations as defers-to-issue rather than fold-into-Haddock by default.
- Two police commits could have been one. The Haddock and CHANGELOG trims are independent, so the per-finding rule applies — but a reviewer reading the PR will see four police-flavoured commits and might appreciate them being labelled by category (
refactor(lowy)vsrefactor(police)) more consistently.
Workflow completed at 2026-05-04 12:23 UTC.
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.
Emanote.Model.SData.mergeAesonno longer drops cascade-declared tags when a child note declares its own. The metadata-cascade merger inheritedaeson-extra'slodashMerge, which aligns arrays element-by-element —tags: [team-doc]infolder.yamlplustags: [internal-note]on the leaf produced[internal-note], dropping the cascaded entry both from the per-page chip strip and from the post-#352 global tag index. Closes #697.What changed
The merger is rewritten in five lines and now applies one universal contract:
The array clause is the load-bearing one. Everything else matches lodash, so the four non-cascade callers (
Note.withAesonDefault,Note.overrideAesonText,View.Template.setErrorPageMeta, andparseSDataCascadingitself) inherit the new behaviour without touching their callsites.aeson-extrawas the merger's only user — now removed fromemanote.cabal.Why concatenate-with-dedup is the right cascade semantic
The cascade's job is "defaults that descendants extend." Lodash's array-by-index merge is a third behaviour — neither replace-the-list nor extend-the-list — that comes from JS treating arrays as objects keyed by numeric indices. The same fix generalises to
pandoc.filtersand any other list-valued field a future schema adds; per-key strategies aren't needed.Coverage
Emanote.Model.SDataSpec(object deep-merge, array union, dedup, single-side preservation, nested-array viapandoc.filters, scalar override across a 3-step fold)/issue-697/note.html— chip strip carries the cascaded tag plus the leaf's frontmatter and inline-body tags simultaneously/-/tags/issue-697-cascaded.html— lists the cascaded note even when it declares its own tagsThe stale "do NOT add inline
#tagshere" comment on theissue-352fixture is gone — the constraint it encoded no longer holds.Try it locally
Generated by
/doon Claude Code (modelclaude-opus-4-7).