Codebase due-diligence cleanup: issues #31–#38

[plx]

Works through GitHub issues #31–#38 from the codebase due-diligence pass — one focused, independently-verified commit per issue.

Commits (one per issue)

Issue	Type	Change
#31	perf	Drop unused item.render() passes on the home / briefs / category list pages (the <Content /> was never used)
#32	perf	Memoize getCategory() so each category.yaml is read/parsed at most once per build instead of once per brief card
#33	fix	Correct readingTime to max(1, ceil(words / 200)) (no more blanket +1 minute); add a Vitest harness + edge-case tests
#34	refactor	Narrow getProjectCardProps to CollectionEntry<"projects"> and extract a shared getStandardCardProps
#35	chore	Remove the unused, broken CallToAction.astro (imported a non-existent SCSS path)
#36	fix	Fix dateRange formatting and open-ended end dates (MMM YYYY - Present); add branch tests
#37	feat	Validate demoURL / repoURL / ogImage as z.string().url() in the content schema
#38	refactor	Centralize the draft-filter + date-desc-sort (+ optional slice) pipeline into src/lib/collections.ts and route every page / getStaticPaths / RSS through it

Test infrastructure

The repo previously had no unit-test runner (only Playwright QA). #33/#36 add a minimal Vitest setup:

• vitest devDependency + test:unit script
• vitest.config.ts scopes Vitest to src/**/*.test.ts so it never collides with the Playwright tests/*.spec.ts suites
• src/lib/utils.test.ts (15 assertions covering reading-time and date-range edge cases)
• a Run Unit Tests step added to the shared build.yml CI workflow (runs on PR validation and pre-deploy) and to the local test:ci aggregates

Validation

• npm run test:ci ✅ — lint, 15 unit tests, source + HTML spellcheck, astro check (0 errors), internal link validation
• npm run qa (Playwright, chromium) ✅ — 30/30 passing (content, navigation, accessibility, responsive)
• Refactors (Avoid unnecessary content rendering on list pages #31, Narrow and deduplicate project card helper typing #34, Centralize duplicated collection query logic #38) verified behavior-preserving: homepage still shows 3 posts / 5 briefs / 2 projects, RSS still newest-first, draft filtering and per-category ordering unchanged
• Independent multi-agent review of the full diff returned 0 confirmed findings

Closes #31
Closes #32
Closes #33
Closes #34
Closes #35
Closes #36
Closes #37
Closes #38

🤖 Generated with Claude Code

[github-actions]

PR Validation ✅

All checks passed! Ready for review.

Checks Performed:

✓ Linting
✓ Spell check (source)
✓ Type checking & Build
✓ Spell check (HTML)
✓ Internal link validation
✓ Artifact upload

This is a complete dry-run of the deployment process, ensuring your changes will deploy successfully when merged.

[claude]

Code Review — Codebase Due-Diligence Cleanup (#31–#38)

Overall this is a well-structured, focused PR. The atomic-commit-per-issue approach makes each change easy to reason about independently, and the test harness addition (Vitest for unit tests, scoped away from Playwright) is a solid foundation. Findings below, roughly ordered by severity.

---

Issues

category.ts shows as a binary diff with 0 additions / 0 deletions

The file is flagged as modified (changeType: MODIFIED) but the GitHub API reports additions: 0, deletions: 0 and git diff renders it as a binary comparison. This means the memoization change from issue #32 is not visible in the diff at all. It is possible this is just a line-ending normalization artifact from the merge commit, but it is worth verifying:

• Did the memoization land in a prior commit that the merge squashed away?
• Or does category.ts on the branch actually differ from what is on main?

If the memoization is genuinely absent, issue #32 is not addressed by this PR. If it is present but git cannot diff it, the binary encoding problem should be fixed before merge (e.g., normalize line endings, remove BOM).

---

z.string().url() may silently break existing content

z.string().url() (Zod's URL validator) requires an absolute URL with a valid scheme. Any existing frontmatter that uses a relative path (/assets/og.png), a protocol-relative URL (//example.com/img.png), or a bare domain (github.com/plx) will now fail schema validation at build time and potentially cause astro build to error. Worth auditing existing content before this lands — especially ogImage, which is the most likely field to hold a path rather than a full URL.

---

Missing newline at end of src/lib/utils.ts

The diff ends with No newline at end of file after the dateRange function. Minor, but conventionally all text files should end with a newline.

---

Observations (non-blocking)

readingTime fix — Correct. The old formula (wordCount / 200) + 1 always added a phantom minute. The new Math.max(1, Math.ceil(wordCount / 200)) is the standard formula. The tag-stripping change from replace(/<[^>]+>/g, "") to replace(/<[^>]+>/g, " ") also correctly prevents adjacent words from being joined when tags are removed (e.g. <em>hello</em><em>world</em> would previously produce "helloworld").

dateRange fix — The original code produced ${startMonth}${startYear} (no space between month and year) and relied on endMonth/endYear variables that were undefined when the end date was omitted. The rewrite is clean and the formatMonthYear helper removes the duplication.

collections.ts — The published / byDateDesc / getPublishedCollection split is a good decomposition: the first two are pure and easily testable, the third wraps the async getCollection call. The one subtle choice is that getPublishedCollection always fetches all entries before slicing — fine for a static-site build where all content is local, just worth noting.

Test coverage — The 15 assertions cover the two fixed functions well. published and byDateDesc from collections.ts are not tested, but they are one-liners that are trivially correct, so that trade-off is fine.

getProjectCardProps narrowing — The original signature CollectionEntry<"blog"> | CollectionEntry<"projects"> was clearly wrong for a function named getProjectCardProps. The narrowing to CollectionEntry<"projects"> is correct.

rss.xml.ts — Still imports getCollection directly, then applies published + byDateDesc from the new module. This is correct: the RSS feed merges two collections before sorting, which getPublishedCollection does not support (single-collection only). The approach is the right one given the current API surface.

---

Summary

The binary diff on category.ts is the main thing to resolve before merging — it either hides the #32 memoization change or indicates an encoding issue that will confuse future git blame/git diff runs. The z.string().url() strictness is worth a quick content audit to confirm no existing frontmatter uses relative paths for those fields. Everything else looks good.