diff --git a/.github/workflows/sync-adventure.yml b/.github/workflows/sync-adventure.yml index 4d143feab..c8b61462f 100644 --- a/.github/workflows/sync-adventure.yml +++ b/.github/workflows/sync-adventure.yml @@ -182,7 +182,8 @@ jobs: ### Before merging - - [ ] Add \`contributor:\` block to \`src/data/adventures/${SLUG}/adventure.yaml\` + - [ ] Check the \`contributor:\` block in \`src/data/adventures/${SLUG}/adventure.yaml\`. It is synced from + \`docs/index.yaml\` in the challenges repo; add it by hand only if the sync log says none was found. \`\`\`yaml contributor: name: "Full Name" diff --git a/ADVENTURES.md b/ADVENTURES.md index e1de41a50..6c865e278 100644 --- a/ADVENTURES.md +++ b/ADVENTURES.md @@ -39,12 +39,12 @@ The authoritative schema is in [`src/content.config.ts`](src/content.config.ts) | `icon` | Optional | Lucide icon name (e.g. `Satellite`) | The sync workflow auto-registers the icon (imports, type union, emoji mapping) and writes `icon:` directly into `adventure.yaml`. Set explicitly in the challenges repo when the emoji alone is insufficient. | | `emoji` | Optional | emoji character | Shown on the adventure card. The sync workflow maps it to a Lucide icon via the `EMOJI_TO_ICON` table; add the mapping there first if the emoji is new. | | `month` | **Required** | `MMM YYYY` | Three-letter uppercase abbreviation + four-digit year. Allowed: `JAN FEB MAR APR MAY JUN JUL AUG SEP OCT NOV DEC`. Validated by Zod regex; wrong format fails sync. | -| `tags` | **Required** | `string[]` (min 1) | Technology/topic labels shown as filter chips. Used when the auto-generated `meta_description` falls back to name + backstory. | -| `meta_description` | **Required** | string, max 160 chars | Validated by the Zod schema; missing field fails `npm run sync`. Max 160 chars. No em dashes; no ` - ` used as a dash. | +| `tags` | **Required** | `string[]` (min 1) | Technology/topic labels for the adventure as a whole. Seeds each level's `topics` at sync time and feeds the auto-generated `meta_description` fallback. **Does not by itself create a filter chip or a `/challenges//` route:** those come from level `topics`, so a tag no level teaches builds nothing. Add the tag to the relevant level's `topics` to make it filterable. | +| `meta_description` | **Required** | string, 160 chars recommended | Validated by the Zod schema; missing field fails `npm run sync`. Over 160 chars logs a warning and still builds, since search engines truncate rather than reject. Trim it before release. No em dashes; no ` - ` used as a dash. | | `story` | Optional | markdown string | Short description shown on adventure cards and at the top of the adventure page. Card views strip HTML; set:html prose uses the rendered version. | | `backstory` | Optional | `string[]` (markdown) | Thematic narrative paragraphs rendered on the adventure page. | | `overview` | Optional | `string[]` (markdown) | Technical/content summary rendered on the adventure page. | -| `contributor` | Optional | object | `name` (required), `url` (optional URL), `about` (optional markdown), `discourse_username` (optional string -- Discourse username used for avatar resolution in community leaderboards). Survives every re-sync once set. | +| `contributor` | Optional | object | `name` (required), `url` (optional URL), `about` (optional markdown), `discourse_username` (optional string -- Discourse username used for avatar resolution in community leaderboards). Copied from the challenges repo's `docs/index.yaml` when absent here; survives every re-sync once set. | | `community_category_id` | Optional | integer | Discourse category ID. Survives every re-sync once set; position is kept directly after `slug`. | | `rewards` | Optional | object | `deadline` (required inside; see format below), `eligibility` (markdown), `tiers` (array of `{label, description}`), `ranking_note` (markdown), `ranking_rules_url` (URL). | | `upcoming_levels` | Optional | object[] | Coming-soon placeholders: `{level?, name, difficulty}`. Survives re-syncs for levels not yet in the challenges repo. | @@ -61,7 +61,7 @@ Each entry in the `levels` array accepts the following fields. | `level` | **Required** | string | Level identifier and URL segment: `beginner`, `intermediate`, or `expert`. | | `name` or `title` | **One required** | string | Display name for the level. | | `devcontainer` | **Required** | string | Devcontainer folder name in the challenges repo `.devcontainer/` directory. | -| `topics` | **Required** | `string[]` | Technologies covered by this level. An empty `[]` is valid and stays empty; inheriting adventure `tags` is done by the sync workflow, not the schema. | +| `topics` | **Required** | `string[]` | Technologies covered by this level, and **the source of truth for the `/challenges/` filter chips and `/challenges//` routes**. A tag page lists exactly the levels carrying that topic, so narrowing this list narrows what the level appears under. An empty `[]` is valid and falls back to the adventure `tags` for filtering; the sync seeds it from them, and inheriting is done by the sync workflow, not the schema. | | `objective` | **Required** | `string[]` (markdown) | Success criteria list shown to participants. | | `toolbox` | **Required** | object[] | `{name, description, url?}` — tools available in the level environment. | | `how_to_play` | **Required** | object[] | `{id?, title, content}` — ordered step-by-step instructions. | @@ -82,12 +82,12 @@ Each entry in the `levels` array accepts the following fields. | `scenario` | Optional | string (markdown) | Scenario prose shown before the how-to-play steps. | | `services` | Optional | object[] | Services exposed by the devcontainer: `{name, port?, url?, credentials?, description, internal?}`. Use `port` for a bare port number or `url` for a full URL (e.g. `http://localhost:5173`). An injected "Explore the UIs" step is generated automatically when at least one non-internal service has a `port` or `url`. | | `helpful_links` | Optional | object[] | Reference links shown at the bottom of the level: `{title, url, description?}`. | -| `meta_description` | Optional | string, max 160 chars | Level-specific meta description. When absent, the generator builds one from `name`/`title` + `intro[0]` + difficulty + topics. | +| `meta_description` | Optional | string, 160 chars recommended | Level-specific meta description. When absent, the generator builds one from `name`/`title` + `intro[0]` + difficulty + topics. Over 160 chars logs a warning and still builds. | | `what_you_learn` or `learnings` | **One required** | `string[]` (min 1 when present) | Learning objectives list. A Zod `.refine()` requires at least one of the two to be set; if both are absent the build fails. | | `verification` | **Required** | object | `{command, description}` — the verification gate command and its description. | | `codespaces_machine` | Optional | `"4core"` | Machine size override for Codespaces. Only `"4core"` is accepted; other values fail the Zod schema. | | `hook` | Optional | string | Verification hook command. | -| `contributor` | Optional | object | Person who built this specific level. Same subfields as the adventure `contributor` (`name`, `url`, `about`, `discourse_username`). **When omitted, the adventure designer is credited as the builder for this level.** When set, takes precedence over the adventure designer for credit display on the level page and in community leaderboard sections. See note below. | +| `contributor` | Optional | object | Person who built this specific level. Same subfields as the adventure `contributor` (`name`, `url`, `about`, `discourse_username`). Synced from that level's YAML in the challenges repo (e.g. `docs/beginner.yaml`), which sets it only when someone other than the designer built the level. A value already in the website YAML always wins, so re-crediting a level upstream needs the same hand-edit as re-crediting the designer. **When omitted, the adventure designer is credited as the builder for this level.** When set, takes precedence over the adventure designer for credit display on the level page and in community leaderboard sections. See note below. | | `solved_count` | Optional | integer | Override for the displayed solved count. | | `top_players` | Optional | object[] | System-populated leaderboard data: `{username, count}`. Set by the leaderboard refresh script; do not edit by hand. | @@ -114,8 +114,9 @@ Go to **Actions → Sync Adventure from Challenges Repo → Run workflow**. 2. If a PR branch (`feat/adventure-`) already exists, restores `adventure.yaml` from that branch so any manual edits already made survive the re-sync. 3. Fetches `docs/index.yaml` and all level YAMLs from the challenges repo. 4. Writes `src/data/adventures//adventure.yaml` and creates `-posts.json` stubs for each new live level. -5. Validates the YAML with `astro sync` (Zod content schema). There is no leaderboard registry to update: `buildAdventureCategories()` in `scripts/refresh-leaderboard.mjs` reads `community_category_id` out of every `adventure.yaml` at runtime, so setting that field (a PR checklist item below) is the whole registration step. Routes and sitemap entries are automatic via `getStaticPaths()` and `src/pages/sitemap.xml.ts`. `public/llms.txt` is updated by hand as part of the PR checklist. -6. Opens (or updates) a PR on `feat/adventure-` with a checklist of steps to complete before merging. +5. Registers the adventure's routes in `e2e/routes.ts` so the `route-coverage.spec.ts` drift gate passes without a manual edit. The `/challenges//` entries are derived from each live level's `topics`, mirroring `getChallengeData()` in `src/lib/challenges.ts`. Adventure `tags` are deliberately not used: a tag no level teaches builds no route, so registering one would list a route that never reaches `dist/` and trip the drift gate from the other side. +6. Validates the YAML with `astro sync` (Zod content schema). There is no leaderboard registry to update: `buildAdventureCategories()` in `scripts/refresh-leaderboard.mjs` reads `community_category_id` out of every `adventure.yaml` at runtime, so setting that field (a PR checklist item below) is the whole registration step. Routes and sitemap entries are automatic via `getStaticPaths()` and `src/pages/sitemap.xml.ts`. `public/llms.txt` is updated by hand as part of the PR checklist. +7. Opens (or updates) a PR on `feat/adventure-` with a checklist of steps to complete before merging. --- @@ -123,7 +124,7 @@ Go to **Actions → Sync Adventure from Challenges Repo → Run workflow**. The PR body lists everything that needs to happen before merging. Here is each item explained. -### Add contributor block +### Check the contributor block ```yaml contributor: @@ -133,7 +134,15 @@ contributor: discourse_username: "their_forum_username" ``` -Add this to `src/data/adventures//adventure.yaml`. The `url`, `about`, and `discourse_username` fields are optional but recommended -- `discourse_username` enables avatar resolution in community leaderboards. Once set, this block survives future re-syncs automatically. +This lives in `src/data/adventures//adventure.yaml`. The sync copies it from `contributor:` in the challenges repo's `docs/index.yaml` when the website YAML does not already have one, keeping only the four fields above -- the challenges repo owns its own schema, and passing an unknown field through would fail `npm run sync` against the strict content schema. Write it by hand only when the sync log warns that no contributor was found upstream. + +`url` must be an absolute URL. A bare domain (`ksick.dev`) is rejected by the content schema, so the sync drops it, keeps the name, and warns. Add the `https://` scheme upstream to restore the link. + +If the challenges repo names no designer but a level YAML names a builder, the sync fails early with a message pointing at `docs/index.yaml`. Left to the content schema that failure lands in the next workflow step, before the PR branch exists, so there is nothing to hand-fix. + +The `url`, `about`, and `discourse_username` fields are optional but recommended -- `discourse_username` enables avatar resolution in community leaderboards. A hand-edited block always wins over the upstream one and survives future re-syncs. + +An adventure with no designer cannot have levels that name their own builder: `creditIntegrityError` in `src/lib/adventure-credit.ts` fails `npm run sync` on that combination. ### Confirm month @@ -242,7 +251,8 @@ If the challenges repo is updated while your PR is still open, or you want to pr | Field | Preserved | Notes | | --- | --- | --- | -| `contributor:` (adventure) | Always | Survives every re-sync once set | +| `contributor:` (adventure) | Always | Survives every re-sync once set. When unset, the sync copies it from `docs/index.yaml` in the challenges repo | +| `contributor:` (level) | Always | Survives every re-sync once set, even if the level's YAML upstream names someone else. When unset, the sync copies it from that level's YAML in the challenges repo. Re-crediting a level is a deliberate hand-edit | | `community_category_id:` (adventure) | Always | Survives every re-sync once set; position is kept directly after `slug` | | `month:` (adventure) | Always | Survives every re-sync once set | | `discussion_url:` / `community_url:` (level) | Always | Website-only fields; never in the challenges repo. Both field aliases are preserved independently | diff --git a/e2e/routes.ts b/e2e/routes.ts index e8d2e4014..afb0129b1 100644 --- a/e2e/routes.ts +++ b/e2e/routes.ts @@ -94,7 +94,7 @@ export const A11Y_PAGES: string[] = [ * gate in route-coverage.spec.ts. */ export const ROUTES_WITHOUT_FULL_COVERAGE: string[] = [ - // 24 remaining /challenges// routes. They are the same page component with + // 23 remaining /challenges// routes. They are the same page component with // a different filter seed, so /challenges/opentelemetry/ is the representative. // Listed explicitly rather than pattern-matched so a new tag still has to be // acknowledged here. @@ -120,6 +120,5 @@ export const ROUTES_WITHOUT_FULL_COVERAGE: string[] = [ "/challenges/python/", "/challenges/spring-boot/", "/challenges/tdd/", - "/challenges/terraform/", "/challenges/trivy/", ]; diff --git a/scripts/sync-adventure.mjs b/scripts/sync-adventure.mjs index cc51aefb0..d0f1521d4 100644 --- a/scripts/sync-adventure.mjs +++ b/scripts/sync-adventure.mjs @@ -152,6 +152,104 @@ function buildAdventureMetaDescription(indexData, activeLevels) { return truncateAtWord(full, 160); } +// `contributor.url` is `z.url()` in the content schema, which rejects a bare +// domain like "ksick.dev". The challenges repo has its own schema and no reason +// to match ours, so an unvalidated copy fails `npm run sync` in the very next +// workflow step, before the PR branch exists for anyone to hand-fix. +// `new URL()` is the same gate `z.url()` applies. +function usableUrl(value, where) { + if (!value) return null; + try { + new URL(value); + return value; + } catch { + console.warn( + ` ${where}: contributor url "${value}" is not an absolute URL, which the website schema requires. ` + + "Dropped the link and kept the name. Add a scheme (https://) upstream to restore it." + ); + return null; + } +} + +/** + * Narrows an upstream contributor block to the four fields the website's strict + * contributor schema accepts. The challenges repo owns its own schema, so a + * field this site has no column for would otherwise fail `astro sync`. + * + * Absent returns null quietly. Present-but-unusable warns first: dropping it in + * silence re-credits the work to the adventure designer via the level fallback, + * which renders as a confident statement about who built something. + */ +export function pickContributor(raw, where = "contributor") { + if (raw == null) return null; + if (typeof raw !== "object" || Array.isArray(raw)) { + console.warn(` ${where}: contributor is ${Array.isArray(raw) ? "a list" : typeof raw}, expected a block with a \`name\`. Ignored.`); + return null; + } + if (!raw.name) { + console.warn(` ${where}: contributor block has no \`name\`, which the website schema requires. Ignored.`); + return null; + } + const url = usableUrl(raw.url, where); + return { + name: raw.name, + ...(url && { url }), + ...(raw.about && { about: raw.about }), + ...(raw.discourse_username && { discourse_username: raw.discourse_username }), + }; +} + +// Mirrors `tagToSlug` in src/lib/challenges.ts, which builds the real route +// params. Held in step by a parity test; the script cannot import the TS module. +export const tagToSlug = (tag) => + tag.toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/(^-|-$)/g, ""); + +/** + * Every tag that generates a /challenges// route for this adventure. + * + * Mirrors `getChallengeData` in src/lib/challenges.ts, which derives the tag set + * from each live level's `topics`. Adventure `tags` are deliberately NOT included: + * a tag no level teaches builds no route there, so registering it here would list + * a route that never reaches `dist/` and trip the drift gate from the other side. + * + * The `adventureTags` fallback matches the same fallback in `getChallengeData`, + * which applies only to a level carrying no topics of its own. + */ +export function challengeTagsOf(adventureTags, levels) { + const perLevel = (levels || []).map((l) => { + const topics = (l.topics ?? []) + .map((t) => (typeof t === "string" ? t : (t?.name ?? ""))) + .filter(Boolean); + return topics.length > 0 ? topics : (adventureTags || []); + }); + return [...new Set(perLevel.flat().filter(Boolean))]; +} + +/** + * Levels may only name their own builder on an adventure that names a designer. + * + * This is the same rule as `creditIntegrityError` in src/lib/adventure-credit.ts, + * which the content schema enforces. Checking it here too is not redundant: left + * to the schema it fails in the next workflow step ("Validate adventure YAML"), + * which runs before the PR is created, so the run goes red with no branch for + * anyone to hand-fix and an Astro error naming a file the reviewer cannot reach. + * Failing here names the upstream file that actually needs the edit. The two are + * held in step by a parity test, since this script is plain ESM and cannot import + * the TypeScript module. + * + * Returns the error message, or null when the adventure is valid. + */ +export function missingDesignerError(contributor, levels, indexPath) { + if (contributor) return null; + const named = (levels || []).filter((l) => l.contributor).map((l) => l.level); + if (named.length === 0) return null; + return ( + `Level(s) ${named.join(", ")} name their own \`contributor\` but the adventure has no designer.\n` + + "Every adventure needs a designer before its levels can credit separate builders.\n" + + `Add a \`contributor:\` block to ${indexPath}, then re-run this sync.` + ); +} + function transformStrings(value, fn) { if (typeof value === "string") return fn(value); if (Array.isArray(value)) return value.map((v) => transformStrings(v, fn)); @@ -268,14 +366,20 @@ function addToLucideIconsMap(content, iconName, kebab) { return content.replace(re, m[1] + block + m[3]); } -function buildLevel(raw, adventureTags, rewardsDeadline) { +export function buildLevel(raw, adventureTags, rewardsDeadline) { // architecture_diagram is stripped here. After all levels are fetched, the sync attempts // to pull the SVG from docs/diagrams/ in the challenges repo and re-adds the field if // successful. If not found there, it must be added manually to src/assets/diagrams/. const { architecture_diagram: _ignored, ...rest } = raw; - const cleaned = transformStrings(rest, stripCodeInLinks); + const { contributor: rawContributor, ...cleaned } = transformStrings(rest, stripCodeInLinks); + // A level `contributor` is the challenge builder, set upstream only when someone + // other than the adventure designer built this level. Filtered through the same + // picker as the designer: the website's contributor schema is strict, so a field + // the challenges repo carries but this site has no column for would fail sync. + const contributor = pickContributor(rawContributor, `level "${raw.level}" upstream`); return { ...cleaned, + ...(contributor && { contributor }), ...(cleaned.deadline && { deadline: parseDeadline(cleaned.deadline, PRESERVE_TZ) }), topics: cleaned.topics || deriveTopics(adventureTags), verification: cleaned.verification || VERIFICATION_STUB, @@ -295,7 +399,7 @@ function buildLevel(raw, adventureTags, rewardsDeadline) { * injected by buildLevel, so manual edits are only preserved when * the upstream did not intentionally change the field. */ -function mergeLevels(existing, incoming, rawFetched) { +export function mergeLevels(existing, incoming, rawFetched) { const levelMap = Object.fromEntries((existing || []).map((l) => [l.level, l])); const rawMap = Object.fromEntries((rawFetched || []).map((l) => [l.level, l])); @@ -303,6 +407,18 @@ function mergeLevels(existing, incoming, rawFetched) { const prev = levelMap[l.level]; const raw = rawMap[l.level]; + // The challenge builder, already credited in the website YAML. Held aside + // because it wins over the upstream value rather than only filling a gap. + const preservedContributor = pickContributor(prev?.contributor, `level "${l.level}" in adventure.yaml`); + const upstreamContributor = pickContributor(raw?.contributor, `level "${l.level}" upstream`); + if (preservedContributor && upstreamContributor && upstreamContributor.name !== preservedContributor.name) { + console.warn( + ` Level "${l.level}": keeping the builder already credited on the website ` + + `(${preservedContributor.name}) over the one named upstream (${upstreamContributor.name}). ` + + "Edit adventure.yaml by hand to re-credit this level." + ); + } + levelMap[l.level] = { ...l, // Preserve the discussion URL set by the add-discussion-url workflow. @@ -320,6 +436,12 @@ function mergeLevels(existing, incoming, rawFetched) { // intentional upstream changes come through. When the upstream did not set them (buildLevel // derived them from adventure tags), preserve any manual refinements from the website. ...(!raw?.topics && prev?.topics && { topics: prev.topics }), + // contributor: the challenge builder. A builder already credited on the website + // always wins, matching the adventure designer: the sync fills this field in from + // the level YAML, it never overwrites it. Re-crediting a level to someone else is + // a hand-edit either way, and losing an existing credit silently misattributes a + // person's work, which is worse than a stale credit a reviewer can see and fix. + ...(preservedContributor && { contributor: preservedContributor }), }; } @@ -471,6 +593,30 @@ async function main() { // Used for level flags and e2e/routes.ts generation below. const allLiveLevels = mergeLevels(existing?.levels, activeLevels, rawFetchedLevels); + // The adventure designer. Same rule as the level builder: a designer already + // credited on the website wins, and the sync only fills the field in. + const websiteDesigner = pickContributor(existing?.contributor, "adventure.yaml"); + const upstreamDesigner = pickContributor(indexData.contributor, "docs/index.yaml"); + const resolvedContributor = websiteDesigner ?? upstreamDesigner; + + if (websiteDesigner && upstreamDesigner && websiteDesigner.name !== upstreamDesigner.name) { + console.warn( + ` Keeping the designer already credited in adventure.yaml (${websiteDesigner.name}) over the one named ` + + `in docs/index.yaml (${upstreamDesigner.name}). Edit adventure.yaml by hand to re-credit the adventure.` + ); + } else if (!websiteDesigner && upstreamDesigner) { + console.log(` Designer from docs/index.yaml: ${upstreamDesigner.name}`); + } else if (!resolvedContributor) { + console.warn(" No designer found in docs/index.yaml. Add a `contributor:` block there, or to adventure.yaml in the PR."); + } + + const designerError = missingDesignerError( + resolvedContributor, + allLiveLevels, + `${adventurePath}/docs/index.yaml in ${repo}`, + ); + if (designerError) fail(designerError); + // Build the combined adventure object using challenges repo field names. // The generator accepts all aliases (name/title, emoji → icon, etc.). const adventure = { @@ -494,8 +640,11 @@ async function main() { ...(indexData.rewards.deadline && { deadline: parseDeadline(indexData.rewards.deadline, PRESERVE_TZ) }), }, }), - // Preserve contributor set by a reviewer; omit otherwise (PR checklist item) - ...(existing?.contributor && { contributor: existing.contributor }), + // A reviewer's hand-edited contributor wins; otherwise take the designer the + // challenges repo already names in docs/index.yaml. Levels may credit their own + // builder, and the content schema rejects that on an adventure with no designer, + // so an unsynced upstream contributor fails the build rather than just losing a pill. + ...(resolvedContributor && { contributor: resolvedContributor }), ...(upcomingLevels.length > 0 && { upcoming_levels: upcomingLevels }), levels: allLiveLevels, }; @@ -558,15 +707,7 @@ async function main() { // ROUTES_WITHOUT_FULL_COVERAGE (or fully covered). We upsert a GENERATED block // so new tags are automatically registered; duplicates with the manual list or // other adventure blocks are harmless (Set-deduplicated at test time). - const tagToSlug = (tag) => - tag.toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/(^-|-$)/g, ""); - const allAdventureTags = [ - ...new Set( - allLiveLevels - .flatMap((l) => (l.topics ?? []).map((t) => (typeof t === "string" ? t : (t.name ?? "")))) - .filter(Boolean), - ), - ]; + const allAdventureTags = challengeTagsOf(adventureTags, allLiveLevels); if (allAdventureTags.length > 0) { const challengeBlockStart = ` // GENERATED:${slug}-challenges`; const challengeBlockEnd = ` // /GENERATED:${slug}-challenges`; diff --git a/src/components/ChallengesFilter.astro b/src/components/ChallengesFilter.astro index 9bd1e7506..488b754ff 100644 --- a/src/components/ChallengesFilter.astro +++ b/src/components/ChallengesFilter.astro @@ -35,7 +35,7 @@ const { entries, tags, base, initialTag, adventureCount, embedded, seeAllHref } const activeTags = initialTag ? [initialTag] : []; const hasFilters = activeTags.length > 0; const matches = (e: ChallengeEntry): boolean => - activeTags.length === 0 || activeTags.some((t) => e.adventureTags.includes(t)); + activeTags.length === 0 || activeTags.some((t) => e.topics.includes(t)); const matchedCount = entries.filter(matches).length; const countText = hasFilters @@ -253,7 +253,7 @@ const dropdownItem = class="contents" data-level-card data-difficulty={e.difficulty} - data-tags={e.adventureTags.map(tagToSlug).join(" ")} + data-tags={e.topics.map(tagToSlug).join(" ")} hidden={!matches(e)} > META_DESCRIPTION_MAX) { + console.warn( + `[content] ${where}: meta_description is ${value.length} chars, over the ${META_DESCRIPTION_MAX}-char SEO limit. Search engines will truncate it.`, + ); + } +} + function requireEither(a: string | undefined | null, b: string | undefined | null, field: string): string { const value = a ?? b; if (value != null && value !== "") return value; @@ -187,7 +202,8 @@ function assertDifficulty( // AdventureLevel (from data/adventures/types.ts) is the single source of truth // for the rendered level shape. renderLevel's return type is checked against it, // so the two cannot drift silently. -async function renderLevel(level: z.infer): Promise { +async function renderLevel(level: z.infer, slug: string): Promise { + warnIfMetaDescriptionLong(level.meta_description, `${slug} → level "${level.level}"`); const difficulty = level.difficulty ?? (level.emoji ? LEVEL_DIFFICULTY_BY_EMOJI[level.emoji as keyof typeof LEVEL_DIFFICULTY_BY_EMOJI] : undefined); const learnings = level.learnings ?? level.what_you_learn ?? []; const intro = level.intro ?? (level.summary ? [level.summary] : undefined); @@ -345,7 +361,8 @@ const adventures = defineCollection({ tags: z.array(z.string()).min(1), contributor: contributorSchema.optional(), community_category_id: z.number().int().optional(), - meta_description: z.string().max(160), + // Length is warned about in the transform, not enforced: see warnIfMetaDescriptionLong. + meta_description: z.string(), backstory: z.array(z.string()).optional(), overview: z.array(z.string()).optional(), rewards: rewardsSchema.optional(), @@ -361,6 +378,7 @@ const adventures = defineCollection({ if (message) ctx.addIssue({ code: "custom", message, path: ["contributor"] }); }) .transform(async (data) => { + warnIfMetaDescriptionLong(data.meta_description, `adventure "${data.slug}"`); const title = requireEither(data.title, data.name, "adventure title/name"); const story = data.story ?? data.meta_description ?? (data.backstory && data.backstory.length > 0 ? data.backstory[0] : ""); @@ -370,7 +388,7 @@ const adventures = defineCollection({ mdToInline(story), data.contributor?.about ? mdToInline(data.contributor.about) : Promise.resolve(null), data.backstory ? mdToInlineArray(data.backstory) : Promise.resolve(null), - Promise.all(data.levels.map(renderLevel)), + Promise.all(data.levels.map((level) => renderLevel(level, data.slug))), data.rewards ? renderRewards(data.rewards) : Promise.resolve(null), ]); diff --git a/src/data/adventures/building-cloudhaven/adventure.yaml b/src/data/adventures/building-cloudhaven/adventure.yaml index 2ace909a1..e691383a6 100644 --- a/src/data/adventures/building-cloudhaven/adventure.yaml +++ b/src/data/adventures/building-cloudhaven/adventure.yaml @@ -9,7 +9,6 @@ story: Join the Infrastructure Guild and modernize CloudHaven's infrastructure f OpenTofu and GitHub Actions. tags: - OpenTofu - - Terraform - GitHub Actions - Trivy - TDD diff --git a/src/data/adventures/the-ai-observatory/adventure.yaml b/src/data/adventures/the-ai-observatory/adventure.yaml index 278226ce7..6153b5f54 100644 --- a/src/data/adventures/the-ai-observatory/adventure.yaml +++ b/src/data/adventures/the-ai-observatory/adventure.yaml @@ -34,6 +34,7 @@ levels: - OpenTelemetry - OpenLLMetry - Jaeger + - Python learnings: - Instrument Python AI apps with OpenLLMetry - Analyze traces in Jaeger @@ -116,6 +117,7 @@ levels: - OpenLLMetry - Jaeger - Prometheus + - Python learnings: - Instrument RAG pipelines with OpenLLMetry - Create custom OpenTelemetry metrics in Python @@ -221,6 +223,7 @@ levels: - OpenTelemetry - OpenLLMetry - Jaeger + - Python learnings: - OpenTelemetry GenAI semantic conventions - Tail sampling in the OTel Collector diff --git a/src/lib/adventure-credit.ts b/src/lib/adventure-credit.ts index 7f96afe74..c8785169c 100644 --- a/src/lib/adventure-credit.ts +++ b/src/lib/adventure-credit.ts @@ -103,8 +103,9 @@ export type PillCredit = { * An adventure with no designer has no level builders either, because the * content schema rejects that combination (see `creditIntegrityError`), so this * returns null rather than promoting a builder into the pill. The no-designer - * case is real: `sync-adventure.mjs` deliberately omits `contributor`, and a - * reviewer adds it as a PR checklist item. + * case is still real: `sync-adventure.mjs` copies `contributor` from the + * challenges repo's `docs/index.yaml`, so an adventure whose upstream names no + * designer yet arrives without one until a reviewer adds it. */ export function adventurePillCredit(adventure: CreditSource): PillCredit | null { const designer = adventure.contributor; diff --git a/src/lib/challenges.ts b/src/lib/challenges.ts index bcccb1764..d178d8737 100644 --- a/src/lib/challenges.ts +++ b/src/lib/challenges.ts @@ -1,6 +1,6 @@ // Derives the flat, filterable level list and the tag set for /challenges, // from the adventures collection. -// tags are ADVENTURE tags (OR match); levels are then filtered by difficulty. +// Tags are per-LEVEL topics (OR match); levels are then filtered by difficulty. import { type Difficulty } from "@/lib/difficulty"; @@ -89,7 +89,14 @@ export type ChallengeEntry = { estimatedTime?: string; adventureId: string; adventureTitle: string; - adventureTags: string[]; + /** + * What this specific challenge teaches, and what a /challenges// page + * matches on. Named for the level, not the adventure, because they differ: an + * adventure tag can belong to one level (or to a level that is not live yet), + * and matching on the adventure's tags listed every challenge in it, including + * ones that never touch the tech. + */ + topics: string[]; adventureIcon?: string; isLive: boolean; url: string; @@ -126,12 +133,19 @@ export function getChallengeData(adventures: AdventureData[]): { estimatedTime: level.estimatedTime, adventureId: a.slug, adventureTitle: a.title, - adventureTags: a.tags, + // Fall back to the adventure tags only when a level carries no topics of + // its own, which the sync never produces (it seeds them from the adventure + // tags). Without the guard a topic-less level would drop out of every tag + // page instead of degrading to the old, broader behaviour. + topics: level.topics.length > 0 ? level.topics : a.tags, adventureIcon: a.icon, isLive: live, url: `/adventures/${a.slug}/levels/${level.id}/`, })); }); - const tags = Array.from(new Set(adventures.flatMap((a) => a.tags))).sort(); + // Derived from the entries, so a tag exists exactly when some challenge + // teaches it. An adventure tag that no live level carries builds no route + // rather than an empty or misleading one; it appears when that level ships. + const tags = Array.from(new Set(entries.flatMap((e) => e.topics))).sort(); return { entries, tags }; } diff --git a/src/test/lib/adventure-credit.test.ts b/src/test/lib/adventure-credit.test.ts index 32159e0f3..f3aa2c980 100644 --- a/src/test/lib/adventure-credit.test.ts +++ b/src/test/lib/adventure-credit.test.ts @@ -229,7 +229,8 @@ describe("creditIntegrityError", () => { expect(creditIntegrityError(adventure("a", KAT, [undefined, SIMON, undefined]))).toBeNull(); }); - // The freshly-synced state: sync-adventure.mjs omits `contributor` and a + // The freshly-synced state when the challenges repo names no designer in + // docs/index.yaml yet: sync-adventure.mjs emits no `contributor` and a // reviewer adds it later, so no-designer-and-no-builders must stay valid. it("passes an adventure with neither, which is what sync-adventure emits", () => { expect(creditIntegrityError(adventure("a", undefined))).toBeNull(); diff --git a/src/test/lib/challenges.test.ts b/src/test/lib/challenges.test.ts index c8ece1503..bd92da6bf 100644 --- a/src/test/lib/challenges.test.ts +++ b/src/test/lib/challenges.test.ts @@ -271,11 +271,26 @@ describe("getChallengeData", () => { expect(entries[0].url).toBe("/adventures/my-slug/levels/level-1/"); }); - it("includes the adventureTags array on the entry", () => { + it("takes the entry topics from the level, not the adventure", () => { + // A tag page matches on this, so an adventure tag the level does not teach + // must not appear here: that listed challenges that never touch the tech. const { entries } = getChallengeData([ - makeAdventure({ tags: ["rust", "cloud"] }), + makeAdventure({ + tags: ["rust", "cloud"], + levels: [{ id: "l1", name: "L1", difficulty: "Beginner", topics: ["rust"] }], + }), + ]); + expect(entries[0].topics).toEqual(["rust"]); + }); + + it("falls back to the adventure tags when a level carries no topics", () => { + const { entries } = getChallengeData([ + makeAdventure({ + tags: ["rust", "cloud"], + levels: [{ id: "l1", name: "L1", difficulty: "Beginner", topics: [] }], + }), ]); - expect(entries[0].adventureTags).toEqual(["rust", "cloud"]); + expect(entries[0].topics).toEqual(["rust", "cloud"]); }); it("copies the icon from the adventure", () => { @@ -414,7 +429,7 @@ describe("getChallengeData", () => { expect(entry).toHaveProperty("learnings"); expect(entry).toHaveProperty("adventureId"); expect(entry).toHaveProperty("adventureTitle"); - expect(entry).toHaveProperty("adventureTags"); + expect(entry).toHaveProperty("topics"); expect(entry).toHaveProperty("isLive"); expect(entry).toHaveProperty("url"); }); diff --git a/src/test/scripts/sync-adventure.test.ts b/src/test/scripts/sync-adventure.test.ts index 35b948c9b..b71c195ee 100644 --- a/src/test/scripts/sync-adventure.test.ts +++ b/src/test/scripts/sync-adventure.test.ts @@ -15,8 +15,19 @@ * schema, providing a second revert-catch that does not depend on the clock. */ -import { describe, it, expect, vi, afterEach } from "vitest"; -import { currentMonth } from "../../../scripts/sync-adventure.mjs"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { z } from "zod"; +import { + buildLevel, + challengeTagsOf, + currentMonth, + mergeLevels, + missingDesignerError, + pickContributor, + tagToSlug, +} from "../../../scripts/sync-adventure.mjs"; +import { creditIntegrityError } from "@/lib/adventure-credit"; +import { tagToSlug as canonicalTagToSlug } from "@/lib/challenges"; const MONTH_SCHEMA = /^[A-Z]{3} \d{4}$/; @@ -54,3 +65,253 @@ describe("currentMonth", () => { expect(result).not.toMatch(MONTH_SCHEMA); }); }); + +describe("pickContributor", () => { + let warn: ReturnType; + + beforeEach(() => { + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warn.mockRestore(); + }); + + it("keeps the four fields the content schema accepts", () => { + expect( + pickContributor({ + name: "Ada Lovelace", + url: "https://example.com", + about: "Writes notes.", + discourse_username: "ada", + }), + ).toEqual({ + name: "Ada Lovelace", + url: "https://example.com", + about: "Writes notes.", + discourse_username: "ada", + }); + expect(warn).not.toHaveBeenCalled(); + }); + + it("drops fields the strict content schema would reject", () => { + // The challenges repo owns its own index.yaml schema and may carry fields + // this site has no column for. Passing them through fails `astro sync`. + const result = pickContributor({ name: "Ada Lovelace", github: "ada", avatar: "a.png" }); + expect(result).toEqual({ name: "Ada Lovelace" }); + }); + + it("omits absent optional fields rather than emitting empty values", () => { + expect(pickContributor({ name: "Ada Lovelace", url: "", about: undefined })).toEqual({ + name: "Ada Lovelace", + }); + }); + + it.each([ + ["null", null], + ["undefined", undefined], + ])("returns null for %s without warning", (_label, input) => { + expect(pickContributor(input)).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + }); + + it.each([ + ["a block with no name", { url: "https://example.com" }], + ["a non-object", "Ada Lovelace"], + ["a list", [{ name: "Ada Lovelace" }]], + ])("returns null for %s and says so", (_label, input) => { + // Silence here would re-credit the level to the designer via the fallback, + // and print "no contributor found", both stated as fact. + expect(pickContributor(input, "docs/index.yaml")).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain("docs/index.yaml"); + }); + + describe("url validation", () => { + // `contributor.url` is `z.url()` in the content schema. An unusable value + // would fail `npm run sync` in the next workflow step, before a PR branch + // exists to hand-fix, so it is dropped here with the name kept. + it.each([ + ["a bare domain", "ksick.dev"], + ["a www host with no scheme", "www.example.com"], + ["a relative path", "/about"], + ])("drops %s and keeps the name", (_label, url) => { + expect(pickContributor({ name: "Ada Lovelace", url })).toEqual({ name: "Ada Lovelace" }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain(url); + }); + + it.each([ + ["https", "https://example.com"], + ["http", "http://example.com"], + ["a path and query", "https://example.com/a?b=1#c"], + ])("keeps a valid %s url", (_label, url) => { + expect(pickContributor({ name: "Ada Lovelace", url })).toEqual({ name: "Ada Lovelace", url }); + expect(warn).not.toHaveBeenCalled(); + }); + + it("agrees with the z.url() gate the content schema applies", () => { + // Parity check: if Zod's rule and this one drift, the sync starts emitting + // YAML that fails validation again, which is the bug this guards. + const schema = z.url(); + for (const url of ["https://a.dev", "ksick.dev", "http://a.b", "www.x.com", "mailto:a@b.c", "/rel"]) { + const kept = pickContributor({ name: "N", url })?.url !== undefined; + expect(kept).toBe(schema.safeParse(url).success); + } + }); + }); +}); + +describe("buildLevel contributor (the challenge builder)", () => { + // The challenges repo puts the designer in docs/index.yaml and, only when a + // different person built a level, a `contributor:` in that level's YAML. When + // designer and builder are the same person the level YAML carries no + // contributor at all and the site falls back to the designer. + const base = { level: "beginner", topics: ["a11y"], verification: { command: "./v.sh", description: "d" } }; + + it("carries a level builder through from the level YAML", () => { + const result = buildLevel({ ...base, contributor: { name: "Grace Hopper", url: "https://example.com" } }, ["a11y"]); + expect(result.contributor).toEqual({ name: "Grace Hopper", url: "https://example.com" }); + }); + + it("drops builder fields the strict content schema would reject", () => { + const result = buildLevel({ ...base, contributor: { name: "Grace Hopper", github: "grace" } }, ["a11y"]); + expect(result.contributor).toEqual({ name: "Grace Hopper" }); + }); + + it("emits no contributor when the level YAML has none, so the designer is credited", () => { + const result = buildLevel({ ...base }, ["a11y"]); + expect(result).not.toHaveProperty("contributor"); + }); +}); + +describe("challengeTagsOf", () => { + it("omits an adventure tag that no level teaches", () => { + // The #243 case. "Guidepup Virtual Screen Reader" belongs to an intermediate + // level that is not live yet, so it must build no route: registering one for + // a route the build never emits trips the drift gate from the other side. + const tags = challengeTagsOf( + ["Accessibility", "Guidepup Virtual Screen Reader", "Playwright"], + [{ level: "beginner", topics: ["Playwright"] }], + ); + expect(tags).toEqual(["Playwright"]); + }); + + it("reports the union of the topics levels do teach", () => { + expect( + challengeTagsOf(["A"], [{ level: "l1", topics: ["B"] }, { level: "l2", topics: ["C"] }]), + ).toEqual(["B", "C"]); + }); + + it("deduplicates a topic two levels share", () => { + expect(challengeTagsOf([], [{ level: "l1", topics: ["A"] }, { level: "l2", topics: ["A"] }])).toEqual(["A"]); + }); + + it("accepts object-shaped topics and drops empty entries", () => { + expect(challengeTagsOf([], [{ level: "l", topics: [{ name: "A" }, { name: "" }, ""] }])).toEqual(["A"]); + }); + + it.each([ + ["a level carries no topics", ["A"], [{ level: "l" }]], + ["a level has an empty topics list", ["A"], [{ level: "l", topics: [] }]], + ])("falls back to adventure tags when %s", (_label, adventureTags, levels) => { + // Same fallback as getChallengeData, so a topic-less level degrades to the + // old broader behaviour instead of dropping out of every tag page. + expect(challengeTagsOf(adventureTags, levels)).toEqual(["A"]); + }); + + it("reports nothing when there are no levels", () => { + expect(challengeTagsOf(["A"], [])).toEqual([]); + }); + + it("slugs tags the same way the route params are built", () => { + // Drift against src/lib/challenges.ts would register a route under a slug the + // build never emits, leaving the real one unregistered and CI red. + for (const tag of ["Accessibility", "Guidepup Virtual Screen Reader", "ArgoCD", "C++", " Trim Me "]) { + expect(tagToSlug(tag)).toBe(canonicalTagToSlug(tag)); + } + }); +}); + +describe("missingDesignerError", () => { + const lvls = (...contributors: (object | undefined)[]) => + contributors.map((c, i) => ({ level: `l${i}`, ...(c && { contributor: c }) })); + + it("fires when a level names a builder but the adventure has no designer", () => { + const msg = missingDesignerError(undefined, lvls({ name: "Ada" }, undefined), "docs/index.yaml in some/repo"); + expect(msg).toContain("l0"); + expect(msg).toContain("docs/index.yaml in some/repo"); + }); + + it("names every offending level, not just the first", () => { + const msg = missingDesignerError(undefined, lvls({ name: "Ada" }, undefined, { name: "Grace" }), "p"); + expect(msg).toContain("l0, l2"); + }); + + it.each([ + ["a designer is set", { name: "Ada" }, lvls({ name: "Grace" })], + ["no level names a builder", undefined, lvls(undefined, undefined)], + ["there are no levels", undefined, []], + ])("passes when %s", (_label, contributor, levels) => { + expect(missingDesignerError(contributor, levels, "p")).toBeNull(); + }); + + it("agrees with creditIntegrityError, the rule the content schema enforces", () => { + // The script cannot import the TypeScript module, so the rule is stated twice. + // Drift means the sync writes YAML that then fails validation, which is exactly + // the failure this gate exists to pre-empt. + const cases: { contributor?: { name: string }; levels: { contributor?: unknown }[] }[] = [ + { contributor: undefined, levels: lvls({ name: "Ada" }) }, + { contributor: undefined, levels: lvls(undefined) }, + { contributor: { name: "Ada" }, levels: lvls({ name: "Grace" }) }, + { contributor: { name: "Ada" }, levels: lvls(undefined) }, + { contributor: undefined, levels: [] }, + ]; + let fired = 0; + for (const { contributor, levels } of cases) { + const scriptFires = missingDesignerError(contributor, levels, "p") !== null; + const schemaFires = creditIntegrityError({ slug: "s", contributor, levels }) !== null; + expect(scriptFires).toBe(schemaFires); + if (scriptFires) fired++; + } + // Both agreeing on "never fires" would satisfy the loop without testing anything. + expect(fired).toBeGreaterThan(0); + }); +}); + +describe("mergeLevels contributor preservation", () => { + const GRACE = { name: "Grace Hopper", url: "https://example.com" }; + const ADA = { name: "Ada Lovelace" }; + const lvl = (extra: object = {}) => ({ level: "beginner", topics: ["a11y"], ...extra }); + + it("keeps a builder already credited on the website when upstream names none", () => { + const merged = mergeLevels([lvl({ contributor: GRACE })], [lvl()], [lvl()]); + expect(merged[0].contributor).toEqual(GRACE); + }); + + it("keeps the website builder even when upstream names a different one, and says so", () => { + // Losing an existing credit silently misattributes someone's work. Re-crediting + // a level is a deliberate hand-edit, the same rule as the adventure designer. + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const merged = mergeLevels([lvl({ contributor: GRACE })], [lvl({ contributor: ADA })], [lvl({ contributor: ADA })]); + expect(merged[0].contributor).toEqual(GRACE); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain("Grace Hopper"); + expect(warn.mock.calls[0][0]).toContain("Ada Lovelace"); + warn.mockRestore(); + }); + + it("takes the upstream builder when the website has none", () => { + const merged = mergeLevels([lvl()], [lvl({ contributor: ADA })], [lvl({ contributor: ADA })]); + expect(merged[0].contributor).toEqual(ADA); + }); + + it("leaves a level with no builder on either side uncredited, so the designer is credited", () => { + const merged = mergeLevels([lvl()], [lvl()], [lvl()]); + expect(merged[0]).not.toHaveProperty("contributor"); + }); + + it("adds a builder to a level that is brand new to the website", () => { + const merged = mergeLevels([], [lvl({ contributor: ADA })], [lvl({ contributor: ADA })]); + expect(merged[0].contributor).toEqual(ADA); + }); +});