forgejo_admin/pal-e-api

Fork 1

Enhance browse landing page as documentation hub #15

Merged

forgejo_admin merged 1 commit from 13-landing-page-docs-hub into main

2026-02-25 08:03:15 +00:00

forgejo_admin commented

2026-02-25 07:56:36 +00:00

Owner

Summary

New landing page at /browse/ with architecture diagram, quick-start links, and stats
Repos listing page at /browse/repos with platform/status display
Validate repo URLs with HttpUrl, add enum for platform/status fields
Consolidate landing page queries for projects, repos, SOPs, conventions, templates

plan-2026-02-24-docs-foundation — Phase 4 docs complete, this ships the landing page from earlier phases
note-conventions — follows HTML style guide

Test plan

Visit /browse/ — landing page renders with architecture diagram, project cards, SOP links
Visit /browse/repos — repos listed with platform badges and status
Verify nav links work between landing, notes, projects, repos

🤖 Generated with Claude Code

## Summary - New landing page at `/browse/` with architecture diagram, quick-start links, and stats - Repos listing page at `/browse/repos` with platform/status display - Validate repo URLs with HttpUrl, add enum for platform/status fields - Consolidate landing page queries for projects, repos, SOPs, conventions, templates ## Related Notes - `plan-2026-02-24-docs-foundation` — Phase 4 docs complete, this ships the landing page from earlier phases - `note-conventions` — follows HTML style guide ## Test plan - [ ] Visit `/browse/` — landing page renders with architecture diagram, project cards, SOP links - [ ] Visit `/browse/repos` — repos listed with platform badges and status - [ ] Verify nav links work between landing, notes, projects, repos 🤖 Generated with [Claude Code](https://claude.com/claude-code)

forgejo_admin commented

2026-02-25 07:58:51 +00:00

Author

Owner

PR Review — #15 Enhance browse landing page as documentation hub

Reviewer: Devy (agent)
Plan: plan-2026-02-24-docs-foundation

BLOCKERS

None. No blocking issues found.

NITS (should fix)

1. Missing `| urlencode` on tag URLs in `landing.html`

Every other template (note.html, tags.html, project_notes.html, tag_notes.html) uses {{ tag.name | urlencode }} in tag hrefs. The landing page omits it in all 4 documentation sections:

<!-- landing.html lines 95, 109, 123, 137 -->
<a href="/browse/tags/{{ tag.name }}" class="tag">{{ tag.name }}</a>

<!-- Should be: -->
<a href="/browse/tags/{{ tag.name | urlencode }}" class="tag">{{ tag.name }}</a>

Tag names with spaces or special characters would produce broken URLs. This is a consistency issue with the existing codebase pattern. 4 occurrences to fix.

2. `Repo` model missing from `tests/conftest.py` imports

conftest.py imports all models so SQLAlchemy creates the tables during tests. Repo is absent:

# conftest.py line 13-20 — missing Repo import
from pal_e_docs.models import (
    Note, NoteLink, NoteRevision, NoteTag, Project, Tag,
)
# Should include: Repo

Not a runtime bug today (no tests exercise repos/frontend routes), but will cause failures when someone adds repo tests. Worth fixing while touching this code.

3. Empty state handling in `landing.html`

The Projects and Repos sections on the landing page render empty <div class="card-grid"></div> containers when there are no items. The projects.html template handles this gracefully with {% else %} on the for loop:

{% else %}
<li class="meta">No projects yet.</li>

Consider adding similar empty-state messages for the Projects and Repos card grids, or wrapping them in {% if projects %} / {% if repos %} blocks like the Documentation subsections already do.

OBSERVATIONS (informational, no action required)

mode="json" fix in repos.py is correct and necessary. Verified that HttpUrl objects serialize to Url type without mode="json", which would fail when passed to SQLAlchemy's String column. Good catch in the review-fix commit.
Literal types for platform/status are a good improvement. The Literal["github", "forgejo"] and Literal["active", "archived"] constraints prevent garbage data at the API boundary. The DB column stays String which is fine — validation at the schema layer is the right place.
Query consolidation in the landing route is clean. Single query for doc notes with Tag.name.in_(doc_tags) then Python-side filtering avoids N+1 and keeps it to 3 queries total (projects, repos, doc_notes). The _has_tag helper is readable.
XSS surface is safe. New templates use only {{ variable }} (auto-escaped by Jinja2). No | safe filters. Repo URLs come from the DB (stored as plain strings from validated HttpUrl input), and Jinja2 auto-escapes them in href attributes. The badge CSS classes use {{ repo.platform }} and {{ repo.status }} which are constrained to Literal values at creation time.
Route reorganization is clean. Moving projects list from /browse/ to /browse/projects and adding the landing page at /browse/ is a sensible restructure. All internal template links already point to /browse/projects/{{ slug }} so no broken links.
Mermaid diagram is hardcoded. The architecture diagram in landing.html is static HTML. This is fine for now — it's a landing page, not generated content. When the architecture changes, someone will need to update this template. Consider noting this in CLAUDE.md or the project page so it doesn't go stale.

VERDICT

Approve with nits. Fix the 4 missing | urlencode filters in landing.html (nit #1), then this is good to merge. Nits #2 and #3 are nice-to-have and can be deferred.

## PR Review — #15 Enhance browse landing page as documentation hub **Reviewer:** Devy (agent) **Plan:** `plan-2026-02-24-docs-foundation` --- ### BLOCKERS **None.** No blocking issues found. --- ### NITS (should fix) #### 1. Missing `| urlencode` on tag URLs in `landing.html` Every other template (`note.html`, `tags.html`, `project_notes.html`, `tag_notes.html`) uses `{{ tag.name | urlencode }}` in tag hrefs. The landing page omits it in all 4 documentation sections: ```html  <a href="/browse/tags/{{ tag.name }}" class="tag">{{ tag.name }}</a>  <a href="/browse/tags/{{ tag.name | urlencode }}" class="tag">{{ tag.name }}</a> ``` Tag names with spaces or special characters would produce broken URLs. This is a consistency issue with the existing codebase pattern. **4 occurrences to fix.** #### 2. `Repo` model missing from `tests/conftest.py` imports `conftest.py` imports all models so SQLAlchemy creates the tables during tests. `Repo` is absent: ```python # conftest.py line 13-20 — missing Repo import from pal_e_docs.models import ( Note, NoteLink, NoteRevision, NoteTag, Project, Tag, ) # Should include: Repo ``` Not a runtime bug today (no tests exercise repos/frontend routes), but will cause failures when someone adds repo tests. Worth fixing while touching this code. #### 3. Empty state handling in `landing.html` The Projects and Repos sections on the landing page render empty `<div class="card-grid"></div>` containers when there are no items. The `projects.html` template handles this gracefully with `{% else %}` on the for loop: ```html {% else %} <li class="meta">No projects yet.</li> ``` Consider adding similar empty-state messages for the Projects and Repos card grids, or wrapping them in `{% if projects %}` / `{% if repos %}` blocks like the Documentation subsections already do. --- ### OBSERVATIONS (informational, no action required) 1. **`mode="json"` fix in `repos.py` is correct and necessary.** Verified that `HttpUrl` objects serialize to `Url` type without `mode="json"`, which would fail when passed to SQLAlchemy's `String` column. Good catch in the review-fix commit. 2. **Literal types for platform/status are a good improvement.** The `Literal["github", "forgejo"]` and `Literal["active", "archived"]` constraints prevent garbage data at the API boundary. The DB column stays `String` which is fine — validation at the schema layer is the right place. 3. **Query consolidation in the landing route is clean.** Single query for doc notes with `Tag.name.in_(doc_tags)` then Python-side filtering avoids N+1 and keeps it to 3 queries total (projects, repos, doc_notes). The `_has_tag` helper is readable. 4. **XSS surface is safe.** New templates use only `{{ variable }}` (auto-escaped by Jinja2). No `| safe` filters. Repo URLs come from the DB (stored as plain strings from validated `HttpUrl` input), and Jinja2 auto-escapes them in href attributes. The badge CSS classes use `{{ repo.platform }}` and `{{ repo.status }}` which are constrained to Literal values at creation time. 5. **Route reorganization is clean.** Moving projects list from `/browse/` to `/browse/projects` and adding the landing page at `/browse/` is a sensible restructure. All internal template links already point to `/browse/projects/{{ slug }}` so no broken links. 6. **Mermaid diagram is hardcoded.** The architecture diagram in `landing.html` is static HTML. This is fine for now — it's a landing page, not generated content. When the architecture changes, someone will need to update this template. Consider noting this in `CLAUDE.md` or the project page so it doesn't go stale. --- ### VERDICT **Approve with nits.** Fix the 4 missing `| urlencode` filters in `landing.html` (nit #1), then this is good to merge. Nits #2 and #3 are nice-to-have and can be deferred.

forgejo_admin referenced this pull request from a commit

2026-02-25 08:00:10 +00:00

Address PR #15 review: urlencode tags, import Repo, add empty-state handling

forgejo_admin added 1 commit

2026-02-25 08:00:10 +00:00

Address PR #15 review: urlencode tags, import Repo, add empty-state handling

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

3fe1776092

- Add | urlencode filter to all 4 tag URL references in landing.html
- Import Repo model in tests/conftest.py so SQLAlchemy creates the table
- Add {% else %} empty-state messages for Projects and Repos card grids

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

forgejo_admin commented

2026-02-25 08:02:33 +00:00

Author

Owner

PR #15 Review — Enhance browse landing page as documentation hub

Reviewer: Fresh PR reviewer (no prior context)
Plan: plan-2026-02-24-docs-foundation

Summary of Changes

This PR (3 commits across the branch) adds:

Landing page (/browse/) — architecture diagram (Mermaid), project cards, repo cards, and categorized documentation lists (SOPs, architecture, conventions, templates)
Repos browse page (/browse/repos) — repos grouped by project with platform/status badges
Schema hardening — RepoCreate/RepoUpdate now use Literal["github", "forgejo"] for platform, Literal["active", "archived"] for status, and HttpUrl for URL validation
Serialization fix — mode="json" added to model_dump() calls in repos.py so HttpUrl objects serialize to plain strings for SQLAlchemy
Empty-state handling — {% else %} blocks on {% for %} loops for projects and repos
URL encoding — | urlencode on tag names in href attributes (consistent with note.html)
Test fixture — Repo model imported in conftest.py so SQLAlchemy creates the repos table

BLOCKERS

None. The code is correct and ready to merge.

NITS

No test coverage for browse routes. The existing test suite covers only the API CRUD endpoints. There are no tests hitting /browse/, /browse/repos, or any frontend route. This is not a blocker for this PR — the routes are simple read-only template renders — but it would be good to add at least a smoke test (assert 200 status) in a follow-up.
Inline styles in landing.html. Lines like <h3 style="margin-top: 1rem; font-size: 1rem; color: #555;"> appear 4 times. These could be a CSS class in base.html (e.g., .doc-section-heading). Not a blocker — the pattern is contained to one file and is easy to refactor later.
Documentation sections repeat the same tag-rendering pattern. The SOPs, Architecture, Conventions, and Templates blocks in landing.html (lines 92-146) are nearly identical — only the variable name differs. A Jinja2 macro or {% include %} with a parameter could DRY this up. Low priority since the repetition is small and readable.
repos.html has no empty state. The landing page now has {% else %} for empty projects/repos, but repos.html has no fallback if there are zero repos. Minor inconsistency.
_has_tag helper is defined inside the route function. This is fine for now, but if more routes need tag filtering, consider extracting it to a utility.

Security Review

Autoescape is ON — FastAPI's Jinja2Templates sets autoescape=True by default. All {{ }} expressions are HTML-escaped.
| safe only used for html_content in note.html, with an explicit security comment documenting the trust boundary. Not changed in this PR.
{{ repo.url }} in href attributes — Jinja2 autoescape handles HTML entity escaping. The HttpUrl validation in the schema ensures only valid HTTP(S) URLs are stored, preventing javascript: URI injection.
| urlencode on tag names — correctly prevents broken URLs from tag names containing special characters.
No SQL injection risk — all queries use SQLAlchemy ORM parameterized queries.

Edge Cases Verified

Empty projects/repos — {% else %} blocks render "No projects yet." / "No repos yet." messages.
Repos without projects — {% if repo.project %} guard prevents None.name errors. repos.html also handles unassigned repos in a separate section.
Notes with multiple matching doc tags — a note tagged both sop and convention will appear in both sections. This is correct behavior (not a duplicate bug).
HttpUrl trailing slash — Pydantic's HttpUrl normalizes https://example.com to https://example.com/. This is stored in the DB and displayed as-is. Acceptable.

Performance Review

Landing page makes 3 queries: projects, repos (with subqueryload), and doc_notes (with subqueryload + tag filter). No N+1 issues. The in-Python filtering for tag categories iterates the doc_notes list 4 times — negligible for the expected data volume.
Repos page makes 2 queries: repos (with subqueryload) and projects. Clean.

Verdict: APPROVED

Clean, well-structured PR. The schema hardening (Literal enums, HttpUrl, mode="json") is a solid improvement. The landing page follows existing patterns. No blockers.

The nits above are suggestions for future cleanup, not blockers for merge.

## PR #15 Review — Enhance browse landing page as documentation hub **Reviewer:** Fresh PR reviewer (no prior context) **Plan:** `plan-2026-02-24-docs-foundation` --- ### Summary of Changes This PR (3 commits across the branch) adds: 1. **Landing page** (`/browse/`) — architecture diagram (Mermaid), project cards, repo cards, and categorized documentation lists (SOPs, architecture, conventions, templates) 2. **Repos browse page** (`/browse/repos`) — repos grouped by project with platform/status badges 3. **Schema hardening** — `RepoCreate`/`RepoUpdate` now use `Literal["github", "forgejo"]` for platform, `Literal["active", "archived"]` for status, and `HttpUrl` for URL validation 4. **Serialization fix** — `mode="json"` added to `model_dump()` calls in `repos.py` so `HttpUrl` objects serialize to plain strings for SQLAlchemy 5. **Empty-state handling** — `{% else %}` blocks on `{% for %}` loops for projects and repos 6. **URL encoding** — `| urlencode` on tag names in href attributes (consistent with `note.html`) 7. **Test fixture** — `Repo` model imported in `conftest.py` so SQLAlchemy creates the repos table --- ### BLOCKERS **None.** The code is correct and ready to merge. --- ### NITS 1. **No test coverage for browse routes.** The existing test suite covers only the API CRUD endpoints. There are no tests hitting `/browse/`, `/browse/repos`, or any frontend route. This is not a blocker for this PR — the routes are simple read-only template renders — but it would be good to add at least a smoke test (assert 200 status) in a follow-up. 2. **Inline styles in `landing.html`.** Lines like `<h3 style="margin-top: 1rem; font-size: 1rem; color: #555;">` appear 4 times. These could be a CSS class in `base.html` (e.g., `.doc-section-heading`). Not a blocker — the pattern is contained to one file and is easy to refactor later. 3. **Documentation sections repeat the same tag-rendering pattern.** The SOPs, Architecture, Conventions, and Templates blocks in `landing.html` (lines 92-146) are nearly identical — only the variable name differs. A Jinja2 macro or `{% include %}` with a parameter could DRY this up. Low priority since the repetition is small and readable. 4. **`repos.html` has no empty state.** The landing page now has `{% else %}` for empty projects/repos, but `repos.html` has no fallback if there are zero repos. Minor inconsistency. 5. **`_has_tag` helper is defined inside the route function.** This is fine for now, but if more routes need tag filtering, consider extracting it to a utility. --- ### Security Review - **Autoescape is ON** — FastAPI's `Jinja2Templates` sets `autoescape=True` by default. All `{{ }}` expressions are HTML-escaped. - **`| safe` only used for `html_content`** in `note.html`, with an explicit security comment documenting the trust boundary. Not changed in this PR. - **`{{ repo.url }}` in href attributes** — Jinja2 autoescape handles HTML entity escaping. The `HttpUrl` validation in the schema ensures only valid HTTP(S) URLs are stored, preventing `javascript:` URI injection. - **`| urlencode` on tag names** — correctly prevents broken URLs from tag names containing special characters. - **No SQL injection risk** — all queries use SQLAlchemy ORM parameterized queries. --- ### Edge Cases Verified - **Empty projects/repos** — `{% else %}` blocks render "No projects yet." / "No repos yet." messages. - **Repos without projects** — `{% if repo.project %}` guard prevents `None.name` errors. `repos.html` also handles unassigned repos in a separate section. - **Notes with multiple matching doc tags** — a note tagged both `sop` and `convention` will appear in both sections. This is correct behavior (not a duplicate bug). - **`HttpUrl` trailing slash** — Pydantic's `HttpUrl` normalizes `https://example.com` to `https://example.com/`. This is stored in the DB and displayed as-is. Acceptable. --- ### Performance Review - **Landing page makes 3 queries**: projects, repos (with subqueryload), and doc_notes (with subqueryload + tag filter). No N+1 issues. The in-Python filtering for tag categories iterates the `doc_notes` list 4 times — negligible for the expected data volume. - **Repos page makes 2 queries**: repos (with subqueryload) and projects. Clean. --- ### Verdict: **APPROVED** Clean, well-structured PR. The schema hardening (`Literal` enums, `HttpUrl`, `mode="json"`) is a solid improvement. The landing page follows existing patterns. No blockers. The nits above are suggestions for future cleanup, not blockers for merge.

forgejo_admin merged commit a84d32c514 into main

2026-02-25 08:03:15 +00:00

forgejo_admin deleted branch 13-landing-page-docs-hub

2026-02-25 08:03:15 +00:00

forgejo_admin referenced this pull request from a commit

2026-02-25 08:03:15 +00:00

Enhance browse landing page as documentation hub (#15)