Cloonar/deckle

Fork 0

Drift audit: rules implemented twice across engine, server, client and theme #234

New issue

Closed

opened 2026-10-12 18:31:54 +02:00 by dominik.polakovics · 0 comments

dominik.polakovics commented

2026-10-12 18:31:54 +02:00

Owner

This was generated by AI during triage.

Summary

Tracking index for the drift audit: every rule the engine, server, client or theme evaluates in two or more places gets one owner (CLAUDE.md "Every rule has exactly one owner", #233). The work is sliced into two child issues; this issue closes when both have landed.

Children

#243 — tree payload: server-owned child role, slot wrapper and summary verdicts (audit items 1, 2, 5).
#244 — one owner per rule: chip grammar, defaults, links, params, names, loaders and client fixtures, plus the four behaviour fixes found on the way (audit items 3, 4, 6-14 and the node lookup). Blocked by #243.

Verification status (triage, 2026-10-12, at `dcc1067`)

Every item was re-checked in the source after #239 landed. Already fixed by #239: PartitionItems, l10n.isItem and the text-merge copies (elemops.IsItem and content.MergeText are the owners). Still open: everything else, with these corrections to the audit:

Item 1: the parent-based Go copies are gone; the child-based copies (pending.go, linktargets.go, the client) remain, plus one the audit missed, the move-sibling visibility check in elementcrud.ts.
Item 3: all three sub-claims confirmed (2.0 → 2, newlines, ASCII names).
Item 4: disagreement confirmed but harmless today.
Item 7: an extra mismatch, publish keys draft links by the default pretty URL while link resolve honours url/slug.
Item 12: generator and loader disagree in both directions; the add-element skill documents the loader's grammar.
Item 14: three facts live only in skills (container: true pairing, image types, 2560px/EXIF).

Decisions (maintainer, 2026-10-12)

Two child issues, not one per item: the split buys a reviewable payload-contract PR separate from the consolidation PR; finer slicing buys nothing with one agent run per issue.
Loader enforces hidden: true ⇔ named as some owner's item (#243).
Save path's rule wins for required params holding their default (#244).
Loader's grammar is the element-name grammar; the generator follows it (#244).

Acceptance criteria

#243 merged.
#244 merged.
No "same as / mirrors / keep in step" comment remains in internal/ or web/src for any rule listed in the original report.

How to verify

Both child PRs merged on main; go test ./... and cd web && npm test green; harness check on the scaffold site passes.

Why this stays with a human

The remaining work is bookkeeping: close this issue when the children land and decide whether anything the children's PRs surfaced (a hash change, a site that fails the name grammar) needs a follow-up.

Out of scope

New audits; a later sweep gets its own issue.

Blocked by

#243
#244

Original report

Follow-up to #230 (one split implemented twice, one copy correct), #232 (its real fix) and #233 (the one-owner rule in CLAUDE.md). A read-only sweep of internal/ and web/src for rules evaluated independently in two or more places, with no shared source of truth and no test tying the copies together. Items marked confirmed were checked in the source by hand; the rest are from the sweep and should be verified before work starts. Line numbers are approximate.

Ranked by drift likelihood × user-visible impact. Each is probably its own change; this issue is the index.

1. Child role: item, nested element or body content — confirmed, copies disagree

internal/l10n/l10n.go isItem, holdsElement; internal/server/translation.go itemAt (third copy of isItem); internal/elemops/save.go PartitionItems, WouldClobberBody: decide by the parent's items.element (plus container, block fallback without schema).
internal/server/pending.go isFoldedElement; internal/server/linktargets.go:108: decide by the child's hidden || group == inline.
Client: web/src/api/content.ts isInlineGroupNode, isHiddenNode, flattenChildren; web/src/elementcard.ts isCardNode/hasSubList: child-based.
The loader (element/load.go validateItems) does not require an item element to be hidden, so one child can be an item for translation and a card for the editor, or the reverse.
Owner: one elemops.ChildRole(parentSchema, child, schemas) used by every server path and serialized into the tree payload so the client stops re-deriving.

2. Slot-wrapper predicate — confirmed, copies disagree

elemops/slots.go SlotWrapperName: block + name slot + non-empty name param.
server/linktargets.go:127: block + name. server/pending.go:414,456: name only, any depth.
Client web/src/elementcrud.ts isSlotWrapper, api/content.ts SLOT_WRAPPER_NAME: block + name.
Differ on a nameless or nested slot. Owner: SlotWrapperName, verdict in the tree payload.

3. Inline chip grammar and param spelling, client vs engine — plausible from code

Client web/src/rte/chip.ts (CHIP_START_RE, PARAM_RE, typedParamValue, chipParamText) vs Go content/serialize.go (paramValueString, quotedSafe, validShortcodeName, validParamName) and Hugo's lexer typing in parse.
2.0 → JS 2 → written back 2, re-lexed as int64: breaks the zero-diff law for floats with a zero fraction (markdown.test.ts covers only 1.5). Client does not refuse newlines in values. Param names ASCII-only on the client, Unicode in Go.
Owner: shared fixture of chip strings and expected typed values, run by go test and vitest.

4. "Value spells its default" — confirmed, copies disagree

elemops/save.go spellsDefault (excludes required params, compares the raw value against Default) vs l10n/hash.go canonicalParam (includes required, Type.Read first, compares against Empty()). A comment claims they spell defaults the same way.
Consequence: a save and the translation hash can disagree on whether a param is "present", flagging translations as changed.
Owner: one element.Param.SpellsEmpty(v).

5. Element summary

elemops/summary.go ElementSummary and web/src/components/ElementForm.tsx rowSummary agree (roles come from the server). web/src/elementcard.ts elementSummary uses body text first, a hard-coded type list, joins with " · ", ignores summary and basename. Cards and the link picker summarize one element differently.
Owner: server computes once, sends per node.

6. Placement rules

Go elemops/placement.go PlacementMsg; client elementcrud.ts placementAllowed and insertRefusal — two client copies, only the latter checks the .inline parent (Go: content/mutate.go checkInsert).
Owner: insertRefusal as the only client copy, backed by a shared fixture.

7. Link destination scanning and classification

Scanning: linkrefs/linkrefs.go nextAnchor, forEachDestination vs server/publish.go markdownLinkRE, hrefLinkRE, normalizeLinkTarget. Classification: server/linktargets.go externalSchemeRE, web/src/components/SetLinkForm.tsx linkTypeOf, theme partials/ce/href.html.
Which params are links: linkrefs uses every string param; elemops/summary.go LinkParamValues (used by publish) only link/page-typed ones. Edges differ: whitespace after ](, non-http schemes, relative paths.
Owner: linkrefs exports Destinations(raw) and Classify(href); publish and the picker reuse them.

8. Front-matter metadata reads

pagetree/build.go lookup duplicates content.MetaLookup; draft/l10n/url/slug parses re-done in pageops/settings.go, pageops/translation_settings.go, pageops/translation_addr.go metaStr, langcmd/content.go. Mode constants exist in pagetree.go and l10n.go. langcmd/content.go:92 hand-builds what l10n.FrontMatterChanges provides. Agree today.
Owner: content/l10n helpers.

9. Shortcode param get/set/sort

Getters/setters in l10n/l10n.go, elemops/uid.go, elemops/save.go, server/forms.go paramLookup, elemops/summary.go; ad hoc name sorts in l10n/hash.go, elemops/create.go, server/forms.go, elemops/save.go. setParam copies the slice, SetShortcodeEnabled filters in place (sc.Params[:0]): aliasing hazard.
Owner: content.Shortcode methods Param, SetParam (sorted), DeleteParam.

10. Reading the disabled flag

server/forms.go shortcodeDisabled, web/src/api/content.ts disabledByParams, theme partials/ce/disabled.html: all accept true or "true"; agree today; single writer elemops/save.go setShortcodeDisabled.
Owner: exported elemops.ShortcodeDisabled plus a fixture shared with client and theme.

11. Language-tag grammar

siteconfig.go, langcmd.go, pagepath.go: documented copies. server/git.go:468 accepts an uppercase primary subtag — already disagrees.
Owner: pagepath.ValidLangTag.

12. Name grammars

Sidecar names: sidecar/sidecar.go (^[a-z0-9-]+$) vs sidecardir.go, repeated in addcmd/children.go. Param names: element/load.go (ASCII) vs content/serialize.go (Unicode).
Owner: sidecardir.ValidName, content.ValidParamName.

13. Loader helpers and unfixtured client mirrors

requireVersion1, parseNameList copied in pagetype/load.go and backendlayout/load.go (documented as identical); editormodule/load.go inlines the version check; element/load.go parseStringList has different nil semantics.
Client mirrors with no shared fixture: pagemanage.ts isValidSlug (pageops/slug.go), mayPlaceUnder (checkChildAllowed); categorypath.ts (categories.SlugOf); rte/allow.ts token list (element/schema.go); the canonical-anchor regex rte/markdown.ts vs elemops/body.go — the latter is the security gate.
Suggestion: the internal/pagepath/testdata pattern (one JSON fixture, both suites).

14. Skill restating facts — confirmed

scaffold/site/.claude/skills/edit-content/SKILL.md:105-133 restates the rte token list, paired vs self-closing (elemops.BlockForSchema), the link grammar and the allowed image types. CLAUDE.md says skills point at AGENTS.md; the drift test cannot catch prose. Replace with pointers.

Lower priority

Page-tree node lookup duplicated in pageops/tree.go, server/modules.go (skips synthesized nodes) and server/linktargets.go (does not).
Text-merge-after-removal in four packages: tracked in #232.

Suggested order

1 and 2 together (same split, same payload change), then 4 (translation hashes), then 3 and the fixture pattern of 13, which gives every later client mirror a home. 14 is a docs-only change that can land any time.

> *This was generated by AI during triage.* ## Summary Tracking index for the drift audit: every rule the engine, server, client or theme evaluates in two or more places gets one owner (CLAUDE.md "Every rule has exactly one owner", #233). The work is sliced into two child issues; this issue closes when both have landed. ## Children - #243 — tree payload: server-owned child role, slot wrapper and summary verdicts (audit items 1, 2, 5). - #244 — one owner per rule: chip grammar, defaults, links, params, names, loaders and client fixtures, plus the four behaviour fixes found on the way (audit items 3, 4, 6-14 and the node lookup). Blocked by #243. ## Verification status (triage, 2026-10-12, at `dcc1067`) Every item was re-checked in the source after #239 landed. Already fixed by #239: `PartitionItems`, `l10n.isItem` and the text-merge copies (`elemops.IsItem` and `content.MergeText` are the owners). Still open: everything else, with these corrections to the audit: - Item 1: the parent-based Go copies are gone; the child-based copies (`pending.go`, `linktargets.go`, the client) remain, plus one the audit missed, the move-sibling visibility check in `elementcrud.ts`. - Item 3: all three sub-claims confirmed (`2.0` → `2`, newlines, ASCII names). - Item 4: disagreement confirmed but harmless today. - Item 7: an extra mismatch, publish keys draft links by the default pretty URL while link resolve honours `url`/`slug`. - Item 12: generator and loader disagree in both directions; the `add-element` skill documents the loader's grammar. - Item 14: three facts live only in skills (`container: true` pairing, image types, 2560px/EXIF). ## Decisions (maintainer, 2026-10-12) - Two child issues, not one per item: the split buys a reviewable payload-contract PR separate from the consolidation PR; finer slicing buys nothing with one agent run per issue. - Loader enforces `hidden: true` ⇔ named as some owner's item (#243). - Save path's rule wins for required params holding their default (#244). - Loader's grammar is the element-name grammar; the generator follows it (#244). ## Acceptance criteria - [x] #243 merged. - [x] #244 merged. - [x] No "same as / mirrors / keep in step" comment remains in `internal/` or `web/src` for any rule listed in the original report. ## How to verify - Both child PRs merged on `main`; `go test ./...` and `cd web && npm test` green; `harness check` on the scaffold site passes. ## Why this stays with a human The remaining work is bookkeeping: close this issue when the children land and decide whether anything the children's PRs surfaced (a hash change, a site that fails the name grammar) needs a follow-up. ## Out of scope - New audits; a later sweep gets its own issue. ## Blocked by - #243 - #244 ## Original report Follow-up to #230 (one split implemented twice, one copy correct), #232 (its real fix) and #233 (the one-owner rule in CLAUDE.md). A read-only sweep of `internal/` and `web/src` for rules evaluated independently in two or more places, with no shared source of truth and no test tying the copies together. Items marked **confirmed** were checked in the source by hand; the rest are from the sweep and should be verified before work starts. Line numbers are approximate. Ranked by drift likelihood × user-visible impact. Each is probably its own change; this issue is the index. ### 1. Child role: item, nested element or body content — confirmed, copies disagree - `internal/l10n/l10n.go` `isItem`, `holdsElement`; `internal/server/translation.go` `itemAt` (third copy of isItem); `internal/elemops/save.go` `PartitionItems`, `WouldClobberBody`: decide by the **parent's** `items.element` (plus `container`, block fallback without schema). - `internal/server/pending.go` `isFoldedElement`; `internal/server/linktargets.go:108`: decide by the **child's** `hidden || group == inline`. - Client: `web/src/api/content.ts` `isInlineGroupNode`, `isHiddenNode`, `flattenChildren`; `web/src/elementcard.ts` `isCardNode`/`hasSubList`: child-based. - The loader (`element/load.go` `validateItems`) does not require an item element to be hidden, so one child can be an item for translation and a card for the editor, or the reverse. - Owner: one `elemops.ChildRole(parentSchema, child, schemas)` used by every server path and serialized into the tree payload so the client stops re-deriving. ### 2. Slot-wrapper predicate — confirmed, copies disagree - `elemops/slots.go` `SlotWrapperName`: block + name `slot` + non-empty `name` param. - `server/linktargets.go:127`: block + name. `server/pending.go:414,456`: name only, any depth. - Client `web/src/elementcrud.ts` `isSlotWrapper`, `api/content.ts` `SLOT_WRAPPER_NAME`: block + name. - Differ on a nameless or nested `slot`. Owner: `SlotWrapperName`, verdict in the tree payload. ### 3. Inline chip grammar and param spelling, client vs engine — plausible from code - Client `web/src/rte/chip.ts` (`CHIP_START_RE`, `PARAM_RE`, `typedParamValue`, `chipParamText`) vs Go `content/serialize.go` (`paramValueString`, `quotedSafe`, `validShortcodeName`, `validParamName`) and Hugo's lexer typing in parse. - `2.0` → JS `2` → written back `2`, re-lexed as int64: breaks the zero-diff law for floats with a zero fraction (`markdown.test.ts` covers only `1.5`). Client does not refuse newlines in values. Param names ASCII-only on the client, Unicode in Go. - Owner: shared fixture of chip strings and expected typed values, run by `go test` and vitest. ### 4. "Value spells its default" — confirmed, copies disagree - `elemops/save.go` `spellsDefault` (excludes required params, compares the raw value against `Default`) vs `l10n/hash.go` `canonicalParam` (includes required, `Type.Read` first, compares against `Empty()`). A comment claims they spell defaults the same way. - Consequence: a save and the translation hash can disagree on whether a param is "present", flagging translations as changed. - Owner: one `element.Param.SpellsEmpty(v)`. ### 5. Element summary - `elemops/summary.go` `ElementSummary` and `web/src/components/ElementForm.tsx` `rowSummary` agree (roles come from the server). `web/src/elementcard.ts` `elementSummary` uses body text first, a hard-coded type list, joins with " · ", ignores `summary` and basename. Cards and the link picker summarize one element differently. - Owner: server computes once, sends per node. ### 6. Placement rules - Go `elemops/placement.go` `PlacementMsg`; client `elementcrud.ts` `placementAllowed` and `insertRefusal` — two client copies, only the latter checks the `.inline` parent (Go: `content/mutate.go` `checkInsert`). - Owner: `insertRefusal` as the only client copy, backed by a shared fixture. ### 7. Link destination scanning and classification - Scanning: `linkrefs/linkrefs.go` `nextAnchor`, `forEachDestination` vs `server/publish.go` `markdownLinkRE`, `hrefLinkRE`, `normalizeLinkTarget`. Classification: `server/linktargets.go` `externalSchemeRE`, `web/src/components/SetLinkForm.tsx` `linkTypeOf`, theme `partials/ce/href.html`. - Which params are links: linkrefs uses every string param; `elemops/summary.go` `LinkParamValues` (used by publish) only link/page-typed ones. Edges differ: whitespace after `](`, non-http schemes, relative paths. - Owner: `linkrefs` exports `Destinations(raw)` and `Classify(href)`; publish and the picker reuse them. ### 8. Front-matter metadata reads - `pagetree/build.go` `lookup` duplicates `content.MetaLookup`; draft/l10n/url/slug parses re-done in `pageops/settings.go`, `pageops/translation_settings.go`, `pageops/translation_addr.go` `metaStr`, `langcmd/content.go`. Mode constants exist in `pagetree.go` and `l10n.go`. `langcmd/content.go:92` hand-builds what `l10n.FrontMatterChanges` provides. Agree today. - Owner: `content`/`l10n` helpers. ### 9. Shortcode param get/set/sort - Getters/setters in `l10n/l10n.go`, `elemops/uid.go`, `elemops/save.go`, `server/forms.go` `paramLookup`, `elemops/summary.go`; ad hoc name sorts in `l10n/hash.go`, `elemops/create.go`, `server/forms.go`, `elemops/save.go`. `setParam` copies the slice, `SetShortcodeEnabled` filters in place (`sc.Params[:0]`): aliasing hazard. - Owner: `content.Shortcode` methods `Param`, `SetParam` (sorted), `DeleteParam`. ### 10. Reading the disabled flag - `server/forms.go` `shortcodeDisabled`, `web/src/api/content.ts` `disabledByParams`, theme `partials/ce/disabled.html`: all accept `true` or `"true"`; agree today; single writer `elemops/save.go` `setShortcodeDisabled`. - Owner: exported `elemops.ShortcodeDisabled` plus a fixture shared with client and theme. ### 11. Language-tag grammar - `siteconfig.go`, `langcmd.go`, `pagepath.go`: documented copies. `server/git.go:468` accepts an uppercase primary subtag — already disagrees. - Owner: `pagepath.ValidLangTag`. ### 12. Name grammars - Sidecar names: `sidecar/sidecar.go` (`^[a-z0-9-]+$`) vs `sidecardir.go`, repeated in `addcmd/children.go`. Param names: `element/load.go` (ASCII) vs `content/serialize.go` (Unicode). - Owner: `sidecardir.ValidName`, `content.ValidParamName`. ### 13. Loader helpers and unfixtured client mirrors - `requireVersion1`, `parseNameList` copied in `pagetype/load.go` and `backendlayout/load.go` (documented as identical); `editormodule/load.go` inlines the version check; `element/load.go` `parseStringList` has different nil semantics. - Client mirrors with no shared fixture: `pagemanage.ts` `isValidSlug` (`pageops/slug.go`), `mayPlaceUnder` (`checkChildAllowed`); `categorypath.ts` (`categories.SlugOf`); `rte/allow.ts` token list (`element/schema.go`); the canonical-anchor regex `rte/markdown.ts` vs `elemops/body.go` — the latter is the security gate. - Suggestion: the `internal/pagepath/testdata` pattern (one JSON fixture, both suites). ### 14. Skill restating facts — confirmed - `scaffold/site/.claude/skills/edit-content/SKILL.md:105-133` restates the rte token list, paired vs self-closing (`elemops.BlockForSchema`), the link grammar and the allowed image types. CLAUDE.md says skills point at AGENTS.md; the drift test cannot catch prose. Replace with pointers. ### Lower priority - Page-tree node lookup duplicated in `pageops/tree.go`, `server/modules.go` (skips synthesized nodes) and `server/linktargets.go` (does not). - Text-merge-after-removal in four packages: tracked in #232. ### Suggested order 1 and 2 together (same split, same payload change), then 4 (translation hashes), then 3 and the fixture pattern of 13, which gives every later client mirror a home. 14 is a docs-only change that can land any time.

dominik.polakovics added the

needs-triage

label

2026-10-12 18:31:54 +02:00

dominik.polakovics added the

enhancement

label

2026-10-12 22:45:33 +02:00

dominik.polakovics referenced this issue

2026-10-12 22:46:34 +02:00

tree payload: server-owned child role, slot wrapper and summary verdicts (drift audit 1, 2, 5) #243

dominik.polakovics referenced this issue