Repository navigation
docs(.agents/skills): add code walkthrough skill - #7776
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new walkthrough skill to generate interactive HTML architecture walkthroughs for the repository. It adds SKILL.md for documentation, template.html for the interactive UI, and render.go to handle Go package fact extraction and validation. The review feedback highlights a validation bug in render.go where the "other" layer is omitted during skeleton initialization, suggests strictly enforcing trailing slashes for directory citations, and recommends refactoring the link hydration logic in template.html to use DOM-based sanitization instead of a regular expression.
2b63a28 to
d498c0b
Compare
3a5b33c to
3a417f1
Compare
noahdietz
left a comment
There was a problem hiding this comment.
Personally I would put the dependency_test.go and CI change in a different PR unless it is strictly related to the skill primarily bc we share this codebase and if it produces a dev regression, it's easier to revert an isolated CL :) but LGTM otherwise
|
@noahdietz done: |
96b5965 to
fb148e2
Compare
fb148e2 to
31a8def
Compare
Adds a skill that produces a self-contained walkthrough.html for the repository at the current commit: an architecture map of every Go package with its import wires, a guided tour, command flows, deep-dive guides and findings. The agent writes the narrative in a JSON file after reading the code; a small renderer computes the facts from go list (packages, imports, non-test line counts, doc synopses, module path, commit), validates every cited path and package reference, HTML-escapes the payload and fills a static template. Keeping the narrative out of Go means a walkthrough can be regenerated on request for any subsystem without a code change, and nothing under .agents/ is matched by ./..., so adding a package to the repository never affects CI. The template keeps only the data-src attribute on author-supplied markup and strips everything else, so guide content cannot inject scripts, links or styling into the page.
…d in browser testing Drives the generated page with a headless Chromium over the DevTools protocol, clicking every tab, card, chip, button and keyboard shortcut on a normal fixture, a full-feature fixture (guides, languages, hooks matrix) and a hostile fixture with script, scheme, namespace, clobbering and overlay payloads. All 128 checks pass with no console errors, exceptions or CSP violations. The problems found are fixed here. The sanitizer previously turned any unknown element into visible raw markup, so an author writing <b> or <table> saw literal tags. It now removes script-capable and resource-loading elements with their subtrees (including foreign-namespace svg and math, matched case-insensitively), unwraps other unknown tags so their text still shows, strips comments, and allows common formatting, list, heading and table tags. The only attribute that survives is still data-src. Clipboard writes now report whether they actually succeeded, and the prompt drawer and button say so instead of always claiming a copy happened. The renderer requires directories to be cited with a trailing slash, which lets the page label paths and build GitHub tree or blob links without guessing from the presence of a dot. Counts are pluralised, Enter in the topic box copies the prompt, and the skill documents that titles are plain text.
…emaining assumptions A second review pass over the renderer and page, looking for security, reproducibility and hidden assumptions. The template is now embedded into render.go, so the renderer carries no assumption about where it lives and works against any module root; a -template flag allows experiments. The module root comes from go env GOMOD, which is correct inside a go.work workspace and gives a clear error outside a module, where go list -m could print several directories. A nil Module in go list output no longer panics. Packages with no non-test Go files are omitted instead of appearing as empty cards, and a module whose root directory has code is included as ".". Source links are now built in the DOM while sanitizing rather than by a regular expression over serialized HTML, which removes a double-escaping bug for paths containing an ampersand and one more string-munging step. The CSP drops the unused img-src and adds base-uri and form-action restrictions. The renderer validates that repo and gitRef, which the page interpolates into GitHub URLs, are a plain org/name and ref name, and that flow and guide ids are unique. -init refuses to overwrite an existing file so authored content cannot be lost by rerunning the first step. The skill documents the remaining scope limits: Go modules only, test-only packages omitted, GitHub-shaped module paths for the link, and that prompt buttons interpolate authored titles.
…and full walkthrough content The languages table had four built-in columns (generator, installer, formatter, publish target), which was the last piece of the template that assumed what a reader wants to compare. Columns are now authored as `langCols` and each language supplies `cells` keyed by column, so a walkthrough can add `.repo-metadata.json`, bump or CI columns without a template change. Cells are validated and sanitized like bodies. The renderer also reports the module's Go version in the subtitle, offers `-facts` to dump the computed packages, imports and LOC as JSON (so importer claims such as "only X imports Y" are derived rather than read by hand), and always emits `imports` as an array instead of null. SKILL.md documents `-facts`, the new table shape and that hook matrices must be derived from the checkout. Validated by regenerating the walkthrough with the full content from the v2 page (13 tour steps, 4 flows, 4 guides, 9 languages, hooks matrix, findings), with corrections against main: dependency_test.go no longer exists, the swift annotator is annotate_model.go, and stale counts were removed from prose. Browser testing exercised every tab, step, flow, guide, table row, source link and copy button: 193 checks passed with no console errors or CSP violations.
The skeleton puts every package on the "other" layer, but merge only declared that layer when a package was missing from the JSON, so rendering an unedited skeleton failed with "uses unknown layer". The layer is now declared whenever any package uses it, and each such package gets a warning so -check stays noisy until it is assigned. An empty <a data-src> now shows its path instead of an invisible link.
…through "walkthrough" alone could mean a product tour or a tutorial; the skill produces a walkthrough of the code, so the directory, skill name and every path in SKILL.md, render.go and .gitignore now say code-walkthrough. Generated file names (walkthrough.json/.html) are unchanged.
…e code excerpts A guide step titled "1. Add ..." rendered as "2. 1. Add ..." because the page numbers steps itself. The renderer now rejects tour, flow-stage and guide-step titles that start with their own ordinal, and SKILL.md says so. How-to guides should show code, so the skill now asks for short verbatim excerpts from the checkout after a link to their file, and asks authors to name types and fields rather than use shorthand. The template styles <pre><code> as a block (no inline pill padding) and renders authored <ol> lists with decimals instead of bullets.
… commit links, defined scope
Below 1024px the tour and guide step lists overflowed off-screen: the
utility rule was written as overflow-x-auto{overflow-x-auto}, so the
row never scrolled. The lists now wrap instead of scrolling, and labels
wrap too, so every step button is inside the viewport at any width.
Map wires were drawn under the layer cards because the layers container
had its own z-index above the SVG. The SVG now paints above layer
backgrounds and package cards paint above the SVG, so a wire is visible
along its whole length.
GitHub links defaulted to main, not the commit named in the subtitle.
gitRef now defaults to the commit SHA; main is a fallback only when
there is no git commit at all, with a warning.
Following SKILL.md literally produced a page with no flows and no deep
dives because it called them optional. The skill now defines what a
whole-architecture request contains (one flow per CLI command, four
kinds of guide, findings, a per-target table when the repository has
one package per target) and how to find each from the checkout. A
single-subsystem request sets focus; without it the renderer warns
about missing flows, guides or findings.
Verified in a headless browser: on the previous template the new
checks fail (0 of 297 wire sample points visible; last step unreachable
at 800px); on this one 214 checks pass.
…dd tests, pin CSP, fix page bugs Validation only matched data-src="..." so single-quoted, unquoted, upper-case and spaced forms rendered live links to missing files. The scan now accepts every form, and the page drops any data-src that is not in the renderer's validated list (emitted as `cited`). The page's CSP allowed any inline script. The renderer now replaces script-src 'unsafe-inline' with the sha256 of the template's script block, so only that code can run. The sanitizer renames handler attributes before parsing because Chrome reports a violation for merely parsing one into an inert template. render_test.go covers every validate error, each data-src variant, numbered titles (now also findings and "Step N:" prefixes), merge (Unclassified packages, gitRef defaulting to the commit, completeness warnings), pinScripts and an -init -> -check round-trip on a temporary module. Exported types have doc comments so golangci-lint passes. -check now exits non-zero while warnings remain. Page fixes: missing utilities (py-4 left the title with no top padding), AA contrast for the emerald and amber tokens in the light theme, layer headings above wires, hook names never break mid-word, links inside a language row no longer toggle it, reset highlight survives leaving the map, the inspect modal lists the package's Go files (from go list GoFiles), and #tab, #tour/N, #guides/<id>/N and #flows/<id> deep links. SKILL.md documents the full JSON shape with one example per section, makes the task read-only (stale doc comments become findings), names the two required pipeline guides (how the main command produces its output; how the engine turns input into output) and replaces the fan-in rule, which selects leaf utilities, with "the subsystem the main command spends its time in". Browser run: 261 checks pass across the generated page, the full-feature fixture and the hostile fixture, with no console errors or CSP reports.
31a8def to
c38120b
Compare
…boxed viewers Normalized newlines and escaped all non-ASCII characters in the HTML template and embedded JSON payload so the rendered page is pure 7-bit ASCII. Viewers that decode the page with a non-UTF-8 default encoding before reading the charset meta tag no longer alter the inline script code units and break its pinned Content-Security-Policy SHA-256 hash. Removed the location.replace fallback when history.replaceState throws in setRoute. Sandboxed about:srcdoc iframe viewers disallow history.replaceState on opaque origins, and falling back to location.replace navigated the iframe away from the document on startup.
…ough Focused requests such as "walk me through rust bump" produced JSON and HTML files under names the .gitignore did not cover, so every new walkthrough showed up as untracked. All output now goes in .walkthrough/ at the module root, which is ignored as a whole, and the renderer rejects any -out outside it. Rendering defaults to .walkthrough/<name>.html beside the input JSON. Each walkthrough is named after its scope (architecture, or the focus in kebab case), so several can coexist without overwriting each other. Every page now has a Walkthroughs bar linking to the other pages in the directory. Rendering a page also rewrites the link list in the pages rendered before it; only the JSON data block changes, so their pinned CSP script hashes stay valid.
Adds a
code-walkthroughskill that produces self-contained, local interactive HTML walkthroughs of this repository at the current checked out commit. This includes things like an architecture map with import wires, a guided tour, one flow per CLI command, deep-dive guides with code excerpts, a per-language table with an exported-hooks matrix, and findings.It is just a skill with some templates to fill out.
Triggered by prompting "walk me through the code", or narrowed to one area, for example "walk me through rust bump". If you ask for multiple walkthroughs in the same session, they'll get added as tabs in the generated site.
All output goes in
.walkthrough/at the repo root, which is gitignored as a whole. Each walkthrough is a pair named after its scope (architecture.{json,html},rust-bump.{json,html}, ...), so several can coexist, and every page has a "Walkthroughs" bar linking to the others.Note: this skill takes some time to generate the site - up to 15 min depending on your model.
Main things to review
walkthrough prompt and the actual walkthrough output site itself.
Agent description
Details
How it works
<name>isarchitecturefor a whole-codebase request, or the focus in kebab case (rust-bump,python-generate). The renderer refuses any-outoutside.walkthrough/.<a data-src="…">citedallowlist for the page.walkthrough/for the "Walkthroughs" barNothing under
.agents/is matched by./..., so the skill never affects CI builds.Generating a walkthrough
From the repository root, in Jetski or any agent that loads
.agents/skills/:or, equivalently, "Generate an interactive walkthrough of this repository."
The skill takes the skeleton from the renderer, reads the code, writes the narrative into
.walkthrough/<name>.json, iterates until-checkis clean, and surfaces.walkthrough/<name>.html(opens fromfile://, no server). Narrow it with a subject ("…of the release pipeline", "walk me through rust bump"), which setsfocusand writes a separate pair. Asking for more on an existing walkthrough edits its JSON and re-renders it instead of starting a new pair.Design points
-factsfor import claims).-checkexits non-zero while any warning remains. A single-subsystem request setsfocus..walkthrough/, and the renderer rejects any other-out, so no walkthrough name ever shows up ingit status..walkthrough/and rewrites the list in the pages rendered earlier. Only their JSON data block changes, so their pinned CSP script hashes stay valid.script-srcis the sha256 of the template's own script, computed at render time. Author markup keeps onlydata-src; script/resource elements are dropped with their subtrees, other tags unwrapped, all other attributes stripped.data-srcis validated in every quoting/casing form, and the page drops any path the renderer did not validate. Sibling links are restricted to plainNAME.htmlfile names, checked by both the renderer and the page.main; tabs, steps, guides and flows are deep-linkable (#guides/<id>/N).Verification
gofmt,goimports,go vet,golangci-lintandgo test -race ./.agents/skills/code-walkthrough/scripts/are clean.main: 13 tour steps, 4 flows, 6 guides (incl. generation and sidekick pipelines), 9 languages × 7 columns, 19-row hooks matrix, 5 findings. Three stale claims from docs: add interactive walkthroughs and site generator #7771 were corrected rather than ported (dependency_test.gois gone; the swift annotator isannotate_model.go; counts removed from prose).javascript:hrefs,img onerror,svg onload,iframe,base,form,</script>breakout, DOM clobbering), plus wire visibility by hit-testing, 800 px / 390 px layouts, AA contrast in both themes and deep links. 261/261 checks pass with no console errors, exceptions, CSP reports or navigations..walkthrough/,git statusstayed empty,git check-ignoreattributes each file to.gitignore, and each page's "Walkthroughs" bar links to the others.