Cloonar/deckle

Fork 0

One generic sidecar-loader skeleton behind element/pagetype/backendlayout LoadDir #114

New issue

Closed

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

dominik.polakovics commented

2026-07-18 14:19:51 +02:00

Owner

This was generated by AI during triage.

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

Files: internal/element, internal/pagetype, internal/backendlayout (each with its own LoadDir), plus the media/pagetype loaders.

Problem: the directory-scan contract — one <name>.yaml|.yml per file, .yaml wins over .yml, the ^[A-Za-z0-9_]… name regex, per-file LoadError, subdirs/other extensions ignored, missing dir = empty + no error — is re-implemented per package with near-identical doc.go prose. (The param vocabulary was correctly deduped via element.ParseParams; the directory-scan/dup-resolution/strict-decode skeleton was not.)

Solution: a generic sidecar-loading module the three packages call with their decode function; each keeps its own schema types and validation.

Benefits: Locality — the loading contract (which scaffold/site/AGENTS.md documents and internal/element/agentsdoc_test.go mechanically guards) is defined once. A behaviour fix lands in one place instead of four, and the contract gets one thorough test suite instead of four partial ones. Note: any observable change to sidecar loading must update scaffold/site/AGENTS.md in the same change (CLAUDE.md rule) — a pure dedup should be behaviour-preserving.

> *This was generated by AI during triage.* Found by an architecture review (deepening opportunities — turning shallow modules into deep ones). **Files**: `internal/element`, `internal/pagetype`, `internal/backendlayout` (each with its own `LoadDir`), plus the media/pagetype loaders. **Problem**: the directory-scan contract — one `<name>.yaml|.yml` per file, `.yaml` wins over `.yml`, the `^[A-Za-z0-9_]…` name regex, per-file `LoadError`, subdirs/other extensions ignored, missing dir = empty + no error — is re-implemented per package with near-identical `doc.go` prose. (The *param vocabulary* was correctly deduped via `element.ParseParams`; the directory-scan/dup-resolution/strict-decode skeleton was not.) **Solution**: a generic sidecar-loading module the three packages call with their decode function; each keeps its own schema types and validation. **Benefits**: *Locality* — the loading contract (which `scaffold/site/AGENTS.md` documents and `internal/element/agentsdoc_test.go` mechanically guards) is defined once. A behaviour fix lands in one place instead of four, and the contract gets one thorough test suite instead of four partial ones. Note: any observable change to sidecar loading must update `scaffold/site/AGENTS.md` in the same change (CLAUDE.md rule) — a pure dedup should be behaviour-preserving.

dominik.polakovics added the

needs-triage

enhancement

labels

2026-07-18 14:19:51 +02:00

dominik.polakovics referenced this issue

2026-07-18 14:27:41 +02:00

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

dominik.polakovics referenced this issue

2026-07-18 15:16:22 +02:00

refactor: deepen the element mutation engine into internal/elemops #118

dominik.polakovics referenced this issue

2026-07-18 15:28:00 +02:00

refactor: deepen the element mutation engine into internal/elemops #118

dominik.polakovics commented

2026-07-18 20:37:27 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Extract the shared sidecar directory-scan skeleton behind element.LoadDir, pagetype.LoadDir, and backendlayout.LoadDir into one generic module; each package keeps its own schema types and validation.

Current behavior:
Three packages — internal/element, internal/pagetype, internal/backendlayout — each implement their own LoadDir(dir string) ([]Schema, []LoadError) with a near-identical skeleton: os.ReadDir with missing-dir → (empty, empty) and any other read error → a single LoadError naming the dir; subdirectories and non-.yaml/.yml entries skipped; filenames sorted; name derived by trimming the extension; duplicate base names resolved first-seen-wins in sorted order (so x.yaml loads and x.yml becomes a "duplicate name" LoadError); the shared ^[A-Za-z0-9_][A-Za-z0-9_-]*$ name rule; per-file LoadError isolation (one bad file never hides the rest); schemas sorted by Name, errors by File; both returned slices never nil. Each package also carries its own structurally identical LoadError{File, Msg} type with the same json:"file"/json:"error" tags, and its own doc prose restating the same contract.

Desired behavior:
The directory-scan / duplicate-resolution / per-file-error skeleton is defined once in a generic module (likely type-parameterized), which the three packages call with their per-family pieces: a family noun for error messages ("element", "page type", "backend layout"), and a decode+validate function for one file's contents. Each package keeps its own schema types, raw decode targets, and validation logic unchanged. All observable behavior — return values, error messages, orderings, JSON shapes — is preserved exactly.

Note: the package name sidecar is already taken by the shortcode-scaffold generator (internal/sidecar), so the new module needs a different name.

Key interfaces:

The three LoadDir(dir string) ([]Schema, []LoadError) signatures must not change — they are called by the server layer and their LoadError JSON (file/error keys) is an HTTP contract.
Each package's LoadError must keep marshalling to the same JSON. Whether the three types become aliases of one shared type or stay per-package wrappers is the implementer's call, as long as the wire shape and Error() string (": ") are unchanged.
internal/element runs a cross-schema pass after per-file loading (rte allowedInline and items.element resolution) that needs each loaded schema paired with its source filename to report errors against the right file — the generic module must expose that pairing.
internal/pagetype has a pre-check that rejects the retired _root base name with a fixed explanatory message. Watch the ordering edge: today every _root.* file gets the obsolete-message error (it never enters duplicate tracking), and separately a file whose decode fails still claims its base name, so a same-named sibling reports "duplicate" — both orderings must survive the extraction.
The duplicate-name, invalid-name, and dir-read error message strings embed the family noun; they must come out byte-identical.

Acceptance criteria:

One generic module owns the scan skeleton; none of the three packages still contains its own os.ReadDir/extension-filter/sort/duplicate-tracking loop.
All existing tests in the three packages pass unmodified — they pin the contract this refactor must preserve.
The generic module has its own focused test suite covering: missing dir, unreadable dir, subdir/other-extension skipping, .yaml-beats-.yml duplicate resolution, duplicate reported even when the first-seen file failed to decode, invalid-name rejection, per-file error isolation, output sorting, never-nil slices.
The /api/... error payload shape for bad sidecars is unchanged (LoadError JSON keys file/error).
scaffold/site/AGENTS.md needs no edit (behaviour-preserving) and internal/element/agentsdoc_test.go still passes; go test ./... is green.

Out of scope:

Any observable behavior change to sidecar loading (which would trigger the AGENTS.md update rule).
Deduplicating parseNameList/requireVersion1 (duplicated between pagetype and backendlayout) — reasonable follow-up, but not required here; include only if it falls out naturally in the same module without widening the diff.
The internal/sidecar shortcode-scaffold package and the media loaders — no directory-scan duplication lives there.
Changing the loading contract itself (name rules, extension policy, error wording).

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Extract the shared sidecar directory-scan skeleton behind `element.LoadDir`, `pagetype.LoadDir`, and `backendlayout.LoadDir` into one generic module; each package keeps its own schema types and validation. **Current behavior:** Three packages — `internal/element`, `internal/pagetype`, `internal/backendlayout` — each implement their own `LoadDir(dir string) ([]Schema, []LoadError)` with a near-identical skeleton: `os.ReadDir` with missing-dir → `(empty, empty)` and any other read error → a single `LoadError` naming the dir; subdirectories and non-`.yaml`/`.yml` entries skipped; filenames sorted; name derived by trimming the extension; duplicate base names resolved first-seen-wins in sorted order (so `x.yaml` loads and `x.yml` becomes a "duplicate <family> name" `LoadError`); the shared `^[A-Za-z0-9_][A-Za-z0-9_-]*$` name rule; per-file `LoadError` isolation (one bad file never hides the rest); schemas sorted by `Name`, errors by `File`; both returned slices never nil. Each package also carries its own structurally identical `LoadError{File, Msg}` type with the same `json:"file"`/`json:"error"` tags, and its own doc prose restating the same contract. **Desired behavior:** The directory-scan / duplicate-resolution / per-file-error skeleton is defined once in a generic module (likely type-parameterized), which the three packages call with their per-family pieces: a family noun for error messages ("element", "page type", "backend layout"), and a decode+validate function for one file's contents. Each package keeps its own schema types, raw decode targets, and validation logic unchanged. All observable behavior — return values, error messages, orderings, JSON shapes — is preserved exactly. Note: the package name `sidecar` is already taken by the shortcode-scaffold generator (`internal/sidecar`), so the new module needs a different name. **Key interfaces:** - The three `LoadDir(dir string) ([]Schema, []LoadError)` signatures must not change — they are called by the server layer and their `LoadError` JSON (`file`/`error` keys) is an HTTP contract. - Each package's `LoadError` must keep marshalling to the same JSON. Whether the three types become aliases of one shared type or stay per-package wrappers is the implementer's call, as long as the wire shape and `Error()` string ("<file>: <msg>") are unchanged. - `internal/element` runs a **cross-schema pass** after per-file loading (rte `allowedInline` and `items.element` resolution) that needs each loaded schema paired with its source filename to report errors against the right file — the generic module must expose that pairing. - `internal/pagetype` has a pre-check that rejects the retired `_root` base name with a fixed explanatory message. Watch the ordering edge: today every `_root.*` file gets the obsolete-message error (it never enters duplicate tracking), and separately a file whose *decode* fails still claims its base name, so a same-named sibling reports "duplicate" — both orderings must survive the extraction. - The duplicate-name, invalid-name, and dir-read error message strings embed the family noun; they must come out byte-identical. **Acceptance criteria:** - [ ] One generic module owns the scan skeleton; none of the three packages still contains its own `os.ReadDir`/extension-filter/sort/duplicate-tracking loop. - [ ] All existing tests in the three packages pass **unmodified** — they pin the contract this refactor must preserve. - [ ] The generic module has its own focused test suite covering: missing dir, unreadable dir, subdir/other-extension skipping, `.yaml`-beats-`.yml` duplicate resolution, duplicate reported even when the first-seen file failed to decode, invalid-name rejection, per-file error isolation, output sorting, never-nil slices. - [ ] The `/api/...` error payload shape for bad sidecars is unchanged (LoadError JSON keys `file`/`error`). - [ ] `scaffold/site/AGENTS.md` needs no edit (behaviour-preserving) and `internal/element/agentsdoc_test.go` still passes; `go test ./...` is green. **Out of scope:** - Any observable behavior change to sidecar loading (which would trigger the AGENTS.md update rule). - Deduplicating `parseNameList`/`requireVersion1` (duplicated between `pagetype` and `backendlayout`) — reasonable follow-up, but not required here; include only if it falls out naturally in the same module without widening the diff. - The `internal/sidecar` shortcode-scaffold package and the media loaders — no directory-scan duplication lives there. - Changing the loading contract itself (name rules, extension policy, error wording).

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-07-18 20:37:38 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-18 20:54:59 +02:00

refactor: extract shared sidecar directory-scan skeleton into internal/sidecardir

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-18 20:55:36 +02:00

refactor: extract shared sidecar directory-scan skeleton into internal/sidecardir #121