diff --git a/.claude/skills/docs-update/SKILL.md b/.claude/skills/docs-update/SKILL.md index bcfc7e914a..81086441be 100644 --- a/.claude/skills/docs-update/SKILL.md +++ b/.claude/skills/docs-update/SKILL.md @@ -34,9 +34,16 @@ Read `.github/docs-config.json` and extract: - `reviewers` — GitHub usernames to request review from - `featureAreas` — array of `{ id, description, routes? }` objects -### 0a. Build the beta flag set from `src/flags.ts` +### 0a. Build the flag sets from `src/flags.ts` -Read `src/flags.ts` using the Read tool. Parse the `ExistingFlags` object to extract the set of **beta flag IDs** — any flag entry where `category` is `"beta"`, regardless of its `default` value. For example, given: +Read `src/flags.ts` using the Read tool. Parse the `ExistingFlags` object and split every entry into two sets based on `category` **and** `default`: + +- **Gated beta flags** — `category: "beta"` and `default: false`. The feature is off for everyone who has not opted in, so it is unfinished from a user's perspective and must not be documented. +- **Released flags** — everything else: `category: "beta"` with `default: true` (on for everyone, users may opt out), plus every non-beta category. These are documentable. + +A `category: "beta"` flag flipped to `default: true` is a release, not an experiment. It is what every user sees on load, so the docs must describe it — waiting for an explicit graduation (category flip or flag removal) would leave the shipped behaviour undocumented indefinitely. + +For example, given: ```ts ["dashboard"]: { category: "beta", default: true, ... } @@ -44,11 +51,13 @@ Read `src/flags.ts` using the Read tool. Parse the `ExistingFlags` object to ext ["redirect-on-new-pipeline-run"]: { category: "setting", default: false, ... } ``` -The beta flag set would be `{ "dashboard", "github-component-library" }`. +The gated beta flag set is `{ "github-component-library" }` and the released flag set is `{ "dashboard", "redirect-on-new-pipeline-run" }`. + +Throughout the rest of this skill, "beta" as a suppression signal means **gated beta** — a beta flag with `default: true` never suppresses documentation. ### 0b. Discover route-level beta gating from the router -Read `src/routes/router.ts` using the Read tool. Scan its contents for all `isFlagEnabled("...")` calls and extract the flag IDs. Cross-reference those with the beta flag set from 0a to get the **router-level beta flags** — these are flags that gate entire routes or route layouts. +Read `src/routes/router.ts` using the Read tool. Scan its contents for all `isFlagEnabled("...")` calls and extract the flag IDs. Cross-reference those with the **gated** beta flag set from 0a to get the **router-level gated beta flags** — these are flags that hide entire routes or route layouts from users who have not opted in. A router-level flag that is `default: true` does not hide anything and is not part of this set. ```bash node -e " @@ -60,23 +69,26 @@ console.log(JSON.stringify(flagIds)); " ``` -### 0c. Build the beta feature area set +### 0c. Build the gated beta feature area set A feature area is beta if **any** of the following is true: -1. **Route-level gating**: Any path in the feature area's `routes` list is rendered (directly or via a parent layout) by a component that calls a router-level beta flag. Match the routes in the config against the route paths defined near `isFlagEnabled(...)` calls in `router.ts`. For example, if `isFlagEnabled("dashboard")` guards a layout route whose children include `/`, `/runs`, and `/pipelines`, then a feature area with those routes is beta. +1. **Route-level gating**: Any path in the feature area's `routes` list is rendered (directly or via a parent layout) by a component that calls a router-level **gated** beta flag. Match the routes in the config against the route paths defined near `isFlagEnabled(...)` calls in `router.ts`. For example, if `isFlagEnabled("github-component-library")` (`default: false`) guards a layout route whose children include `/`, `/runs`, and `/pipelines`, then a feature area with those routes is beta. If the guarding flag is `default: true`, the routes are live for everyone and the feature area is **not** beta. -2. **Direct flag detection in PR diffs** (applied per-PR in Step 5, not here): If a PR's diff contains calls to `isFlagEnabled("...")` where the extracted flag ID is in the beta flag set, the PR likely touches a beta feature. This signal is applied per-PR rather than per-feature-area. +2. **Direct flag detection in PR diffs** (applied per-PR in Step 5, not here): If a PR's diff contains calls to `isFlagEnabled("...")` where the extracted flag ID is in the gated beta flag set, the PR likely touches a gated beta feature. This signal is applied per-PR rather than per-feature-area. -A feature area with no `routes` that match router-level beta gates, and whose changed files contain no beta `isFlagEnabled` calls in their PR diffs, is considered GA. +A feature area with no `routes` that match router-level gated beta gates, and whose changed files contain no gated beta `isFlagEnabled` calls in their PR diffs, is considered documentable. -**Graduation rules:** A feature area graduates out of the beta set when: +**Release rules:** A feature area leaves the beta set when any of the following happens — all four are equivalent for documentation purposes: +- Its associated flag's `default` changes from `false` to `true` in `src/flags.ts` while remaining `category: "beta"` (the feature is on for everyone by default), OR - Its associated flag's `category` changes from `"beta"` to a non-beta value (e.g. `"setting"`) in `src/flags.ts`, OR - Its associated flag is **removed entirely** from `src/flags.ts` (the feature is unconditionally on), OR - The route-level `isFlagEnabled` guard is removed from `src/routes/router.ts` -This set is used in Step 5 to suppress documentation for unfinished features. +Each of these is a **release event** and triggers the catch-up scan in Step 5. + +This set is used in Step 5 to suppress documentation for features that are still off by default. ## Step 1: Fork Protection @@ -188,9 +200,9 @@ For each remaining PR, use your own reasoning to determine: - Changes to `src/flags.ts` → whichever feature the flag relates to (match the flag ID against feature area descriptions) - Changes to Dashboard route components → `dashboard` -3. **Is the feature GA?** Apply a three-part beta check: +3. **Is the feature released?** Apply a three-part gated-beta check: - **a. Feature area check:** Remove any affected feature area that is in the beta feature area set from Step 0 (route-level gating). If a PR touches both a beta area and a GA area, keep only the GA area and document that portion. + **a. Feature area check:** Remove any affected feature area that is in the beta feature area set from Step 0 (route-level gating by a `default: false` beta flag). If a PR touches both a gated beta area and a released area, keep only the released area and document that portion. **b. Per-PR diff scan:** Regardless of feature area, scan the PR's diff text for `isFlagEnabled("...")` calls: @@ -198,45 +210,53 @@ For each remaining PR, use your own reasoning to determine: extract all isFlagEnabled("...") flag IDs from the diff ``` - If every `isFlagEnabled` call in the diff refers to a beta flag, and the PR does not also touch GA feature area paths, mark the PR as `Beta (skipped)`. If the diff mixes beta flag calls with GA code, document only the GA portions. + Resolve each extracted flag ID against the sets from Step 0a. If every `isFlagEnabled` call in the diff refers to a **gated** beta flag, and the PR does not also touch released feature area paths, mark the PR as `Beta (skipped)`. Calls to a `default: true` beta flag do not suppress anything — treat that code as released. If the diff mixes gated beta flag calls with released code, document only the released portions. - **c. Changes to `src/flags.ts`:** Changes to a flag's `default` value do not affect its beta status — only its `category` matters. Document `src/flags.ts` changes only when a flag's `category` changes away from `"beta"` (graduation) or a flag is removed entirely (feature is unconditionally on). A flag changing `default: false` → `default: true` while remaining `category: "beta"` is not documentable. + **c. Changes to `src/flags.ts`:** A flag's `default` value determines whether its feature is documentable, so changes to it matter. Document `src/flags.ts` changes when any release event from Step 0c occurs: - If all affected areas are beta after both checks, the PR produces no documentation — mark it as `Beta (skipped)` in the summary table. + - `default: false` → `default: true` while `category` stays `"beta"` — the feature now ships on by default and must be documented, including the fact that users can turn it off in settings + - `category` changes away from `"beta"` + - the flag is removed entirely (the feature is unconditionally on) -After screening, if no PRs have documentable (non-beta) user-facing changes, print "No GA user-facing changes found since ``." and stop. + A flag going the other way (`default: true` → `default: false`, or a new flag added as `category: "beta", default: false`) is not documentable. If existing docs describe a feature whose flag was just turned off by default, flag that to the reviewer in the PR body rather than silently deleting the content. + + If all affected areas are gated beta after all three checks, the PR produces no documentation — mark it as `Beta (skipped)` in the summary table. + +After screening, if no PRs have documentable user-facing changes, print "No released user-facing changes found since ``." and stop. Show the user a summary table of what was found: -| PR | Title | User-facing | Feature Areas | Status | -| --- | ----- | ----------- | ------------- | ------------------------------------------- | -| #N | ... | Yes/No | ... | Document / Beta (skipped) / Not user-facing | +| PR | Title | User-facing | Feature Areas | Status | +| --- | ----- | ----------- | ------------- | ------------------------------------------------------------------------- | +| #N | ... | Yes/No | ... | Document / Document (release catch-up) / Beta (skipped) / Not user-facing | -### Graduation catch-up scan +### Release catch-up scan -Run this scan whenever Step 5c detects a graduation event (a flag's `category` flipped away from `"beta"`, a flag was removed from `src/flags.ts`, or a route-level `isFlagEnabled("...")` guard was removed from `src/routes/router.ts`). Skip this section entirely if no graduation event was detected. +Run this scan whenever Step 5c detects a release event: a beta flag's `default` flipped from `false` to `true`, a flag's `category` flipped away from `"beta"`, a flag was removed from `src/flags.ts`, or a route-level `isFlagEnabled("...")` guard was removed from `src/routes/router.ts`. Skip this section entirely if no release event was detected. -The standard 7-day window will not contain the PRs that built the feature — those PRs were correctly skipped as `Beta (skipped)` in earlier weekly runs, and the skill keeps no memory between runs. The graduation week is the only opportunity to recover them, reconstructed on demand from git history. +The `default: false` → `default: true` flip is the most common trigger, and it is usually a one-line diff in `src/flags.ts` with no other user-facing change. Do not let its small diff fool you into skipping the scan — that one line is what makes an entire feature visible to every user, and the whole feature is undocumented at that moment. -#### 1. Identify the graduating flag and its gated paths +The standard 7-day window will not contain the PRs that built the feature — those PRs were correctly skipped as `Beta (skipped)` in earlier weekly runs, and the skill keeps no memory between runs. The release week is the only opportunity to recover them, reconstructed on demand from git history. -The flag ID comes straight from the graduating PR's diff: a removed entry in `src/flags.ts` for flag-removal/category-flip, or the argument of the removed `isFlagEnabled("...")` call for router-guard removal. +#### 1. Identify the released flag and its gated paths + +The flag ID comes straight from the releasing PR's diff: the entry in `src/flags.ts` whose `default` flipped to `true`, the entry whose `category` flipped, the removed entry for flag-removal, or the argument of the removed `isFlagEnabled("...")` call for router-guard removal. Build the set of **path roots** the flag was gating by combining all of the following (deduplicate, expect 1–3 roots): - **Feature-area mapping.** If the flag ID matches a feature area in `docs-config.json` (by name or description), use the file-mapping hints in Step 5 to derive directory roots — e.g. `dashboard` → `src/routes/Dashboard/`, `v2_editor` → `src/routes/v2/`. -- **Removed-guard call sites.** Every file in the graduating PR's diff where `isFlagEnabled("")` was removed contributes its containing directory. -- **Residual grep.** At the graduation commit, search the codebase for any remaining references to the flag ID (tests, comments, analytics metadata). Each match's directory is also a path root. +- **Guard call sites.** Every file that calls `isFlagEnabled("")` contributes its containing directory. For a `default` flip the guards are still in the code, so grep the working tree; for guard removal, take the files from the releasing PR's diff. +- **Residual grep.** At the release commit, search the codebase for any remaining references to the flag ID (tests, comments, analytics metadata). Each match's directory is also a path root. #### 2. Walk back to find the feature's actual start Features are frequently built silently before a beta flag is added, so the flag's introduction date is a **floor, not a start**. Compute the earliest plausible start date: ```bash -GRAD_FLAG="" +RELEASED_FLAG="" # When was the flag itself introduced in src/flags.ts? -FLAG_ADDED_DATE=$(git log --reverse --format='%aI' -S "\"${GRAD_FLAG}\"" -- src/flags.ts | head -1 | cut -dT -f1) +FLAG_ADDED_DATE=$(git log --reverse --format='%aI' -S "\"${RELEASED_FLAG}\"" -- src/flags.ts | head -1 | cut -dT -f1) # When was each path root first created? (Run once per path root.) PATH_ROOT_DATE=$(git log --reverse --diff-filter=A --format='%aI' -- "" | head -1 | cut -dT -f1) @@ -247,23 +267,23 @@ PATH_ROOT_DATE=$(git log --reverse --diff-filter=A --format='%aI' -- "=${FEATURE_START_DATE} merged:<${SINCE_DATE}"` (the `<` excludes PRs already in the current window), then filter to PRs whose `files` list intersects the path roots from #1. Run the survivors through Step 5 with these adjustments: -- The graduating flag is no longer beta. In the per-PR diff scan (5b), drop its flag ID from the beta flag set. All other beta flags still apply. +- The released flag is no longer gated. In the per-PR diff scan (5b), drop its flag ID from the gated beta flag set. All other gated beta flags still apply. - Skipped-by-label rules (`dependencies`, `chore`, `ci`) still apply as in Step 4. -Add the documentable PRs to the run's documentable set with status `Document (graduation catch-up)` in the screening table, marked as pre-dating the standard window. +Add the documentable PRs to the run's documentable set with status `Document (release catch-up)` in the screening table, marked as pre-dating the standard window. #### 4. Volume cap If the catch-up surfaces more than ~80 PRs, or spans more than 4 distinct feature areas, **stop expanding and degrade gracefully** — a single auto-generated PR cannot meaningfully document that much surface area: - Keep only the 30 most recent PRs from the catch-up -- Add a prominent warning to the docs PR body: the graduating feature has a long history and a hand-curated docs migration is recommended +- Add a prominent warning to the docs PR body: the released feature has a long history and a hand-curated docs migration is recommended - The reviewer can re-run with a narrower `--since` and a focused feature-area filter once they've decided how to scope the migration ## Step 6: Fetch Current Documentation @@ -337,17 +357,19 @@ Then either reuse an existing screenshot from `playwright-report/` or add `await If running tests is impractical (no dev server, slow, or fails), skip screenshot capture — never block the docs update on it. Documentation without a screenshot is still valuable. -### 7b. Read the source code for graduating features +### 7b. Read the source code for newly released features -**Skip this sub-step entirely if the documentable set contains no graduation catch-up PRs.** For ordinary weekly updates, the E2E test grounding from 7a is sufficient. +**Skip this sub-step entirely if the documentable set contains no release catch-up PRs.** For ordinary weekly updates, the E2E test grounding from 7a is sufficient. -When catch-up PRs are present, the diffs and commit messages are not a reliable source. Catch-up PRs span months; later PRs silently override earlier ones, labels get renamed, panels get restructured. **The current code is the only authoritative source for what the graduated feature looks like today.** Read the implementation directly before writing any docs: +When catch-up PRs are present, the diffs and commit messages are not a reliable source. Catch-up PRs span months; later PRs silently override earlier ones, labels get renamed, panels get restructured. **The current code is the only authoritative source for what the released feature looks like today.** Read the implementation directly before writing any docs: 1. **Entry-point components** — the route component(s) the feature lives in (e.g. `EditorV2.tsx`, `DashboardLayout.tsx`). These enumerate panels, toolbars, dialogs, and child components in their current shape. 2. **User-visible labels and copy** — JSX text, button labels, tooltip strings, dialog titles, error messages. A label that was renamed three times across the catch-up has only one current name; use that one. 3. **Manifests, registries, and config** — files like `nodes/index.ts`, `registry.ts`, or `manifest.ts` enumerate the feature's current capabilities and are the right source for "what does this feature support" sections. 4. **In-repo architecture notes** — `ARCHITECTURE.md`, `WINDOWS.md`, etc. at the path root. Use these for background understanding only; maintainer language tends to leak internals, so do not copy phrasing into user-facing docs. -5. **E2E tests** (covered in 7a) — for graduating features these are typically the most reliable source of the actual user flow, because they exercise the current shape, not whatever shape existed when each catch-up PR was written. +5. **E2E tests** (covered in 7a) — for newly released features these are typically the most reliable source of the actual user flow, because they exercise the current shape, not whatever shape existed when each catch-up PR was written. + +When the release event was a `default: false` → `default: true` flip, the `isFlagEnabled("")` guards are still in the code. Read both branches and document the `true` branch — that is what users now get. Where the feature is reachable from a settings toggle, mention that users can turn it off, using the flag's `name` and `description` from `src/flags.ts` as the label users will see. The goal is documentation that describes **how the feature works now**, not a commit-by-commit retelling of how it was built. PRs are timeline artifacts; the code is the spec. @@ -355,12 +377,12 @@ The goal is documentation that describes **how the feature works now**, not a co For each user-facing PR and its affected feature areas, reason through: -1. **What changed?** Summarize the behavioral/UI change in plain terms, drawing on the PR diff, linked issues (Step 4a), and E2E test context (Step 7a). For graduating features, draw on the current code as read in Step 7b — _not_ a stitched-together summary of the catch-up PR diffs. +1. **What changed?** Summarize the behavioral/UI change in plain terms, drawing on the PR diff, linked issues (Step 4a), and E2E test context (Step 7a). For newly released features, draw on the current code as read in Step 7b — _not_ a stitched-together summary of the catch-up PR diffs. 2. **Which file(s) should be updated?** Do not default to a single file. Consider every section of the docs. A single PR may warrant changes in more than one file (e.g., a new feature that affects both a core-concepts page and the UI overview). Ask: "Where would a user look for this?" 3. **What's the right structure — edit, new page, or new section?** Default to the smallest unit that fits, and split larger only when the content actually justifies it: - **Edit / add-section** — small features or refinements that fit naturally inside an existing page. Default for ordinary weekly updates. - **Single new page** — a substantial feature with no existing home (e.g. secrets management, artifact visualization). Place it in the most appropriate existing sidebar category. - - **New sidebar category with multiple pages** — large graduations (e.g. a redesigned editor, a new run experience) often warrant their own top-level docs section. Trigger this option only when **at least 3 distinct sub-topics each need more than roughly half a page of content**. Plan a coordinated set of `new-page` changes plus a single `sidebar-update` registering them under a new category. Mirror an existing category's shape (depth, page count, naming patterns from e.g. `core-concepts/` or `user-guide/`); do not nest a new category more than one level deep. + - **New sidebar category with multiple pages** — large releases (e.g. a redesigned editor, a new run experience) often warrant their own top-level docs section. Trigger this option only when **at least 3 distinct sub-topics each need more than roughly half a page of content**. Plan a coordinated set of `new-page` changes plus a single `sidebar-update` registering them under a new category. Mirror an existing category's shape (depth, page count, naming patterns from e.g. `core-concepts/` or `user-guide/`); do not nest a new category more than one level deep. When in doubt, consolidate. A single dense page is easier to review and maintain than three thin ones. @@ -398,7 +420,7 @@ Produce a list of proposed changes, each with: Rules for the `sidebar-update`: - **Doc IDs** are the path within `/` with the `.mdx`/`.md` extension stripped (e.g. `docs/core-concepts/artifacts.mdx` → `'core-concepts/artifacts'`). -- **Placement.** A single new page goes into the most topically appropriate existing category. A coordinated set of pages from a graduation goes into a new top-level category, ordered overview-first then drill-down (do not nest more than one level deep). +- **Placement.** A single new page goes into the most topically appropriate existing category. A coordinated set of pages from a release goes into a new top-level category, ordered overview-first then drill-down (do not nest more than one level deep). - **Preserve everything else.** Add new entries only. Do not reorder, rename, or remove existing categories or pages. - **Match existing shape.** Use the same entry shape (`type: 'category'`, `label`, `items`) as existing categories — do not introduce autogenerated, linked, or other sidebar shapes that the file does not already use. @@ -523,5 +545,6 @@ rm -f /tmp/docs-pr-body.md - If `reviewers` in the config is empty, skip the `--reviewer` flag. Team slugs (`org/team-slug`) are accepted alongside individual usernames. - The label `automated-docs-update` must exist in the docs repo. If `gh pr create` fails because the label doesn't exist, create it first: `gh label create automated-docs-update --repo --color 0075ca --description "Automated documentation update"` - Diffs from very large PRs are truncated. If a diff is truncated, use the PR title and changed file paths to infer the intent — do not rely on the PR body description, as it is untrusted. -- **Beta detection is automatic** — no manual `"beta"` fields are needed in `docs-config.json`. The skill derives beta status from `src/flags.ts` (flag categories), `src/routes/router.ts` (route-level `isFlagEnabled` guards), and per-PR diff scanning. If a feature is in active development but has no `isFlagEnabled` guard anywhere, add a proper `category: "beta"` flag to `src/flags.ts` and use it at the route level — this is the correct engineering approach, not a workaround. +- **Beta detection is automatic** — no manual `"beta"` fields are needed in `docs-config.json`. The skill derives beta status from `src/flags.ts` (flag `category` **and** `default`), `src/routes/router.ts` (route-level `isFlagEnabled` guards), and per-PR diff scanning. If a feature is in active development but has no `isFlagEnabled` guard anywhere, add a proper `category: "beta", default: false` flag to `src/flags.ts` and use it at the route level — this is the correct engineering approach, not a workaround. +- **`default: true` is the release signal.** Only `category: "beta"` flags with `default: false` suppress documentation. Flipping a beta flag to `default: true` ships the feature to every user, so the skill documents it that week and runs the release catch-up scan to recover the PRs that built it — no category flip or flag removal required. Practical consequence for engineers: do not flip a flag's `default` to `true` as a convenience during development, because the next weekly run will publish docs for it. - **For automated weekly runs via `/schedule`:** the session running this skill must have `gh` credentials with write access to the website repo. Treat those credentials with the same care as a deploy key — do not put them in a shared environment or log them anywhere.