docs(settings): Metadata Settings page + stale route inventory cleanup [shot:settings-shell] (bookshelf-2puu) #1355

Merged

zombor merged 2 commits from bd-bookshelf-2puu into main 2026-08-06 16:06:25 +00:00

Conversation 17 Commits 2 Files changed 5 +193 -6

zombor commented 2026-08-06 01:07:00 +00:00

Owner

Copy link

Summary

• Add docs/content/docs/metadata/metadata-settings.md — a new end-user-facing docs page covering the full Settings → Metadata page: providers, field priority matrix (all 16 fields, P1-P4 slots, enabled/disabled toggles, replace mode, merge-categories), per-library overrides, match weights, and bookdrop merge order. Wired into the metadata/ card grid.
• Update docs/route-inventory.md to reflect that GET /settings/field-priority now permanently redirects to /settings/metadata; add entries for the tabbed shell and the full-matrix PUT /settings/metadata-field-priority.
• Update docs/content/docs/reference/api.md to clarify PUT /settings/field-priority/rating is the legacy single-chain endpoint; PUT /settings/metadata-field-priority is the full-matrix endpoint.
• Add a screenshot capture step to the existing settings-shell browser journey so the Metadata tab (field priority matrix) is posted to this PR via [shot:settings-shell].

The field priority matrix itself was already fully implemented in the settings shell (the DEFERRED bead's core feature shipped in an earlier PR). This PR completes the bead by delivering the required end-user docs and cleaning up stale route inventory references.

Screenshot

The [shot:settings-shell] marker in the PR title triggers the CI browser-e2e job to capture and post a screenshot of the Settings → Metadata tab (field priority matrix) as a PR comment automatically.

Test plan

• make test passes (all unit tests green)
• make coverage passes (zero uncovered statement blocks)
• make e2e-policy-check passes (all Describes are Ordered journey containers)
• go build ./... clean
• go build -tags e2e ./e2e/... clean
• No new e2e/browser journey added (screenshot added to existing Journey: Settings Shell)
• Docs are end-user framing, no source-code/route/internal-field references

Closes bead bookshelf-2puu on merge.

## Summary - Add `docs/content/docs/metadata/metadata-settings.md` — a new end-user-facing docs page covering the full **Settings → Metadata** page: providers, field priority matrix (all 16 fields, P1-P4 slots, enabled/disabled toggles, replace mode, merge-categories), per-library overrides, match weights, and bookdrop merge order. Wired into the `metadata/` card grid. - Update `docs/route-inventory.md` to reflect that `GET /settings/field-priority` now permanently redirects to `/settings/metadata`; add entries for the tabbed shell and the full-matrix `PUT /settings/metadata-field-priority`. - Update `docs/content/docs/reference/api.md` to clarify `PUT /settings/field-priority/rating` is the legacy single-chain endpoint; `PUT /settings/metadata-field-priority` is the full-matrix endpoint. - Add a screenshot capture step to the existing settings-shell browser journey so the Metadata tab (field priority matrix) is posted to this PR via `[shot:settings-shell]`. The field priority matrix itself was already fully implemented in the settings shell (the `DEFERRED` bead's core feature shipped in an earlier PR). This PR completes the bead by delivering the required end-user docs and cleaning up stale route inventory references. ## Screenshot The `[shot:settings-shell]` marker in the PR title triggers the CI browser-e2e job to capture and post a screenshot of the Settings → Metadata tab (field priority matrix) as a PR comment automatically. ## Test plan - [x] `make test` passes (all unit tests green) - [x] `make coverage` passes (zero uncovered statement blocks) - [x] `make e2e-policy-check` passes (all Describes are Ordered journey containers) - [x] `go build ./...` clean - [x] `go build -tags e2e ./e2e/...` clean - [x] No new e2e/browser journey added (screenshot added to existing `Journey: Settings Shell`) - [x] Docs are end-user framing, no source-code/route/internal-field references Closes bead bookshelf-2puu on merge.

settings-ratelimits-7-rows-clean.png

149 KiB

settings-comic-weights-save.png

77 KiB

settings-metadata-field-priority.png

73 KiB

settings-shell-all-tabs.png

59 KiB

settings-ratelimits-7-rows-clean.png

148 KiB

settings-comic-weights-save.png

77 KiB

settings-shell-email-tab.png

70 KiB

settings-metadata-field-priority.png

73 KiB

settings-shell-all-tabs.png

59 KiB

settings-ratelimits-7-rows-clean.png

148 KiB

settings-comic-weights-save.png

77 KiB

settings-shell-email-tab.png

70 KiB

settings-metadata-field-priority.png

73 KiB

settings-shell-all-tabs.png

59 KiB

settings-shell-email-tab.png

70 KiB

zombor added 1 commit 2026-08-06 01:07:00 +00:00

docs(settings): add Metadata Settings docs page + update stale route refs (bookshelf-2puu) ... d763ae5afb

Add docs/content/docs/metadata/metadata-settings.md covering the full
Settings → Metadata page: providers, field priority matrix (all 16 fields,
P1-P4 slots, enabled/disabled toggles, replace mode, merge-categories),
per-library overrides, match weights, and bookdrop merge order. Wire it
into the metadata/ _index.md card grid as the first card.

Update stale docs:
- docs/route-inventory.md: reflect that GET /settings/field-priority now
  permanently redirects to /settings/metadata; add entries for the tabbed
  shell and PUT /settings/metadata-field-priority.
- docs/content/docs/reference/api.md: clarify PUT /settings/field-priority/rating
  is the legacy single-chain endpoint; PUT /settings/metadata-field-priority
  is the full-matrix endpoint.

Add a screenshot capture step to the existing settings-shell browser
journey so the Metadata tab (field priority matrix) is posted to the PR
via [shot:settings-shell].

Closes bead bookshelf-2puu on merge.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

forgejo-actions commented 2026-08-06 01:12:46 +00:00

Copy link

Settings shell screenshot (settings-shell-email-tab)

**Settings shell screenshot** (settings-shell-email-tab) 

forgejo-actions commented 2026-08-06 01:12:52 +00:00

Copy link

Settings shell screenshot (settings-metadata-field-priority)

**Settings shell screenshot** (settings-metadata-field-priority) 

forgejo-actions commented 2026-08-06 01:14:52 +00:00

Copy link

Settings shell screenshot (settings-shell-all-tabs)

**Settings shell screenshot** (settings-shell-all-tabs) 

forgejo-actions commented 2026-08-06 01:14:54 +00:00

Copy link

Settings shell screenshot (settings-ratelimits-7-rows-clean)

**Settings shell screenshot** (settings-ratelimits-7-rows-clean) 

forgejo-actions commented 2026-08-06 01:14:57 +00:00

Copy link

Settings shell screenshot (settings-comic-weights-save)

**Settings shell screenshot** (settings-comic-weights-save) 

zombor commented 2026-08-06 01:20:53 +00:00

Author

Owner

Copy link

Security Review — PR #1355 (bookshelf-2puu)

Scope verification (adversarial check): Diffed all 5 files against origin/main. Confirmed no production Go, template, route, auth, or migration file was smuggled in. The diff is exactly:

• docs/content/docs/metadata/_index.md — index card addition (docs only)
• docs/content/docs/metadata/metadata-settings.md — new end-user doc page (docs only)
• docs/content/docs/reference/api.md — description-text clarifications on two already-documented admin-gated rows
• docs/route-inventory.md — route-coverage table update reflecting absorbed redirects + new routes
• e2e/browser/journey_settings_shell_test.go — 4-line addition: screenshot capture inside an existing Ordered journey It block, errors silently discarded (_ = upload...)

Security checks:

Admin-gate labels in api.md / route-inventory.md: Verified against internal/settings/routes.go lines 58 and 70 — both PUT /settings/field-priority/rating and PUT /settings/metadata-field-priority are wrapped in adminRequired(...). The "admin" access label in the docs accurately reflects the production gate.

Docs content (metadata-settings.md): No unsafe guidance. API key handling is described accurately: keys are masked at display (••••abcd), the "leave field empty to keep stored key unchanged" UX is documented correctly. No advice to disable a security control, no URLs or credentials, no internal symbol references.

e2e test addition: The 4 added lines insert a screenshot-upload call inside an existing It block inside an Ordered journey. The call is fully guarded (if ssErr == nil) and the return value discarded (_ = ...), so a failure cannot affect the test outcome. The uploadSettingsScreenshotToPR helper is defined earlier in the same file (line 30) and was already present on origin/main. Package declaration is package browser_test (black-box). Build tag is //go:build e2e. No new unexported symbols exposed.

No security surface exists in this diff.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Security Review — PR #1355 (bookshelf-2puu) **Scope verification (adversarial check):** Diffed all 5 files against `origin/main`. Confirmed no production Go, template, route, auth, or migration file was smuggled in. The diff is exactly: - `docs/content/docs/metadata/_index.md` — index card addition (docs only) - `docs/content/docs/metadata/metadata-settings.md` — new end-user doc page (docs only) - `docs/content/docs/reference/api.md` — description-text clarifications on two already-documented admin-gated rows - `docs/route-inventory.md` — route-coverage table update reflecting absorbed redirects + new routes - `e2e/browser/journey_settings_shell_test.go` — 4-line addition: screenshot capture inside an existing `Ordered` journey `It` block, errors silently discarded (`_ = upload...`) **Security checks:** **Admin-gate labels in api.md / route-inventory.md:** Verified against `internal/settings/routes.go` lines 58 and 70 — both `PUT /settings/field-priority/rating` and `PUT /settings/metadata-field-priority` are wrapped in `adminRequired(...)`. The "admin" access label in the docs accurately reflects the production gate. **Docs content (metadata-settings.md):** No unsafe guidance. API key handling is described accurately: keys are masked at display (`••••abcd`), the "leave field empty to keep stored key unchanged" UX is documented correctly. No advice to disable a security control, no URLs or credentials, no internal symbol references. **e2e test addition:** The 4 added lines insert a screenshot-upload call inside an existing `It` block inside an `Ordered` journey. The call is fully guarded (`if ssErr == nil`) and the return value discarded (`_ = ...`), so a failure cannot affect the test outcome. The `uploadSettingsScreenshotToPR` helper is defined earlier in the same file (line 30) and was already present on `origin/main`. Package declaration is `package browser_test` (black-box). Build tag is `//go:build e2e`. No new unexported symbols exposed. No security surface exists in this diff. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-08-06 01:21:56 +00:00

Author

Owner

Copy link

Code review of PR #1355 (bead bookshelf-2puu) — diff-only; CI is green and is the source of behavioral truth.

---

[MAJOR] docs/content/docs/metadata/metadata-settings.md:49 — API key claim excludes Google Books, Open Library, Audnexus, and AniList

The doc states "If the provider requires an API key (Hardcover, ComicVine, Metron), paste your key into the API Key field." This is wrong on two counts:

1. Metron uses HTTP Basic auth (username + password), not an API key. The template renders a Username+Password pair for AuthTypeBasic providers (shell template ~line 462), not an API key field. Telling users to "paste your key" for Metron will confuse them.
2. Google Books, Open Library, Audnexus, and AniList are all AuthTypeKey providers in providerEntries (provider_settings.go:85–91). All four render an API key field in the UI. The list omits all of them.

The correct statement should describe that all providers except Open Library and AniList require credentials, and that Metron specifically asks for a username and password rather than an API key. At minimum, "Hardcover, ComicVine, Metron" must be expanded and the username/password distinction for Metron noted.

Fix: update the parenthetical to enumerate providers that require credentials accurately, and note the Metron username/password distinction.

---

[MAJOR] docs/content/docs/metadata/metadata-settings.md:80 — match weight range "0 to 10" is wrong for comic fields

The doc states "Each weight is a number from 0 to 10." Comic-specific weight fields accept 0–15 (MaxWeight = 15 in internal/settings/match_weights.go:138). The UI also renders the comic weights section as part of the same Match Weights panel on the Metadata tab. A user setting comic weights above 10 would correctly be accepted by the server but contradicts the doc.

Fix: clarify that book/ebook fields accept 0–10 and comic-specific fields accept 0–15, or at minimum say "0 to 10 for standard fields" with a note that comic-specific fields have a higher ceiling.

---

[MINOR] docs/content/docs/metadata/metadata-settings.md:1 — "Bookdrop Merge Order" listed as a Metadata tab section but is part of the Metadata tab's own content

The table of contents at the top of the doc lists "Bookdrop Merge Order — which provider wins during BookDrop import" as a separate bulleted section, but the page has no corresponding ## Bookdrop Merge Order heading — the topic is addressed in the _index.md card linking to metadata-providers. If Bookdrop Merge Order is truly surfaced on the Metadata tab (it is — MetaBookdropMergeOrder is loaded in loadMetadataTabData), it should have its own section, or the bullet should be removed from the intro list. As-is the doc promises a section that doesn't exist.

Fix: either add a ## Bookdrop Merge Order section describing the drag-to-reorder UI, or remove the bullet from the intro list.

---

[MINOR] docs/route-inventory.md:135 — redirect coverage claim for GET /settings/metadata-providers mischaracterises the redirect path

The new inventory entry says GET /settings/metadata-providers "redirects to /settings/metadata". The actual route handler wraps the redirect with adminRequired (routes.go:~line 48), so unauthenticated users get a 401/403 before ever seeing the redirect, not a 301. The note is fine for documentation purposes but the word "Permanent redirect" in the Notes column implies the redirect is unconditional; the auth gate is invisible. Same applies to the GET /settings/match-weights and GET /settings/metadata-field-priority redirect rows. Not a correctness blocker for the inventory doc but worth noting.

Fix: add "(auth-gated)" to the Notes for those three redirect rows to match what the code actually does.

---

[MINOR] e2e/browser/journey_settings_shell_test.go:144 — screenshot capture silently discards upload errors

The new screenshot block does _ = uploadSettingsScreenshotToPR(...) — discarding the return value. The existing usages at lines 121, 189, 262, 310 all do the same, so this is consistent with the file's convention. However, if the upload fails (e.g., Forgejo API is unavailable in a CI run), there is no log or warning, making PR screenshot failures invisible. This is a pre-existing pattern, not introduced by this PR, but it is worth noting.

Fix (optional): consider logging a warning on upload failure. Out of scope for this PR — suggest filing a follow-up bead.

---

REVIEW VERDICT: 0 blocker, 2 major, 3 minor

Code review of PR #1355 (bead bookshelf-2puu) — diff-only; CI is green and is the source of behavioral truth. --- [MAJOR] docs/content/docs/metadata/metadata-settings.md:49 — API key claim excludes Google Books, Open Library, Audnexus, and AniList The doc states "If the provider requires an API key (Hardcover, ComicVine, Metron), paste your key into the **API Key** field." This is wrong on two counts: 1. **Metron uses HTTP Basic auth (username + password), not an API key.** The template renders a Username+Password pair for `AuthTypeBasic` providers (shell template ~line 462), not an API key field. Telling users to "paste your key" for Metron will confuse them. 2. **Google Books, Open Library, Audnexus, and AniList are all `AuthTypeKey` providers** in `providerEntries` (provider_settings.go:85–91). All four render an API key field in the UI. The list omits all of them. The correct statement should describe that _all_ providers except Open Library and AniList require credentials, and that Metron specifically asks for a username and password rather than an API key. At minimum, "Hardcover, ComicVine, Metron" must be expanded and the username/password distinction for Metron noted. Fix: update the parenthetical to enumerate providers that require credentials accurately, and note the Metron username/password distinction. --- [MAJOR] docs/content/docs/metadata/metadata-settings.md:80 — match weight range "0 to 10" is wrong for comic fields The doc states "Each weight is a number from 0 to 10." Comic-specific weight fields accept 0–15 (`MaxWeight = 15` in `internal/settings/match_weights.go:138`). The UI also renders the comic weights section as part of the same Match Weights panel on the Metadata tab. A user setting comic weights above 10 would correctly be accepted by the server but contradicts the doc. Fix: clarify that book/ebook fields accept 0–10 and comic-specific fields accept 0–15, or at minimum say "0 to 10 for standard fields" with a note that comic-specific fields have a higher ceiling. --- [MINOR] docs/content/docs/metadata/metadata-settings.md:1 — "Bookdrop Merge Order" listed as a Metadata tab section but is part of the Metadata tab's own content The table of contents at the top of the doc lists "**Bookdrop Merge Order** — which provider wins during BookDrop import" as a separate bulleted section, but the page has no corresponding `## Bookdrop Merge Order` heading — the topic is addressed in the `_index.md` card linking to `metadata-providers`. If Bookdrop Merge Order is truly surfaced on the Metadata tab (it is — `MetaBookdropMergeOrder` is loaded in `loadMetadataTabData`), it should have its own section, or the bullet should be removed from the intro list. As-is the doc promises a section that doesn't exist. Fix: either add a `## Bookdrop Merge Order` section describing the drag-to-reorder UI, or remove the bullet from the intro list. --- [MINOR] docs/route-inventory.md:135 — redirect coverage claim for GET /settings/metadata-providers mischaracterises the redirect path The new inventory entry says `GET /settings/metadata-providers` "redirects to /settings/metadata". The actual route handler wraps the redirect with `adminRequired` (routes.go:~line 48), so unauthenticated users get a 401/403 before ever seeing the redirect, not a 301. The note is fine for documentation purposes but the word "Permanent redirect" in the Notes column implies the redirect is unconditional; the auth gate is invisible. Same applies to the GET /settings/match-weights and GET /settings/metadata-field-priority redirect rows. Not a correctness blocker for the inventory doc but worth noting. Fix: add "(auth-gated)" to the Notes for those three redirect rows to match what the code actually does. --- [MINOR] e2e/browser/journey_settings_shell_test.go:144 — screenshot capture silently discards upload errors The new screenshot block does `_ = uploadSettingsScreenshotToPR(...)` — discarding the return value. The existing usages at lines 121, 189, 262, 310 all do the same, so this is consistent with the file's convention. However, if the upload fails (e.g., Forgejo API is unavailable in a CI run), there is no log or warning, making PR screenshot failures invisible. This is a pre-existing pattern, not introduced by this PR, but it is worth noting. Fix (optional): consider logging a warning on upload failure. Out of scope for this PR — suggest filing a follow-up bead. --- REVIEW VERDICT: 0 blocker, 2 major, 3 minor

zombor added 1 commit 2026-08-06 01:26:10 +00:00

fix(docs): correct provider auth types + match-weight ranges + add merge-order section (2puu review) ... 83fca6798f

- Metron uses HTTP Basic auth (username+password), not an API key;
  rewrite step-3 to branch key-providers vs Metron
- Add all key-based providers to the list (Google Books, Open Library,
  Hardcover, ComicVine, Audnexus, AniList)
- Match-weight range: book fields 0-10, comic fields 0-15 (verified
  against MaxBookWeight=10 / MaxWeight=15 in match_weights.go)
- Add Bookdrop Merge Order section (drag-to-reorder precedence for
  BookDrop import; backed by MetaBookdropMergeOrder in settings_shell.html)
- route-inventory.md: add missing GET /settings/metadata-field-priority
  and GET /settings/match-weights redirect rows; mark all three
  adminRequired redirects as (auth-gated)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

forgejo-actions commented 2026-08-06 01:44:20 +00:00

Copy link

Settings shell screenshot (settings-shell-email-tab)

**Settings shell screenshot** (settings-shell-email-tab) 

forgejo-actions commented 2026-08-06 01:44:27 +00:00

Copy link

Settings shell screenshot (settings-metadata-field-priority)

**Settings shell screenshot** (settings-metadata-field-priority) 

forgejo-actions commented 2026-08-06 01:46:28 +00:00

Copy link

Settings shell screenshot (settings-shell-all-tabs)

**Settings shell screenshot** (settings-shell-all-tabs) 

forgejo-actions commented 2026-08-06 01:46:30 +00:00

Copy link

Settings shell screenshot (settings-ratelimits-7-rows-clean)

**Settings shell screenshot** (settings-ratelimits-7-rows-clean) 

forgejo-actions commented 2026-08-06 01:46:34 +00:00

Copy link

Settings shell screenshot (settings-comic-weights-save)

**Settings shell screenshot** (settings-comic-weights-save) 

zombor referenced this pull request 2026-08-06 02:19:57 +00:00

fix(e2e): bz643.3 — downgrade server-HTML browser specs to API+Vitest tier (bookshelf-bz643.3) #1357

zombor force-pushed bd-bookshelf-2puu from 83fca6798f to c113cfef22 2026-08-06 15:58:33 +00:00 Compare

forgejo-actions commented 2026-08-06 16:02:41 +00:00

Copy link

Settings shell screenshot (settings-shell-email-tab)

**Settings shell screenshot** (settings-shell-email-tab) 

forgejo-actions commented 2026-08-06 16:02:47 +00:00

Copy link

Settings shell screenshot (settings-metadata-field-priority)

**Settings shell screenshot** (settings-metadata-field-priority) 

forgejo-actions commented 2026-08-06 16:04:48 +00:00

Copy link

Settings shell screenshot (settings-shell-all-tabs)

**Settings shell screenshot** (settings-shell-all-tabs) 

forgejo-actions commented 2026-08-06 16:04:51 +00:00

Copy link

Settings shell screenshot (settings-ratelimits-7-rows-clean)

**Settings shell screenshot** (settings-ratelimits-7-rows-clean) 

forgejo-actions commented 2026-08-06 16:04:54 +00:00

Copy link

Settings shell screenshot (settings-comic-weights-save)

**Settings shell screenshot** (settings-comic-weights-save) 

zombor merged commit f5daddc219 into main 2026-08-06 16:06:25 +00:00

zombor referenced this pull request from a commit 2026-08-06 16:06:26 +00:00

Merge pull request 'docs(settings): Metadata Settings page + stale route inventory cleanup [shot:settings-shell] (bookshelf-2puu)' (#1355) from bd-bookshelf-2puu into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

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

zombor/pergamum!1355

No description provided.