Skip to content

Curate projects listing#1350

Open
schalkneethling wants to merge 20 commits into
mainfrom
codex/curated-projects-listing-1344
Open

Curate projects listing#1350
schalkneethling wants to merge 20 commits into
mainfrom
codex/curated-projects-listing-1344

Conversation

@schalkneethling

@schalkneethling schalkneethling commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Reworks /projects as the curated "What I want to exist" listing.
  • Adds the project content collection, stable ordering/grouping, the 13 starter projects, and the Little demos section.
  • Adds internal project detail pages with editorial project summaries, repo/live/doc links, project images, technologies, contribution guidance, and accessibility coverage.
  • Completes starter project content and adds a manual tracked cache for GOAL.md and ROADMAP.md source material.
  • Adds the project-local Codex skill for maintaining What I want to exist project entries over time.

Fixes #1344
Fixes #1345
Fixes #1346
Fixes #1347
Fixes #1348
Fixes #1349

Verification

  • pnpm run test:unit
  • pnpm run typecheck
  • pnpm run build
  • pnpm run test:a11y

Notes

  • src/content/posts/because-i-want-it-to-exist.md is intentionally excluded from this PR.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR replaces the /projects page with a curated content-backed listing and adds a matching project detail route. It also seeds project entries, updates project card rendering, and adjusts related tests, navigation, and Playwright setup.

Changes

Curated Projects Feature

Layer / File(s) Summary
Project content collection schema
src/content.config.ts
Adds a projectsCollection schema and registers it in collections.
Project filtering and sorting utilities
src/lib/projectFilters.ts, tests/projectFilters.test.ts
Adds ProjectCategory, sortProjectsByOrder, and getProjectsByCategory, with tests for ordering and category grouping.
ProjectCard and ProjectList components
src/components/Projects/ProjectCard.astro, src/components/Projects/ProjectList.astro
Adds optional linkLabel support to ProjectCard and a new ProjectList component for categorized masonry rendering.
Projects page and navigation rewrite
src/pages/projects.astro, src/components/Header/Navigation.astro, tests/urls.json, tests/a11y.spec.ts, playwright.config.ts, tests/projects.spec.ts
Rewrites /projects to load content entries by category, updates page copy and nav text, and adjusts Playwright setup and checks.
Project detail page
src/pages/projects/[slug].astro
Adds the dynamic project detail route with static path generation, conditional technologies/live-link sections, and page-specific styles.
Seeded project entries
src/content/projects/*.md
Adds project Markdown entries with frontmatter metadata for the curated listing.
Draft frontmatter
src/content/drafts/throwing-confetti-with-css-random.mdx
Adds frontmatter metadata for a new MDX draft file.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning An unrelated draft MDX file was added under src/content/drafts, which is outside the /projects listing objectives. Remove the draft content file from this PR or split it into a separate change if intentional.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR satisfies #1344 by renaming /projects, adding the content model, seeding the 13 starters and Little demos, and routing cards to internal project pages.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: turning the projects page into a curated listing.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/curated-projects-listing-1344

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
src/content.config.ts (1)

31-31: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

No uniqueness constraint on order within a category.

Ties in order fall back to the underlying glob-loader enumeration order via the stable sort in sortProjectsByOrder, which isn't guaranteed to reflect the "agreed activity-based order" across filesystems/CI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/content.config.ts` at line 31, The `order` field in `content.config.ts`
only validates positivity, so duplicate values can still lead to unstable
category sorting in `sortProjectsByOrder`. Add a uniqueness check for `order`
within each category (or equivalent validation in the schema/config path) so
conflicting entries are rejected instead of relying on glob-loader enumeration
order. Use the `order` schema definition and the `sortProjectsByOrder` flow as
the main points to update.
src/components/Projects/ProjectCard.astro (2)

49-51: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Default link text no longer matches the link's destination.

url now points to an internal /projects/{slug} page (per ProjectList.astro Line 27), not GitHub, but the fallback text is still "View on GitHub". It's currently masked because ProjectList always passes linkLabel="View project", but the stale default is misleading for any future caller (or a11y users on screen readers) that omits the prop.

🛠️ Proposed fix
-      {linkLabel ?? "View on GitHub"} <span aria-hidden="true">&rarr;</span>
+      {linkLabel ?? "View project"} <span aria-hidden="true">&rarr;</span>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Projects/ProjectCard.astro` around lines 49 - 51, The default
fallback label in ProjectCard.astro no longer matches the destination for url,
since it now points to an internal project page rather than GitHub. Update the
fallback text used by the anchor in ProjectCard so the default aligns with the
internal projects route, and keep the linkLabel override behavior intact for
callers like ProjectList that already pass an explicit label.

2-11: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

stars should be optional to avoid a misleading "0 stars" badge.

The content schema (src/content.config.ts) declares stars as z.number().int().nonnegative().optional(), but Props.stars here is required (stars: number;), and the footer badge at Lines 41-47 always renders it unconditionally (unlike language, which is guarded). This forces the call site in ProjectList.astro (Line 29) to coerce missing stars to 0 (project.data.stars ?? 0), which will display "0 stars" for demo entries that never tracked a star count instead of omitting the badge — misrepresenting untracked projects as unpopular.

🛠️ Proposed fix
 interface Props {
   name: string;
   description: string;
   url: string;
   language: string | null;
-  stars: number;
+  stars: number | null;
   imageUrl: string;
   featured?: boolean;
   linkLabel?: string;
 }
 const { name, description, url, language, stars, imageUrl, featured, linkLabel } =
   Astro.props;
-      <span
-        class="project-card-stars"
-        aria-label={`${stars} ${stars === 1 ? "star" : "stars"} on GitHub`}
-      >
-        <span aria-hidden="true">&`#9733`;</span>
-        {stars}
-      </span>
+      {
+        stars !== null && (
+          <span
+            class="project-card-stars"
+            aria-label={`${stars} ${stars === 1 ? "star" : "stars"} on GitHub`}
+          >
+            <span aria-hidden="true">&`#9733`;</span>
+            {stars}
+          </span>
+        )
+      }

Also applies to: 41-47

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Projects/ProjectCard.astro` around lines 2 - 11, Make
ProjectCard.astro treat stars as optional instead of required, since the content
schema allows it to be omitted and missing values should not render a misleading
“0 stars” badge. Update the Props interface in ProjectCard to use an optional
stars field, and adjust the footer badge rendering logic in ProjectCard so it
only shows when stars is present, similar to how language is guarded. Then
remove the fallback coercion in ProjectList so it passes through the value
directly from project.data.stars.
src/pages/projects.astro (1)

3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Mixed import conventions (alias vs. relative).

ProjectList is imported via the @components/... alias while getProjectsByCategory uses a relative path (../lib/projectFilters) and BaseLayout also uses a relative path. Consider using one convention consistently within the file.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/projects.astro` around lines 3 - 5, The imports in the projects
page mix alias-based and relative paths, so make the module style consistent
across the file. Update the imports in the projects page to use one convention
for `ProjectList`, `getProjectsByCategory`, and `BaseLayout` rather than mixing
`@components/...` with relative paths. Keep the existing symbols unchanged and
only adjust the import specifiers so they all follow the same project-wide
convention.
src/components/Projects/ProjectList.astro (1)

29-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Downstream of ProjectCard's required stars prop.

project.data.stars ?? 0 synthesizes a fake 0 for entries without a tracked star count (schema marks stars optional). See the root-cause comment in ProjectCard.astro (Lines 2-11, 41-47) recommending an optional stars with conditional rendering instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Projects/ProjectList.astro` at line 29, Update ProjectList so
it no longer fabricates a default star count for projects with no tracked stars;
the `project.data.stars ?? 0` fallback should be removed so downstream logic can
distinguish missing data. Adjust the `ProjectCard` integration to match the
root-cause guidance in `ProjectCard.astro` by making the `stars` prop optional
and rendering the stars UI only when a real value exists, using the
`ProjectList`/`ProjectCard` symbols to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/Projects/ProjectList.astro`:
- Around line 23-34: The ProjectList.astro cards are linking to project detail
URLs that do not exist yet, so add the missing projects detail route under
src/pages before keeping those links. Create a page that matches the ProjectCard
url pattern used in ProjectList and uses the project id to render a detail view,
so /projects/${project.id} resolves instead of 404ing.

---

Nitpick comments:
In `@src/components/Projects/ProjectCard.astro`:
- Around line 49-51: The default fallback label in ProjectCard.astro no longer
matches the destination for url, since it now points to an internal project page
rather than GitHub. Update the fallback text used by the anchor in ProjectCard
so the default aligns with the internal projects route, and keep the linkLabel
override behavior intact for callers like ProjectList that already pass an
explicit label.
- Around line 2-11: Make ProjectCard.astro treat stars as optional instead of
required, since the content schema allows it to be omitted and missing values
should not render a misleading “0 stars” badge. Update the Props interface in
ProjectCard to use an optional stars field, and adjust the footer badge
rendering logic in ProjectCard so it only shows when stars is present, similar
to how language is guarded. Then remove the fallback coercion in ProjectList so
it passes through the value directly from project.data.stars.

In `@src/components/Projects/ProjectList.astro`:
- Line 29: Update ProjectList so it no longer fabricates a default star count
for projects with no tracked stars; the `project.data.stars ?? 0` fallback
should be removed so downstream logic can distinguish missing data. Adjust the
`ProjectCard` integration to match the root-cause guidance in
`ProjectCard.astro` by making the `stars` prop optional and rendering the stars
UI only when a real value exists, using the `ProjectList`/`ProjectCard` symbols
to locate the change.

In `@src/content.config.ts`:
- Line 31: The `order` field in `content.config.ts` only validates positivity,
so duplicate values can still lead to unstable category sorting in
`sortProjectsByOrder`. Add a uniqueness check for `order` within each category
(or equivalent validation in the schema/config path) so conflicting entries are
rejected instead of relying on glob-loader enumeration order. Use the `order`
schema definition and the `sortProjectsByOrder` flow as the main points to
update.

In `@src/pages/projects.astro`:
- Around line 3-5: The imports in the projects page mix alias-based and relative
paths, so make the module style consistent across the file. Update the imports
in the projects page to use one convention for `ProjectList`,
`getProjectsByCategory`, and `BaseLayout` rather than mixing `@components/...`
with relative paths. Keep the existing symbols unchanged and only adjust the
import specifiers so they all follow the same project-wide convention.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 257a5060-6d39-4860-ade6-b56de4184844

📥 Commits

Reviewing files that changed from the base of the PR and between 5188b24 and 355c43c.

📒 Files selected for processing (25)
  • playwright.config.ts
  • src/components/Header/Navigation.astro
  • src/components/Projects/ProjectCard.astro
  • src/components/Projects/ProjectList.astro
  • src/content.config.ts
  • src/content/drafts/throwing-confetti-with-css-random.mdx
  • src/content/projects/common-components.md
  • src/content/projects/create-project-calavera.md
  • src/content/projects/css-benchpress.md
  • src/content/projects/css-community-reset.md
  • src/content/projects/css-custom-property-inspector.md
  • src/content/projects/css-expect.md
  • src/content/projects/css-media-pseudo-polyfill.md
  • src/content/projects/css-property-type-validator.md
  • src/content/projects/css-tree-ast-viewer.md
  • src/content/projects/jsconsole.md
  • src/content/projects/little-demos.md
  • src/content/projects/masonry-gridlanes-wc.md
  • src/content/projects/ossreleasefeed-v2.md
  • src/content/projects/refined-plan-mode.md
  • src/content/projects/skills-autoresearch-flue.md
  • src/content/projects/web-platform-pulse.md
  • src/lib/projectFilters.ts
  • src/pages/projects.astro
  • tests/projectFilters.test.ts

Comment thread src/components/Projects/ProjectList.astro

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/pages/projects/[slug].astro (1)

40-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

rel="noopener noreferrer" without target="_blank" is a no-op.

Neither link opens in a new tab, so the rel attribute provides no security or UX benefit here. Consider adding target="_blank" to open external repo/live links in a new tab (keeping rel="noopener noreferrer" meaningful), or drop the attribute if same-tab navigation is intended.

♻️ Suggested fix
-          <a href={project.data.repoUrl} rel="noopener noreferrer">
+          <a href={project.data.repoUrl} target="_blank" rel="noopener noreferrer">
             View the repository
           </a>
...
-              <a href={project.data.liveUrl} rel="noopener noreferrer">
+              <a href={project.data.liveUrl} target="_blank" rel="noopener noreferrer">
                 Visit the live project
               </a>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/projects/`[slug].astro around lines 40 - 58, The links in the
project details section currently use rel="noopener noreferrer" without opening
in a new tab, so the attribute has no effect. Update the anchor elements in the
project links markup to either add target="_blank" for the repository and live
project links while keeping rel="noopener noreferrer", or remove rel if same-tab
navigation is intended. Use the project links section in the [slug] page and its
existing project.data.repoUrl / project.data.liveUrl anchors to locate the
change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/pages/projects/`[slug].astro:
- Around line 40-58: The links in the project details section currently use
rel="noopener noreferrer" without opening in a new tab, so the attribute has no
effect. Update the anchor elements in the project links markup to either add
target="_blank" for the repository and live project links while keeping
rel="noopener noreferrer", or remove rel if same-tab navigation is intended. Use
the project links section in the [slug] page and its existing
project.data.repoUrl / project.data.liveUrl anchors to locate the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2cdd0d96-ec3f-4ce5-8a2c-0287385639ca

📥 Commits

Reviewing files that changed from the base of the PR and between 355c43c and 98ae281.

📒 Files selected for processing (3)
  • src/pages/projects/[slug].astro
  • tests/a11y.spec.ts
  • tests/urls.json

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
tests/projects.spec.ts (2)

3-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test only verifies one card, but title/PR intent implies checking that cards stay visible broadly.

The test name says "keeps project cards visible after masonry upgrade" (plural), and the referenced commit "Fix projects masonry card visibility" suggests a layout bug affecting multiple/all cards. Asserting visibility of just the first heading doesn't validate that other cards (e.g., later items in mainProjects or the "Little demos" section) also render correctly in the masonry layout.

Also, hardcoding "css-community-reset" as first project couples the test to the exact order value in seed content with no explanation; a future reordering of seeded projects would silently break this test without indicating why.

Consider asserting visibility of multiple cards (e.g., first and last in each ProjectList section) to better match the masonry-visibility intent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/projects.spec.ts` around lines 3 - 13, The projects visibility test
only checks one hardcoded card, so it doesn’t cover the broader masonry
visibility behavior implied by the test name and commit intent. Update the test
in projects.spec.ts to assert multiple project headings are visible across the
layout, ideally from both ProjectList sections and including later items, and
avoid relying solely on the fixed "css-community-reset" heading by selecting
cards in a way that reflects the intended section coverage.

1-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Spec doesn't use axe-core per coding guidelines.

This file matches **/*.spec.{ts,tsx} and **/*.{spec,test}.{ts,js}, both of which mandate accessibility-focused testing with custom axe-core integration. This test only checks visibility, with no axe-core scan.

If accessibility coverage for /projects is intentionally centralized in tests/a11y.spec.ts (per the stack outline), consider naming this file differently (e.g. projects.visual.spec.ts or projects.masonry.spec.ts) so it's not implicitly bound by the accessibility-testing guideline, or add the axe-core check here to comply.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/projects.spec.ts` around lines 1 - 13, The /projects Playwright spec
currently only performs a visibility assertion, but files named as *.spec.ts are
expected to include axe-core accessibility coverage. Update the test in
tests/projects.spec.ts by either adding the required custom axe-core scan
alongside the existing assertion, or renaming the test file to a
non-a11y-constrained name such as projects.visual.spec.ts if accessibility is
already covered elsewhere. Use the existing test case and page interaction in
the /projects spec as the place to add or relocate this coverage.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/projects.spec.ts`:
- Around line 3-13: The projects visibility test only checks one hardcoded card,
so it doesn’t cover the broader masonry visibility behavior implied by the test
name and commit intent. Update the test in projects.spec.ts to assert multiple
project headings are visible across the layout, ideally from both ProjectList
sections and including later items, and avoid relying solely on the fixed
"css-community-reset" heading by selecting cards in a way that reflects the
intended section coverage.
- Around line 1-13: The /projects Playwright spec currently only performs a
visibility assertion, but files named as *.spec.ts are expected to include
axe-core accessibility coverage. Update the test in tests/projects.spec.ts by
either adding the required custom axe-core scan alongside the existing
assertion, or renaming the test file to a non-a11y-constrained name such as
projects.visual.spec.ts if accessibility is already covered elsewhere. Use the
existing test case and page interaction in the /projects spec as the place to
add or relocate this coverage.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b400d6d-442f-4048-b5c9-2de149763418

📥 Commits

Reviewing files that changed from the base of the PR and between 98ae281 and 4a7bbd5.

📒 Files selected for processing (2)
  • src/components/Projects/ProjectList.astro
  • tests/projects.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Projects/ProjectList.astro

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
tests/projects.spec.ts (1)

15-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Route interception only covers two hostnames — other project images will hit the real network.

The page.route regex only matches opengraph.githubassets.com and repository-images.githubusercontent.com. Per Context snippet 3, css-community-reset.md's imageUrl matches this pattern, so the specific assertion should pass, but any other seeded project entry whose imageUrl points elsewhere (or is empty) will trigger real network requests during this test, which introduces flakiness in CI environments without outbound network access. Consider intercepting all image requests more broadly (e.g., **/*.{png,jpg,jpeg} or by resourceType() === 'image') and returning the same fixture, only asserting the specific src pattern for the first card.

🔧 Suggested broader interception
   await page.route(
-    /https:\/\/(opengraph\.githubassets\.com|repository-images\.githubusercontent\.com)\/.*/,
+    (url) => url.protocol === "https:",
     async (route) => {
-      await route.fulfill({
-        body: transparentPng,
-        contentType: "image/png",
-      });
+      if (route.request().resourceType() === "image") {
+        await route.fulfill({ body: transparentPng, contentType: "image/png" });
+      } else {
+        await route.continue();
+      }
     },
   );

Since this depends on live network behavior in Playwright, please confirm: does an unmatched page.route pattern fall through to a real network request by default?

Does Playwright page.route only intercept matching URLs and let unmatched requests go to the real network by default?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/projects.spec.ts` around lines 15 - 43, The image interception in the
/projects test only matches two GitHub hostnames, so other seeded project cards
can still make real network requests and make the test flaky. Update the
interception in the projects spec to catch all image requests more broadly in
the test setup around page.route, while still fulfilling them with the same
transparent fixture. Keep the existing assertion on the first card’s
img.project-card-image src pattern, but make sure the route covers any image
source used by the project list.

Source: MCP tools

tests/projectContent.test.ts (2)

19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Regex frontmatter parsing is fragile; prefer a real YAML/frontmatter parser.

The regex /^imageUrl:\s+".+"/m only matches double-quoted values on a single line. Any entry using single quotes, unquoted plain scalars, or a multi-line value would produce a false failure even though the underlying imageUrl field is valid per the Zod schema (z.string().optional() in src/content.config.ts, which doesn't mandate quoting style). Since the goal is to validate frontmatter content, parsing the actual YAML frontmatter (e.g., with gray-matter, already a common Astro-ecosystem dependency) or using Astro's content collection loader directly would be more robust than string matching.

♻️ Suggested approach using gray-matter
-import fs from "node:fs/promises";
+import fs from "node:fs/promises";
+import matter from "gray-matter";
 import path from "node:path";
 import { describe, expect, it } from "vitest";
@@
-      if (!/^imageUrl:\s+".+"/m.test(contents)) {
+      const { data } = matter(contents);
+      if (!data.imageUrl || typeof data.imageUrl !== "string" || data.imageUrl.trim() === "") {
         filesWithoutImages.push(filename);
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/projectContent.test.ts` at line 19, The `tests/projectContent.test.ts`
check for `imageUrl` is using a fragile regex that only accepts double-quoted
single-line values. Replace the string match in this test with real frontmatter
parsing (for example via `gray-matter` or Astro’s content loader) and assert
against the parsed `imageUrl` field instead. Keep the test focused on the
`imageUrl` frontmatter value so it accepts valid YAML forms like single quotes,
plain scalars, or multiline content.

8-25: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Business rule enforced only via test, not the content schema.

src/content.config.ts declares imageUrl: z.string().optional(), but this test asserts every entry must define a non-empty imageUrl. That leaves the mandatory-image requirement enforced only in a separate unit test rather than at the schema/build level, so a future project entry missing imageUrl will fail Astro's own content:sync/build validation only after this test also passes/fails independently, and drift between the two contracts is possible. Consider tightening the Zod schema (e.g., z.string().min(1) without .optional()) so the requirement is enforced where content is actually validated, and this test becomes a secondary safeguard rather than the sole enforcement mechanism.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/projectContent.test.ts` around lines 8 - 25, The mandatory imageUrl
rule is only enforced in the test, while the actual content contract in
content.config.ts still allows imageUrl to be optional. Update the schema used
by the content collection definition (the relevant Zod shape in
src/content.config.ts) so imageUrl is required and non-empty instead of
optional, and keep the projectContent.test.ts check as a backup validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/projectContent.test.ts`:
- Line 19: The `tests/projectContent.test.ts` check for `imageUrl` is using a
fragile regex that only accepts double-quoted single-line values. Replace the
string match in this test with real frontmatter parsing (for example via
`gray-matter` or Astro’s content loader) and assert against the parsed
`imageUrl` field instead. Keep the test focused on the `imageUrl` frontmatter
value so it accepts valid YAML forms like single quotes, plain scalars, or
multiline content.
- Around line 8-25: The mandatory imageUrl rule is only enforced in the test,
while the actual content contract in content.config.ts still allows imageUrl to
be optional. Update the schema used by the content collection definition (the
relevant Zod shape in src/content.config.ts) so imageUrl is required and
non-empty instead of optional, and keep the projectContent.test.ts check as a
backup validation.

In `@tests/projects.spec.ts`:
- Around line 15-43: The image interception in the /projects test only matches
two GitHub hostnames, so other seeded project cards can still make real network
requests and make the test flaky. Update the interception in the projects spec
to catch all image requests more broadly in the test setup around page.route,
while still fulfilling them with the same transparent fixture. Keep the existing
assertion on the first card’s img.project-card-image src pattern, but make sure
the route covers any image source used by the project list.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 71686cc4-70e8-4f31-a7da-18b58a2743fe

📥 Commits

Reviewing files that changed from the base of the PR and between 4a7bbd5 and 7a7c079.

📒 Files selected for processing (18)
  • src/content/projects/common-components.md
  • src/content/projects/create-project-calavera.md
  • src/content/projects/css-benchpress.md
  • src/content/projects/css-community-reset.md
  • src/content/projects/css-custom-property-inspector.md
  • src/content/projects/css-expect.md
  • src/content/projects/css-media-pseudo-polyfill.md
  • src/content/projects/css-property-type-validator.md
  • src/content/projects/css-tree-ast-viewer.md
  • src/content/projects/jsconsole.md
  • src/content/projects/little-demos.md
  • src/content/projects/masonry-gridlanes-wc.md
  • src/content/projects/ossreleasefeed-v2.md
  • src/content/projects/refined-plan-mode.md
  • src/content/projects/skills-autoresearch-flue.md
  • src/content/projects/web-platform-pulse.md
  • tests/projectContent.test.ts
  • tests/projects.spec.ts
✅ Files skipped from review due to trivial changes (13)
  • src/content/projects/css-property-type-validator.md
  • src/content/projects/masonry-gridlanes-wc.md
  • src/content/projects/web-platform-pulse.md
  • src/content/projects/css-community-reset.md
  • src/content/projects/refined-plan-mode.md
  • src/content/projects/create-project-calavera.md
  • src/content/projects/css-benchpress.md
  • src/content/projects/css-media-pseudo-polyfill.md
  • src/content/projects/ossreleasefeed-v2.md
  • src/content/projects/little-demos.md
  • src/content/projects/css-tree-ast-viewer.md
  • src/content/projects/css-custom-property-inspector.md
  • src/content/projects/skills-autoresearch-flue.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/content/projects/jsconsole.md
  • src/content/projects/common-components.md
  • src/content/projects/css-expect.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant