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

Merged

dominik.polakovics merged 1 commit from afk/114 into main

2026-07-19 00:06:33 +02:00

dominik.polakovics commented

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

Owner

Closes #114

The directory-scan contract shared by element.LoadDir, pagetype.LoadDir, and backendlayout.LoadDir — one <name>.yaml|.yml per file, .yaml beating .yml in sorted filename order, the shared ^[A-Za-z0-9_][A-Za-z0-9_-]*$ base-name rule, per-file LoadError isolation, missing dir = empty + no error, subdirs/other extensions ignored, results sorted by Name/File, never-nil slices — was re-implemented in all three packages. It is now defined once in the new internal/sidecardir package (sidecar was taken by the shortcode-scaffold generator).

Design

sidecardir.Load[S](dir, Config[S]) owns the scan skeleton. Each family supplies via Config: the error-message noun ("element", "page type", "backend layout"), its Decode (the existing loadOne, minus the now-shared name-regex check), and optionally a Reject hook.
Load returns []Loaded[S] pairing each schema with its source filename — element's cross-schema pass (rte allowedInline / items.element resolution) uses the pairing to report errors against the right file, unchanged.
Each family's LoadError is now a type alias of sidecardir.LoadError: JSON keys file/error, pointer-receiver Error() = "<file>: <msg>", so the /api/... error payload shape is untouched.
pagetype's retired _root pre-check became the Reject hook, which runs before duplicate tracking and does not claim the name — so both _root.yaml and _root.yml still get the obsolete-message error, never "duplicate".
Ordering edges preserved exactly: a file whose name is invalid or whose decode fails still claims its base name (so a same-named sibling reports "duplicate"); the name check still runs before the file is read; all error strings are byte-identical.

Verification

All existing tests in the three packages pass unmodified (no *_test.go outside internal/sidecardir/ touched).
internal/sidecardir has its own 12-test suite: missing dir, dir-read error (via a regular file as dir — root-proof, no chmod), subdir/extension skipping, .yaml-beats-.yml, duplicate-after-failed-decode, invalid-name claiming, reject-hook non-claiming, per-file isolation, name-vs-filename sort order (a/a-b), errs sorted by File, Loaded.File pairing, Error() format.
Behavior-preserving, so scaffold/site/AGENTS.md needs no edit; agentsdoc_test.go passes.
Full go test ./... green (with web/dist built and hugo on PATH so the hugo-gated server/theme/supervisor tests actually ran); go vet ./... clean; gofmt clean on all touched files.

Out of scope, per the issue: deduplicating parseNameList/requireVersion1 between pagetype and backendlayout (kept as-is to avoid widening the diff).

🤖 Generated with Claude Code

Closes #114 The directory-scan contract shared by `element.LoadDir`, `pagetype.LoadDir`, and `backendlayout.LoadDir` — one `<name>.yaml|.yml` per file, `.yaml` beating `.yml` in sorted filename order, the shared `^[A-Za-z0-9_][A-Za-z0-9_-]*$` base-name rule, per-file `LoadError` isolation, missing dir = empty + no error, subdirs/other extensions ignored, results sorted by Name/File, never-nil slices — was re-implemented in all three packages. It is now defined once in the new `internal/sidecardir` package (`sidecar` was taken by the shortcode-scaffold generator). ## Design - `sidecardir.Load[S](dir, Config[S])` owns the scan skeleton. Each family supplies via `Config`: the error-message noun ("element", "page type", "backend layout"), its `Decode` (the existing `loadOne`, minus the now-shared name-regex check), and optionally a `Reject` hook. - `Load` returns `[]Loaded[S]` pairing each schema with its source filename — element's cross-schema pass (rte `allowedInline` / `items.element` resolution) uses the pairing to report errors against the right file, unchanged. - Each family's `LoadError` is now a type alias of `sidecardir.LoadError`: JSON keys `file`/`error`, pointer-receiver `Error()` = `"<file>: <msg>"`, so the `/api/...` error payload shape is untouched. - `pagetype`'s retired `_root` pre-check became the `Reject` hook, which runs **before** duplicate tracking and does not claim the name — so both `_root.yaml` and `_root.yml` still get the obsolete-message error, never "duplicate". - Ordering edges preserved exactly: a file whose name is invalid or whose decode fails still claims its base name (so a same-named sibling reports "duplicate"); the name check still runs before the file is read; all error strings are byte-identical. ## Verification - All existing tests in the three packages pass **unmodified** (no `*_test.go` outside `internal/sidecardir/` touched). - `internal/sidecardir` has its own 12-test suite: missing dir, dir-read error (via a regular file as dir — root-proof, no chmod), subdir/extension skipping, `.yaml`-beats-`.yml`, duplicate-after-failed-decode, invalid-name claiming, reject-hook non-claiming, per-file isolation, name-vs-filename sort order (`a`/`a-b`), errs sorted by File, `Loaded.File` pairing, `Error()` format. - Behavior-preserving, so `scaffold/site/AGENTS.md` needs no edit; `agentsdoc_test.go` passes. - Full `go test ./...` green (with `web/dist` built and hugo on PATH so the hugo-gated server/theme/supervisor tests actually ran); `go vet ./...` clean; gofmt clean on all touched files. Out of scope, per the issue: deduplicating `parseNameList`/`requireVersion1` between pagetype and backendlayout (kept as-is to avoid widening the diff). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 1 commit

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

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

ci / build-and-test (pull_request) Successful in 8m22s

Details

28a0be554a

The directory-scan contract — one <name>.yaml|.yml per file, .yaml beating
.yml in sorted order, the shared base-name rule, per-file LoadError
isolation, missing dir = empty + no error, sorted never-nil results — was
re-implemented in element, pagetype, and backendlayout. It is now defined
once in internal/sidecardir; each family supplies only its error-message
noun, its decode+validate function, and (for pagetype's retired _root
declaration) a reject hook that runs before duplicate tracking.

Each family's LoadError becomes a type alias of sidecardir.LoadError, so
the HTTP error contract (json keys file/error, "<file>: <msg>" rendering)
is unchanged. Load returns schemas paired with their source filename, which
element's cross-schema pass uses to report dangling references against the
right file. Observable behavior is preserved exactly: the existing tests in
all three packages pass unmodified, and the ordering edges — a file that
fails to decode or has an invalid name still claims its base name, while a
rejected _root never does — are pinned by the new package's own test suite.

Closes #114

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

dominik.polakovics commented

2026-07-19 00:05:39 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing validation — PASS

Verification signal relied on: Forgejo Actions ci / build-and-test (pull_request) — success (8m22s, run 109). Not re-run locally; it covers go test ./..., go vet, gofmt and the agentsdoc_test.go drift test.

Checked:

Conventions — title is Conventional Commits (refactor:); body carries a working Closes #114; head is afk/114, matching the AFK contract.
Mergeable — git merge-tree against origin/main is conflict-free.
Scope — 8 files, +532/−241; no *_test.go outside internal/sidecardir/ touched, so the three families' existing suites pin the contract unmodified, exactly as the issue required.
Behaviour preservation, read against the pre-image:
- Check ordering is byte-identical per family. Old pagetype: _root → duplicate → claim → loadOne(name regex → read). New: Reject → duplicate → claim → nameRe → Decode. Old element/backendlayout: duplicate → claim → loadOne(name regex → read). New: same, minus the hook.
- The two ordering edges called out in the issue both survive and are pinned by new tests: a file that fails the name check or fails decode still claims its base name (TestLoadInvalidNameStillClaimsIt, TestLoadDuplicateReportedEvenWhenFirstFileFailsToDecode), while a Rejected _root does not (TestLoadRejectHookDoesNotClaimName).
- Error strings unchanged — the family noun is threaded through Config.Family and the regex string comes from the same shared pattern.
- LoadError is now a type alias, so the /api/... JSON keys (file/error) and the pointer-receiver Error() = "<file>: <msg>" are literally the same type.
- The one substantive swap, verified: sorting moved from schemas[i].Name to baseName(items[i].File). These agree only if Schema.Name is always the derived base name — confirmed in all three loadOnes (element L167, pagetype L124, backendlayout L97 all build &Schema{Name: name}; none reads a name: key from YAML). Element's dropped post-pass sort.Slice is likewise safe: items arrives name-sorted and the cross-schema pass only filters.
- Never-nil slices preserved on every path, including the dir-read-error path (make([]Schema, 0, len(items))).
Acceptance criteria — all met: no package retains its own os.ReadDir/filter/sort/dup loop; the new package ships a focused 12-test suite covering every case the issue listed; AGENTS.md correctly needs no edit.

Findings: none blocking. No nits worth holding the merge for.

Verdict: PASS — awaiting the human's free-text merge go-ahead.

> *This was generated by AI while landing a PR.* ## Landing validation — PASS **Verification signal relied on:** Forgejo Actions `ci / build-and-test (pull_request)` — **success** (8m22s, run 109). Not re-run locally; it covers `go test ./...`, `go vet`, gofmt and the `agentsdoc_test.go` drift test. **Checked:** - **Conventions** — title is Conventional Commits (`refactor:`); body carries a working `Closes #114`; head is `afk/114`, matching the AFK contract. - **Mergeable** — `git merge-tree` against `origin/main` is conflict-free. - **Scope** — 8 files, +532/−241; no `*_test.go` outside `internal/sidecardir/` touched, so the three families' existing suites pin the contract unmodified, exactly as the issue required. - **Behaviour preservation, read against the pre-image:** - Check ordering is byte-identical per family. Old pagetype: `_root` → duplicate → claim → `loadOne`(name regex → read). New: `Reject` → duplicate → claim → `nameRe` → `Decode`. Old element/backendlayout: duplicate → claim → `loadOne`(name regex → read). New: same, minus the hook. - The two ordering edges called out in the issue both survive and are pinned by new tests: a file that fails the name check or fails decode still claims its base name (`TestLoadInvalidNameStillClaimsIt`, `TestLoadDuplicateReportedEvenWhenFirstFileFailsToDecode`), while a `Reject`ed `_root` does not (`TestLoadRejectHookDoesNotClaimName`). - Error strings unchanged — the family noun is threaded through `Config.Family` and the regex string comes from the same shared pattern. - `LoadError` is now a type alias, so the `/api/...` JSON keys (`file`/`error`) and the pointer-receiver `Error()` = `"<file>: <msg>"` are literally the same type. - **The one substantive swap, verified:** sorting moved from `schemas[i].Name` to `baseName(items[i].File)`. These agree only if `Schema.Name` is always the derived base name — confirmed in all three `loadOne`s (`element` L167, `pagetype` L124, `backendlayout` L97 all build `&Schema{Name: name}`; none reads a `name:` key from YAML). Element's dropped post-pass `sort.Slice` is likewise safe: `items` arrives name-sorted and the cross-schema pass only filters. - Never-nil slices preserved on every path, including the dir-read-error path (`make([]Schema, 0, len(items))`). - **Acceptance criteria** — all met: no package retains its own `os.ReadDir`/filter/sort/dup loop; the new package ships a focused 12-test suite covering every case the issue listed; `AGENTS.md` correctly needs no edit. **Findings:** none blocking. No nits worth holding the merge for. **Verdict: PASS** — awaiting the human's free-text merge go-ahead.

dominik.polakovics merged commit 4daeb18696 into main

2026-07-19 00:06:33 +02:00

dominik.polakovics referenced this pull request from a commit

2026-07-19 00:06:34 +02:00

Merge pull request 'refactor: extract shared sidecar directory-scan skeleton into internal/sidecardir' (#121) from afk/114 into main