Deepen the element mutation engine: move shortcode-shaping semantics out of internal/server #111

Closed
opened 2026-07-18 14:19:07 +02:00 by dominik.polakovics · 1 comment

This was generated by AI during triage.

Found by an architecture review (deepening opportunities — turning shallow modules into deep ones).

Files: internal/server/server.go (handleSavePage, ~190 lines), internal/server/element.go, internal/server/uid.go, internal/server/slots.go, internal/server/body.go; contrast internal/content/mutate.go, internal/element, internal/pageops/settings.go.

Problem: internal/content exposes only structural mutation (InsertChild/RemoveChild/MoveChild), so all semantic element shaping — uid stamping, the slot wrapper, item-shortcode composition, param merging, the reserved-param vocabulary — lives as ~20 free functions inside internal/server that hand-manipulate content.Shortcode internals (newShortcode, mergeParams, partitionItems, buildItemShortcodes, stampUIDs, setShortcodeDisabled, coalesceText, …). The validate → coerce → apply save contract is written twice (server.handleSavePage and pageops.SaveSettings, including the <link>Target companion set/delete dance in both). The four-way child-composition switch in handleSavePage (server.go:771-790) is only testable by POSTing JSON at an httptest server over a temp site.

Solution: deepen a single element-tree module (a neighbor of content/element, not inside the server) whose interface is roughly "apply this validated edit to this element" — absorbing uid stamping, slot wrapping, item composition, and the save contract. handleSavePage shrinks to parse request → validate → apply → write; pageops.SaveSettings calls the same module. The duplicated front-matter map readers (server.metaString vs pageops.metaLookup) fold into the same cleanup.

Benefits: Locality — the on-disk shape of an element is currently understood only by crossing content ↔ element ↔ server three times; it would live in one place. Leverage — the server and pageops become thin callers. Tests move from HTTP-over-tempdir to direct through-the-interface tests on the composition logic, which is where the real bug surface lives (index-path staleness across RemoveChild text-merge, the four-axis placement interaction).

> *This was generated by AI during triage.* Found by an architecture review (deepening opportunities — turning shallow modules into deep ones). **Files**: `internal/server/server.go` (`handleSavePage`, ~190 lines), `internal/server/element.go`, `internal/server/uid.go`, `internal/server/slots.go`, `internal/server/body.go`; contrast `internal/content/mutate.go`, `internal/element`, `internal/pageops/settings.go`. **Problem**: `internal/content` exposes only *structural* mutation (`InsertChild`/`RemoveChild`/`MoveChild`), so all *semantic* element shaping — uid stamping, the `slot` wrapper, item-shortcode composition, param merging, the reserved-param vocabulary — lives as ~20 free functions inside `internal/server` that hand-manipulate `content.Shortcode` internals (`newShortcode`, `mergeParams`, `partitionItems`, `buildItemShortcodes`, `stampUIDs`, `setShortcodeDisabled`, `coalesceText`, …). The validate → coerce → apply save contract is written twice (`server.handleSavePage` and `pageops.SaveSettings`, including the `<link>Target` companion set/delete dance in both). The four-way child-composition switch in `handleSavePage` (server.go:771-790) is only testable by POSTing JSON at an `httptest` server over a temp site. **Solution**: deepen a single element-tree module (a neighbor of `content`/`element`, not inside the server) whose interface is roughly "apply this validated edit to this element" — absorbing uid stamping, slot wrapping, item composition, and the save contract. `handleSavePage` shrinks to parse request → validate → apply → write; `pageops.SaveSettings` calls the same module. The duplicated front-matter map readers (`server.metaString` vs `pageops.metaLookup`) fold into the same cleanup. **Benefits**: *Locality* — the on-disk shape of an element is currently understood only by crossing content ↔ element ↔ server three times; it would live in one place. *Leverage* — the server and `pageops` become thin callers. Tests move from HTTP-over-tempdir to direct through-the-interface tests on the composition logic, which is where the real bug surface lives (index-path staleness across `RemoveChild` text-merge, the four-axis placement interaction).
Author
Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Extract element-tree semantic mutation — uid stamping, slot wrapping, item-shortcode composition, param merging, the validate→coerce→apply save contract — out of the HTTP server package into a dedicated module that both the server and pageops call.

Current behavior:
internal/content exposes only structural mutation (InsertChild / RemoveChild / MoveChild). Everything semantic about shaping an element lives as free functions inside the HTTP server package (internal/server): newShortcode, mergeParams, partitionItems, buildItemShortcodes, stampUIDs/restampUIDs, setShortcodeDisabled, coalesceText, and the slot-wrapper family (ensureSlotWrapper, topLevelSlot, slotIsEmpty, …). The page-save handler contains a four-way items/body child-composition switch with documented byte-for-byte compatibility promises (legacy self-closing elements upgrade to paired tags on any items save; interleaved newline text nodes are coalesced when items are extracted; a body-capable element is canonically a block even with an empty body). The validate → coerce → apply contract is written twice — once in the element save handler, once in pageops.SaveSettings — including the <name>Target link-companion set/delete dance in both (see element.LinkTargetSuffix). Two near-identical front-matter map readers exist (metaString in the server, metaLookup in pageops). The composition logic is only testable by POSTing JSON at an httptest server over a temp site.

Desired behavior:
A single element-mutation module — a neighbor of content and element, not inside the server package — owns semantic element shaping behind an interface of roughly "apply this validated edit to this element". It absorbs uid stamping, slot wrapping, item composition, param merging, and the shared save contract. The HTTP save handler shrinks to parse request → validate → apply → write; pageops.SaveSettings calls the same module for the shared parts. This is a behavior-preserving refactor: on-disk output must stay byte-for-byte identical, including every branch of the four-way composition switch and the <name>Target companion semantics. Move first, mechanically; do not "improve" those semantics.

Key interfaces:

  • New package (sibling of internal/content / internal/element) exposing the semantic mutation API; internal/server and internal/pageops become thin callers.
  • content.Shortcode internals should stop being hand-manipulated from the server package; the new module is the one place that knows an element's on-disk shape.
  • The shared validate → coerce → apply contract, including element.LinkTargetSuffix companion set/delete, defined once and called from both element save and pageops.SaveSettings.
  • One front-matter lookup helper replacing the metaString / metaLookup twins.

Acceptance criteria:

  • The four-way items/body composition rules, uid stamping, slot wrapping, and param merging are covered by direct unit tests through the new module's interface — no HTTP server needed.
  • The HTTP element-save handler and pageops.SaveSettings contain no duplicated coerce/apply or <name>Target companion logic; both delegate to the shared module.
  • No semantic shaping free functions remain in the server package (structural HTTP concerns — routing, status codes, request parsing — stay).
  • All existing tests pass unchanged (go test ./...), including content roundtrip/fidelity/fuzz tests and the existing HTTP-level tests — they are the byte-for-byte compatibility net.
  • scaffold/site/AGENTS.md is checked for drift; content canonicalization rules it documents must still be accurate (a pure move should not change them).

Out of scope:

  • Any change to on-disk shortcode format, canonicalization, or save semantics.
  • Any change to the HTTP API surface (routes, request/response shapes, error messages).
  • The atomic-write/rollback engine (#116), sidecar-loader consolidation (#114), effective-page-type unification (#115) — separate issues.
  • Web frontend changes.
> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Extract element-tree *semantic* mutation — uid stamping, slot wrapping, item-shortcode composition, param merging, the validate→coerce→apply save contract — out of the HTTP server package into a dedicated module that both the server and `pageops` call. **Current behavior:** `internal/content` exposes only structural mutation (`InsertChild` / `RemoveChild` / `MoveChild`). Everything semantic about shaping an element lives as free functions inside the HTTP server package (`internal/server`): `newShortcode`, `mergeParams`, `partitionItems`, `buildItemShortcodes`, `stampUIDs`/`restampUIDs`, `setShortcodeDisabled`, `coalesceText`, and the slot-wrapper family (`ensureSlotWrapper`, `topLevelSlot`, `slotIsEmpty`, …). The page-save handler contains a four-way items/body child-composition switch with documented byte-for-byte compatibility promises (legacy self-closing elements upgrade to paired tags on any items save; interleaved newline text nodes are coalesced when items are extracted; a body-capable element is canonically a block even with an empty body). The validate → coerce → apply contract is written twice — once in the element save handler, once in `pageops.SaveSettings` — including the `<name>Target` link-companion set/delete dance in both (see `element.LinkTargetSuffix`). Two near-identical front-matter map readers exist (`metaString` in the server, `metaLookup` in `pageops`). The composition logic is only testable by POSTing JSON at an `httptest` server over a temp site. **Desired behavior:** A single element-mutation module — a neighbor of `content` and `element`, **not** inside the server package — owns semantic element shaping behind an interface of roughly "apply this validated edit to this element". It absorbs uid stamping, slot wrapping, item composition, param merging, and the shared save contract. The HTTP save handler shrinks to parse request → validate → apply → write; `pageops.SaveSettings` calls the same module for the shared parts. This is a **behavior-preserving refactor**: on-disk output must stay byte-for-byte identical, including every branch of the four-way composition switch and the `<name>Target` companion semantics. Move first, mechanically; do not "improve" those semantics. **Key interfaces:** - New package (sibling of `internal/content` / `internal/element`) exposing the semantic mutation API; `internal/server` and `internal/pageops` become thin callers. - `content.Shortcode` internals should stop being hand-manipulated from the server package; the new module is the one place that knows an element's on-disk shape. - The shared validate → coerce → apply contract, including `element.LinkTargetSuffix` companion set/delete, defined once and called from both element save and `pageops.SaveSettings`. - One front-matter lookup helper replacing the `metaString` / `metaLookup` twins. **Acceptance criteria:** - [ ] The four-way items/body composition rules, uid stamping, slot wrapping, and param merging are covered by direct unit tests through the new module's interface — no HTTP server needed. - [ ] The HTTP element-save handler and `pageops.SaveSettings` contain no duplicated coerce/apply or `<name>Target` companion logic; both delegate to the shared module. - [ ] No semantic shaping free functions remain in the server package (structural HTTP concerns — routing, status codes, request parsing — stay). - [ ] All existing tests pass unchanged (`go test ./...`), including content roundtrip/fidelity/fuzz tests and the existing HTTP-level tests — they are the byte-for-byte compatibility net. - [ ] `scaffold/site/AGENTS.md` is checked for drift; content canonicalization rules it documents must still be accurate (a pure move should not change them). **Out of scope:** - Any change to on-disk shortcode format, canonicalization, or save semantics. - Any change to the HTTP API surface (routes, request/response shapes, error messages). - The atomic-write/rollback engine (#116), sidecar-loader consolidation (#114), effective-page-type unification (#115) — separate issues. - Web frontend changes.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Cloonar/deckle#111
No description provided.