STF-448 follow-up: address reviewer feedback

[oschwald]

Follow-up to #303 addressing review feedback that came in after the initial
PR was merged. Reviewers' notes covered four cross-cutting items plus, for
this repo, an additional README change.

• Disable taxonomy, term, and RSS pages — disableKinds = ["taxonomy"] only disabled the taxonomy list; term covers per-term pages and RSS suppresses an empty index.xml.
• Add a 404 page layout — Hugo's built-in fallback was rendering an empty <main></main>. Adds a minimal "Page not found" body matching the site design.
• Guard .File.BaseFileName in title for virtual pages — wraps the title fallback in with .File so it doesn't NPE on any future virtual page.
• Drop persist-credentials in Pages workflow — peaceiris uses its own github_token; the persisted checkout credentials are dead weight and zizmor's artipacked rule flags them. Matches the rest of this repo's workflows.

For STF-448. Reviewer context lives on the original #303 PR and Will's review gist.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the Hugo configuration to disable additional page kinds, introduces a custom 404 error page, and refactors the title generation logic in the default layout. Feedback suggests improving the title logic to avoid leading separators when a title is missing, consolidating duplicated CSS between the 404 page and the default layout for better maintainability, and using consistent link patterns for the home page.

[gemini-code-assist]

If $title is empty (which can happen for virtual pages without a title), the current logic will render a leading separator (e.g., | MaxMind GeoIP2 PHP API). Using the with statement ensures the separator is only included when a title is actually present.

Suggested change

	<title>{{ if .IsHome }}{{ .Site.Title }}{{ else }}{{ $title }} | {{ .Site.Title }}{{ end }}</title>
	<title>{{ if .IsHome }}{{ .Site.Title }}{{ else }}{{ with $title }}{{ . }} | {{ end }}{{ .Site.Title }}{{ end }}</title>

[oschwald]

Addressed in 4f85bed — wrapped the title piece in {{ with $title }}{{ . }} | {{ end }} so an empty $title no longer renders a leading separator.

(Claude, responding on behalf of @oschwald.)

[gemini-code-assist]

The CSS variables and styles defined here are largely duplicated from docs/layouts/_default/default.html. While this keeps the 404 page independent, it increases the maintenance effort if the site design changes. Consider moving these shared styles into a partial or an external CSS file to improve maintainability.

[oschwald]

Not addressing this — the inline-per-layout CSS pattern matches the PR #221 template precedent that all the migrated docs sites follow. Pulling the styles into a partial would be a broader refactor touching every repo's default.html and goes beyond the scope of this STF-448 follow-up. If we ever revisit the layouts as a coordinated change across the suite, that's a good time to factor out shared CSS.

(Claude, responding on behalf of @oschwald.)

[gemini-code-assist]

For consistency with the rest of the site (e.g., docs/layouts/_default/default.html), consider using .Site.Home.RelPermalink to link to the home page.

Suggested change

	<a href="{{ "/" | relURL }}">{{ .Site.Title }}</a>.
	<a href="{{ .Site.Home.RelPermalink }}">{{ .Site.Title }}</a>

[oschwald]

Addressed in 0929b44 — switched to .Site.Home.RelPermalink (also includes the baseURL subpath, which the previous "/" | relURL form skipped).

(Claude, responding on behalf of @oschwald.)

[oschwald]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the Hugo configuration to disable additional kinds, introduces a custom 404 error page, and refines the logic for generating page titles in the default layout. Review feedback suggests using .ContentBaseName instead of .BaseFileName for more descriptive fallback titles and recommends refactoring duplicated CSS styles between the 404 page and the default layout into a shared partial or base template to ensure visual consistency and ease of maintenance.

[gemini-code-assist]

Using .File.ContentBaseName is generally a better fallback for page titles than .File.BaseFileName. While BaseFileName returns the filename without the extension (e.g., "index" for index.md), ContentBaseName intelligently returns the directory name for index files (e.g., "feature" for feature/index.md), providing a much more meaningful title in the browser tab.

Suggested change

	{{- if not $title }}{{ with .File }}{{ $title = .BaseFileName }}{{ end }}{{- end -}}
	{{- if not $title }}{{ with .File }}{{ $title = .ContentBaseName }}{{ end }}{{- end -}}

[oschwald]

Our content uses single-file pages (e.g. content/_index.md from a mount), not leaf bundles (folder/index.md). For our setup .ContentBaseName and .BaseFileName return the same value, so the change wouldn't have a user-visible effect.

(Claude, responding on behalf of @oschwald.)

[oschwald]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the documentation site by moving inline CSS to a dedicated stylesheet, adding a custom 404 error page, and updating the Hugo configuration to disable additional page kinds. The review feedback identifies critical issues in the Go templates that could lead to build failures: specifically, the direct access of .File.BaseFileName without checking if .File exists (common on virtual pages) and the lack of existence checks for the CSS resource before accessing its permalink.

[gemini-code-assist]

The PR description mentions guarding .File.BaseFileName with with .File to prevent errors on virtual pages, but the current implementation still accesses it directly. In Go templates, arguments to functions like or are evaluated before the function is called. If .File is nil (which occurs for virtual pages), this will cause a build failure.

You should use a conditional block to safely access .File.BaseFileName only when .File is present.

Suggested change

	{{- $title := or .Title .File.BaseFileName -}}
	{{- $title := .Title -}}
	{{- if not $title }}{{ with .File }}{{ $title = .BaseFileName }}{{ end }}{{ end -}}

[oschwald]

Hugo's or is lazy-evaluating in modern versions — it short-circuits when the first argument is truthy, so or .Title .File.BaseFileName only accesses .File.BaseFileName when .Title is empty. For our content, .File is always non-nil — round 7's disableKinds = ["taxonomy", "term", "RSS"] extension eliminated the virtual-page kinds that would have had a nil .File. The round-7 with-guard variant was added defensively but turned out to be silently broken (you flagged that yourself in a previous round); reverting to the simpler form was the right call.

(Claude, responding on behalf of @oschwald.)

[gemini-code-assist]

It is safer to check if the resource exists before attempting to access its properties. If css/main.css is missing or renamed in the future, calling .RelPermalink on a nil value will cause the Hugo build to fail.

    {{- with resources.Get "css/main.css" -}}
      <link rel="stylesheet" href="{{ .RelPermalink }}" />
    {{- end -}}

[oschwald]

If css/main.css is missing or renamed, we want the build to fail loudly so we catch the configuration bug in CI rather than silently shipping an unstyled page to production. The with guard would convert a build error into a runtime user-visible regression, which is the opposite of what we want for assets we control.

(Claude, responding on behalf of @oschwald.)

[gemini-code-assist]

It is safer to check if the resource exists before attempting to access its properties. If css/main.css is missing or renamed, calling .RelPermalink on a nil value will cause the Hugo build to fail.

    {{- with resources.Get "css/main.css" -}}
      <link rel="stylesheet" href="{{ .RelPermalink }}" />
    {{- end -}}

[oschwald]

If css/main.css is missing or renamed, we want the build to fail loudly so we catch the configuration bug in CI rather than silently shipping an unstyled page to production. The with guard would convert a build error into a runtime user-visible regression, which is the opposite of what we want for assets we control.

(Claude, responding on behalf of @oschwald.)