Define effective page type once (currently three byte-for-byte mirrors) #115

Closed
opened 2026-07-18 14:19:51 +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/pageops/pagetypes.go (effectiveTypeOf/impliedType), internal/pagetree/build.go:384-413 (comments: "mirrors pageops.effectiveTypeOf", "mirrors pageops.impliedType byte for byte"), plus the frontend mirror effectiveNodeType.

Problem: Hugo's explicit-front-matter-type-else-section-name-fallback rule exists three times because pageops imports pagetree and not the reverse — an import cycle fossilized as duplication the comments openly apologize for. pageops.EffectiveType is itself a one-line pass-through wrapper existing only to export the private function to server.

Solution: move the rule into a module both Go packages can import (pagetree itself, or a tiny leaf package). The frontend copy stays (it cannot import Go) but gets a conformance test against shared fixtures.

Benefits: Locality — a change to type resolution (new page-type feature, Hugo behaviour change) is one edit, not three synchronized ones. Small, low-risk, high-confidence extraction.

> *This was generated by AI during triage.* Found by an architecture review (deepening opportunities — turning shallow modules into deep ones). **Files**: `internal/pageops/pagetypes.go` (`effectiveTypeOf`/`impliedType`), `internal/pagetree/build.go:384-413` (comments: "mirrors pageops.effectiveTypeOf", "mirrors pageops.impliedType byte for byte"), plus the frontend mirror `effectiveNodeType`. **Problem**: Hugo's explicit-front-matter-`type`-else-section-name-fallback rule exists three times because `pageops` imports `pagetree` and not the reverse — an import cycle fossilized as duplication the comments openly apologize for. `pageops.EffectiveType` is itself a one-line pass-through wrapper existing only to export the private function to `server`. **Solution**: move the rule into a module both Go packages can import (`pagetree` itself, or a tiny leaf package). The frontend copy stays (it cannot import Go) but gets a conformance test against shared fixtures. **Benefits**: *Locality* — a change to type resolution (new page-type feature, Hugo behaviour change) is one edit, not three synchronized ones. Small, low-risk, high-confidence extraction.
Author
Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Define Hugo's effective-page-type rule once in a new tiny leaf Go package; delete the two duplicated Go implementations; conformance-test the frontend mirror against shared fixtures.

Current behavior:
The rule "a page's effective type is its explicit front-matter type when non-empty, else Hugo's section-name fallback" is implemented three times:

  1. internal/pageops — private effectiveTypeOf (reads the page file's front matter itself) plus impliedType; the exported pageops.EffectiveType(root, page string) string is a one-line pass-through that the HTTP server layer calls.
  2. internal/pagetree — private effectiveType(pagePath, explicitType string) (pure — the tree builder already has the parsed front matter) plus its own impliedType, whose comments admit "mirrors pageops.impliedType byte for byte; kept local because pageops imports pagetree, not the reverse".
  3. The web client — effectiveNodeType / impliedPageType in the page-management module, unit-tested but with no link guaranteeing it matches the Go rule.

The fallback semantics all three encode: the page's top-level section name — posts/first.mdposts, posts/_index.mdposts; a leaf bundle's own directory does not count (posts/foo/index.mdposts, foo/index.md at the root → ""); root-level pages (including the content root _index.md) → "".

Desired behavior:

  • One new small leaf package (maintainer decision: a new package, not an export from pagetree) owns the pure rule: an implied-type function of the page path, and an effective-type function of (path, explicit type). It should depend on the standard library only, so anything may import it. Pick a short descriptive name (e.g. something like pagepath); document the semantics above, including the leaf-bundle and content-root cases.
  • internal/pageops and internal/pagetree both delegate to it; their private duplicates and the "mirrors … byte for byte" apology comments disappear. pageops keeps only its thin file-reading wrapper (read front matter, take explicit type if non-empty string, else fall back), and pageops.EffectiveType keeps its exported signature and behavior — the server call sites should not need changes.
  • The frontend copy stays (it cannot import Go) but gains a conformance test: a shared fixture file of cases (path, explicit type, expected effective type) that BOTH a Go test in the new package AND a web unit test consume, so a divergence fails a test instead of hiding in a comment. Put the fixtures wherever both test runners can read the same file (e.g. the new package's testdata, read from the web test via a relative path). Cover at least: explicit type set, explicit empty, nested section page, _index.md section page, leaf bundle in a section, leaf bundle at root, root-level page, content-root _index.md.
  • No behavior change anywhere — this is a pure consolidation.

Key interfaces:

  • New leaf package: pure functions, stdlib-only imports.
  • pageops.EffectiveType(root, page string) string — must remain exported with identical behavior (HTTP layer depends on it).
  • internal/pagetype deliberately does NOT compute this fallback (its package doc says so) — do not move the rule there or change that boundary.

Acceptance criteria:

  • The implied-type/effective-type logic has exactly one Go implementation; no copy remains in pageops or pagetree.
  • The new package imports only the standard library.
  • A shared fixture file exists and is consumed by both a Go conformance test and a web conformance test; both pass, and editing an expected value in the fixtures makes both fail.
  • go test ./... and the web test suite pass with no behavior change.
  • The "mirrors … byte for byte" comments are gone.

Out of scope:

  • Changing internal/pagetype's no-resolution boundary.
  • Replacing the frontend implementation with server-provided resolution.
  • scaffold/site/AGENTS.md — nothing it documents changes here.
  • Issues #116 and #117 (separate consolidations from the same review).
> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Define Hugo's effective-page-type rule once in a new tiny leaf Go package; delete the two duplicated Go implementations; conformance-test the frontend mirror against shared fixtures. **Current behavior:** The rule "a page's effective type is its explicit front-matter `type` when non-empty, else Hugo's section-name fallback" is implemented three times: 1. `internal/pageops` — private `effectiveTypeOf` (reads the page file's front matter itself) plus `impliedType`; the exported `pageops.EffectiveType(root, page string) string` is a one-line pass-through that the HTTP server layer calls. 2. `internal/pagetree` — private `effectiveType(pagePath, explicitType string)` (pure — the tree builder already has the parsed front matter) plus its own `impliedType`, whose comments admit "mirrors pageops.impliedType byte for byte; kept local because pageops imports pagetree, not the reverse". 3. The web client — `effectiveNodeType` / `impliedPageType` in the page-management module, unit-tested but with no link guaranteeing it matches the Go rule. The fallback semantics all three encode: the page's top-level section name — `posts/first.md` → `posts`, `posts/_index.md` → `posts`; a leaf bundle's own directory does not count (`posts/foo/index.md` → `posts`, `foo/index.md` at the root → `""`); root-level pages (including the content root `_index.md`) → `""`. **Desired behavior:** - One new small leaf package (maintainer decision: a new package, not an export from `pagetree`) owns the pure rule: an implied-type function of the page path, and an effective-type function of (path, explicit type). It should depend on the standard library only, so anything may import it. Pick a short descriptive name (e.g. something like `pagepath`); document the semantics above, including the leaf-bundle and content-root cases. - `internal/pageops` and `internal/pagetree` both delegate to it; their private duplicates and the "mirrors … byte for byte" apology comments disappear. `pageops` keeps only its thin file-reading wrapper (read front matter, take explicit `type` if non-empty string, else fall back), and `pageops.EffectiveType` keeps its exported signature and behavior — the server call sites should not need changes. - The frontend copy stays (it cannot import Go) but gains a conformance test: a shared fixture file of cases (path, explicit type, expected effective type) that BOTH a Go test in the new package AND a web unit test consume, so a divergence fails a test instead of hiding in a comment. Put the fixtures wherever both test runners can read the same file (e.g. the new package's testdata, read from the web test via a relative path). Cover at least: explicit type set, explicit empty, nested section page, `_index.md` section page, leaf bundle in a section, leaf bundle at root, root-level page, content-root `_index.md`. - No behavior change anywhere — this is a pure consolidation. **Key interfaces:** - New leaf package: pure functions, stdlib-only imports. - `pageops.EffectiveType(root, page string) string` — must remain exported with identical behavior (HTTP layer depends on it). - `internal/pagetype` deliberately does NOT compute this fallback (its package doc says so) — do not move the rule there or change that boundary. **Acceptance criteria:** - [ ] The implied-type/effective-type logic has exactly one Go implementation; no copy remains in `pageops` or `pagetree`. - [ ] The new package imports only the standard library. - [ ] A shared fixture file exists and is consumed by both a Go conformance test and a web conformance test; both pass, and editing an expected value in the fixtures makes both fail. - [ ] `go test ./...` and the web test suite pass with no behavior change. - [ ] The "mirrors … byte for byte" comments are gone. **Out of scope:** - Changing `internal/pagetype`'s no-resolution boundary. - Replacing the frontend implementation with server-provided resolution. - `scaffold/site/AGENTS.md` — nothing it documents changes here. - Issues #116 and #117 (separate consolidations from the same review).
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#115
No description provided.