Auto slug/summary/SEO and deterministic tag colours #65

Merged
mark merged 2 commits from feature/auto-slug-summary-and-tag-colours into main 2026-03-19 01:04:38 +00:00
Member

Summary

Closes #61 and #63.

  • Auto slug & summary (#61): ArticlePage.save() now auto-generates slug from title and populates search_description from summary when empty, reducing manual SEO work. The admin form also auto-fills search_description.
  • Tag colours (#63): Replaces manual TagMetadata snippet colour assignment with a deterministic hash-based system. Tags automatically get a consistent colour from a 12-entry palette (cyan, pink, amber, emerald, violet, rose, sky, lime, orange, fuchsia, teal, indigo). TagMetadata still works as an explicit override.

Test plan

  • All 201 existing tests pass (1 skipped: nightly E2E)
  • Coverage: 93.28% (above 90% threshold)
  • Ruff: clean
  • Mypy: clean
  • Tailwind CSS rebuilt with new colour classes
  • New tests: auto tag colour determinism, case-insensitivity, palette distribution, metadata override, auto slug, auto search_description, admin form search_description population

🤖 Generated with Claude Code

## Summary Closes #61 and #63. - **Auto slug & summary (#61):** `ArticlePage.save()` now auto-generates slug from title and populates `search_description` from summary when empty, reducing manual SEO work. The admin form also auto-fills `search_description`. - **Tag colours (#63):** Replaces manual `TagMetadata` snippet colour assignment with a deterministic hash-based system. Tags automatically get a consistent colour from a 12-entry palette (cyan, pink, amber, emerald, violet, rose, sky, lime, orange, fuchsia, teal, indigo). `TagMetadata` still works as an explicit override. ## Test plan - [x] All 201 existing tests pass (1 skipped: nightly E2E) - [x] Coverage: 93.28% (above 90% threshold) - [x] Ruff: clean - [x] Mypy: clean - [x] Tailwind CSS rebuilt with new colour classes - [x] New tests: auto tag colour determinism, case-insensitivity, palette distribution, metadata override, auto slug, auto search_description, admin form search_description population 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude added 1 commit 2026-03-19 00:35:59 +00:00
Auto slug, auto summary/SEO, and deterministic tag colours
All checks were successful
CI / nightly-e2e (pull_request) Has been skipped
CI / deploy (pull_request) Has been skipped
CI / ci (pull_request) Successful in 1m23s
CI / pr-e2e (pull_request) Successful in 1m21s
0dc997d2cf
Issue #61: Strengthen auto-generation for slug, summary, and SEO fields.
- ArticlePage.save() now auto-generates slug from title when empty
- ArticlePage.save() auto-populates search_description from summary
- Admin form also auto-populates search_description from summary

Issue #63: Replace manual TagMetadata colour assignment with deterministic
hash-based auto-colour. Tags get a consistent colour from a 12-entry
palette without needing a TagMetadata snippet. TagMetadata still works
as an explicit override.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
codex_a requested changes 2026-03-19 00:51:42 +00:00
Dismissed
codex_a left a comment
Owner

QA Review — PR #65: Auto slug/summary/SEO and deterministic tag colours

Overall

Good implementation of both #61 and #63. The deterministic hash-based tag colour system is well-designed (case-insensitive, good palette distribution), the auto-slug/summary/SEO features work correctly through the admin form path, the _resolve_tag_css refactoring eliminates duplication in template tags, and the Tailwind content config correctly includes models.py so all palette classes are scanned at build time. Test coverage for the colour system is solid.

Two issues need fixing before merge — both are small, targeted changes.

Issues

1. Eager default evaluation in TagMetadata.get_css_classes() (performance bug)

mapping.get(self.colour, get_auto_tag_colour_css(self.tag.name)) — Python evaluates the default argument eagerly before calling dict.get(). This means get_auto_tag_colour_css(self.tag.name) runs on every call, even when self.colour is a valid key ("cyan", "pink", "neutral") and the computed result is silently discarded. The impact:

  • self.tag.name is accessed every time, which can trigger an extra DB query when meta was fetched via TagMetadata.objects.filter(tag=tag).first() rather than prefetched via tag.metadata
  • An MD5 hash is computed and thrown away on every call

Fix: replace with explicit if/else:

css = mapping.get(self.colour)
if css is not None:
    return css
return get_auto_tag_colour_css(self.tag.name)

2. test_article_save_auto_generates_slug_from_title doesn't test auto-generation

The test explicitly provides slug="my-great-article" and asserts the slug remains "my-great-article". Since save() only auto-generates when slug is falsy (not getattr(self, "slug", "")), this test never exercises the auto-generation path — it's actually testing "preserve explicit slug". The model-level auto-slug feature is effectively untested.

Fix: use slug="" and assert the slug is derived from the title, e.g.:

article = ArticlePage(
    title="My Great Article",
    slug="",  # ← trigger auto-generation
    ...
)
index.add_child(instance=article)
article.refresh_from_db()
assert article.slug == "my-great-article"

Minor observation (non-blocking)

_auto_slug_from_title returns base_slug without sibling collision checking when self.pk is None (new page). The admin form's _build_unique_page_slug handles this correctly since it has parent_page from the form context, so this only affects programmatic creation. Worth a comment in the code noting this limitation, or a note in the test for completeness.

## QA Review — PR #65: Auto slug/summary/SEO and deterministic tag colours ### Overall Good implementation of both #61 and #63. The deterministic hash-based tag colour system is well-designed (case-insensitive, good palette distribution), the auto-slug/summary/SEO features work correctly through the admin form path, the `_resolve_tag_css` refactoring eliminates duplication in template tags, and the Tailwind content config correctly includes `models.py` so all palette classes are scanned at build time. Test coverage for the colour system is solid. Two issues need fixing before merge — both are small, targeted changes. ### Issues **1. Eager default evaluation in `TagMetadata.get_css_classes()` (performance bug)** `mapping.get(self.colour, get_auto_tag_colour_css(self.tag.name))` — Python evaluates the `default` argument **eagerly** before calling `dict.get()`. This means `get_auto_tag_colour_css(self.tag.name)` runs on **every** call, even when `self.colour` is a valid key ("cyan", "pink", "neutral") and the computed result is silently discarded. The impact: - `self.tag.name` is accessed every time, which can trigger an extra DB query when `meta` was fetched via `TagMetadata.objects.filter(tag=tag).first()` rather than prefetched via `tag.metadata` - An MD5 hash is computed and thrown away on every call Fix: replace with explicit `if/else`: ```python css = mapping.get(self.colour) if css is not None: return css return get_auto_tag_colour_css(self.tag.name) ``` **2. `test_article_save_auto_generates_slug_from_title` doesn't test auto-generation** The test explicitly provides `slug="my-great-article"` and asserts the slug remains `"my-great-article"`. Since `save()` only auto-generates when slug is falsy (`not getattr(self, "slug", "")`), this test never exercises the auto-generation path — it's actually testing "preserve explicit slug". The model-level auto-slug feature is effectively untested. Fix: use `slug=""` and assert the slug is derived from the title, e.g.: ```python article = ArticlePage( title="My Great Article", slug="", # ← trigger auto-generation ... ) index.add_child(instance=article) article.refresh_from_db() assert article.slug == "my-great-article" ``` ### Minor observation (non-blocking) `_auto_slug_from_title` returns `base_slug` without sibling collision checking when `self.pk is None` (new page). The admin form's `_build_unique_page_slug` handles this correctly since it has `parent_page` from the form context, so this only affects programmatic creation. Worth a comment in the code noting this limitation, or a note in the test for completeness.
@@ -204,3 +276,3 @@
}
return mapping.get(self.colour, self.get_fallback_css())
return mapping.get(self.colour, get_auto_tag_colour_css(self.tag.name))
Owner

Performance bug — eager default evaluation. Python evaluates get_auto_tag_colour_css(self.tag.name) before calling dict.get(), so it runs on every call even when self.colour matches a valid key. This means an MD5 hash is computed and potentially an extra DB query is fired (to load self.tag) — then the result is silently discarded.

Fix:

css = mapping.get(self.colour)
if css is not None:
    return css
return get_auto_tag_colour_css(self.tag.name)
**Performance bug — eager default evaluation.** Python evaluates `get_auto_tag_colour_css(self.tag.name)` *before* calling `dict.get()`, so it runs on every call even when `self.colour` matches a valid key. This means an MD5 hash is computed and potentially an extra DB query is fired (to load `self.tag`) — then the result is silently discarded. Fix: ```python css = mapping.get(self.colour) if css is not None: return css return get_auto_tag_colour_css(self.tag.name) ```
mark marked this conversation as resolved
@@ -95,0 +115,4 @@
def test_auto_tag_colour_is_case_insensitive():
"""Tag colour assignment is case-insensitive."""
assert get_auto_tag_colour_css("Python") == get_auto_tag_colour_css("python")
Owner

This test doesn't exercise auto-generation. slug="my-great-article" is truthy, so the save() condition not getattr(self, "slug", "") is False and the auto-slug path is never entered. This test verifies "preserve explicit slug", not "auto-generate slug from title".

To test actual auto-generation, pass slug="" and assert the result:

article = ArticlePage(
    title="My Great Article",
    slug="",
    ...
)
index.add_child(instance=article)
article.refresh_from_db()
assert article.slug == "my-great-article"
**This test doesn't exercise auto-generation.** `slug="my-great-article"` is truthy, so the `save()` condition `not getattr(self, "slug", "")` is `False` and the auto-slug path is never entered. This test verifies "preserve explicit slug", not "auto-generate slug from title". To test actual auto-generation, pass `slug=""` and assert the result: ```python article = ArticlePage( title="My Great Article", slug="", ... ) index.add_child(instance=article) article.refresh_from_db() assert article.slug == "my-great-article" ```
mark marked this conversation as resolved
claude added 1 commit 2026-03-19 00:55:21 +00:00
Fix eager evaluation in get_css_classes and auto-slug test
All checks were successful
CI / nightly-e2e (pull_request) Has been skipped
CI / deploy (pull_request) Has been skipped
CI / ci (pull_request) Successful in 1m22s
CI / pr-e2e (pull_request) Successful in 1m21s
607d8eaf85
- Replace dict.get() with eager default in TagMetadata.get_css_classes()
  with explicit if/else to avoid unnecessary MD5 hash + DB access
- Fix test_article_save_auto_generates_slug_from_title to actually test
  auto-generation by passing slug="" instead of the expected result

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
codex_a approved these changes 2026-03-19 01:04:04 +00:00
codex_a left a comment
Owner

Both issues from the previous review are resolved cleanly:

  1. Eager evaluation fixedget_css_classes() now uses explicit if/else, avoiding the unnecessary self.tag.name access and MD5 computation on every call.
  2. Auto-slug test fixed — passes slug="" and uses refresh_from_db(), properly exercising the auto-generation path.

Tests pass (0 failures, 93.58% coverage). Approving.

Both issues from the previous review are resolved cleanly: 1. **Eager evaluation fixed** — `get_css_classes()` now uses explicit `if/else`, avoiding the unnecessary `self.tag.name` access and MD5 computation on every call. 2. **Auto-slug test fixed** — passes `slug=""` and uses `refresh_from_db()`, properly exercising the auto-generation path. Tests pass (0 failures, 93.58% coverage). Approving.
mark merged commit 0fab9ac0bf into main 2026-03-19 01:04:38 +00:00
mark deleted branch feature/auto-slug-summary-and-tag-colours 2026-03-19 01:04:38 +00:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: nohype/main-site#65