Uh oh!
There was an error while loading. Please reload this page.
feat(visualize): scene-graph contract, four renderers, and skill surface - #21
Open
harrymove-ctrl wants to merge 10 commits into
Open
feat(visualize): scene-graph contract, four renderers, and skill surface#21harrymove-ctrl wants to merge 10 commits into
harrymove-ctrl wants to merge 10 commits into
Conversation
Reviewer found validateSceneGraph never checked doc.repo, doc.diagramType, or doc.altitude despite the schema declaring all three required, so a document omitting them passed as valid. Enforce them following the existing error-message style, and make the schema drift test actually call validateSceneGraph for every schema-required field instead of only comparing hardcoded literals.
The scene-graph payload was embedded via raw JSON.stringify, so a node label, citation, or sample containing the literal substring </script> would truncate the script element early: JSON.parse fails client-side, the inspector/dots/click wiring never runs, and the remainder of the payload becomes a live executing script (injection). Add an exported embedJson helper that escapes < as \u003c (still valid JSON, round-trips exactly) and use it at the one call site. Add a regression test that renders a label containing </script>, extracts the embedded payload, and asserts it parses back to the original content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements deliverable 1 of the
cmk:visualizedesign, up to but not including the dogfood pass.Base branch is
docs/visualize-skill-design, notmain. This is stacked on #20 so the design docs stay in their own review. Retarget tomainonce #20 merges.What this is
cmk:visualizerenders a codebase as a map you can discuss with an agent. The model's only output is a validated JSON scene graph; fixed renderers draw it. Citations are enforced by the validator, so an uncited node or edge cannot reach a picture.What is here
scene-graph.schema.jsonplusvalidate.mjs. Enforces the eight top-level fields, rejects any node or edge with an emptycitationsarray, rejects edges naming unknown nodes, pins thefoldedandgapsitem shapes, and caps nesting depth at 3.renderSvgfor static output,renderHtmlfor interactive, withisometric,flat, andthree-dprojections. Both refuse to render an invalid document and report the validation errors instead.SKILL.mdat 66 lines,references/analysis.md,references/scene-graph.md,eval.json,TESTS.md.npm testand added to Frontend CI. Zero new dependencies.This is the kit's first code-carrying skill. The renderer bundle is vanilla, offline, and build-free, because
cmk:agent-vendorsforbids a package referencing anything outside itself.What is NOT here
Task 6 of the plan: the real scene graph of this repo, and registration in
lib/skills.tsand the plugin manifest. The skill therefore does not yet appear on/skills, and nothing has pointed it at a real codebase yet. That is the next commit, not a follow-up ticket.Defects found and fixed during implementation
All five were defects in the plan, not in the execution:
node --test <dir>does not work; Node treats a bare directory as an entry point. Replaced with a glob.repo,diagramType, andaltitudethough the schema declared them required. A document omitting all three returnedvalid: true.JSON.stringify(doc)was embedded raw inside a<script>tag. Any label or snippet containing</scripttruncated the payload, killed the interactive page, and opened a script-injection path. Verified broken and then verified fixed in a real browser against a hostile document.foldedandgapsitem shapes were unspecified in schema, validator, and docs, so a differently-keyed entry validated cleanly and renderedundefinedinto the explainer panel.Deferred, recorded not dropped
projections[doc.style]. Deliberately not added: a silent fallback would mask the drift it guards against.Verification
38/38 tests, ESLint clean,
tsc --noEmitclean,skill-lint: OK,npm run buildgreen.Beyond the suite, the interactive renderer was checked in a real browser: clicking a dot shows the payload snippet with its own
file:line, the three projections produce measurably different geometry (three-d compresses column spacing from 150px to 132px between rows while flat holds at 150px), and the rendered page makes zero external requests.TESTS.mdcontains no results. The pressure-test runs have not been performed, and the file says so rather than presenting invented ones.