core: add DiffOverlay type + persistence + blast-radius computation - #574
core: add DiffOverlay type + persistence + blast-radius computation#574zeroaltitude wants to merge 1 commit into
Conversation
Formalizes an on-disk contract that already existed informally: the understand-diff skill has always read/written <ua-dir>/diff-overlay.json in this exact shape (SKILL.md step 8), and the dashboard's store/GraphView already consume changedNodeIds/affectedNodeIds for the diff-mode overlay UI — but core had no first-class type or persistence pair for it, unlike the sibling domain-graph.json (saveDomainGraph/loadDomainGraph). - DiffOverlay type in types.ts. - saveDiffOverlay/loadDiffOverlay in persistence/index.ts, mirroring the domain-graph save/load pattern exactly (same directory resolution, same file-path sanitization so absolute paths never leak to disk — extracted the per-path sanitization logic out of sanitiseFilePaths into a reusable sanitiseFilePath so both the node-array and plain-string-array shapes can share it, no behavior change to the existing function). - computeDiffOverlay(graph, changedFiles, baseBranch?) in the new diff-overlay.ts: pure, deterministic graph traversal — maps changed file paths to their file/function/class nodes (graph-builder stamps the same filePath on all three), then walks one hop of edges in both directions to find the blast radius, excluding the changed nodes themselves. No LLM involved, mirrors the skill's own documented algorithm (grep node IDs in edges, 1-hop) exactly. 11 new tests (5 persistence round-trip/sanitization, 6 traversal logic). Full suite: 942/942 tests pass (931 existing + 11 new).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fe8d03dda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dir = ensureDir(projectRoot); | ||
| const sanitised: DiffOverlay = { | ||
| ...overlay, | ||
| changedFiles: overlay.changedFiles.map((fp) => sanitiseFilePath(fp, projectRoot)), |
There was a problem hiding this comment.
Keep outside diff paths from escaping the project
When saveDiffOverlay receives an absolute changedFiles entry that is outside projectRoot but merely shares its string prefix (for example projectRoot=/tmp/app and /tmp/app-old/src/a.ts), the sanitizer treats it as in-project and writes a ../app-old/src/a.ts path into .ua/diff-overlay.json instead of falling back to the basename. That violates the outside-path privacy contract and can expose sibling directory layout; the relativization should only run after a real path-boundary check.
Useful? React with 👍 / 👎.
Summary
Gives
@understand-anything/corea first-class, deterministic way to answer "what did this diff touch, and what else does that touch?" — aDiffOverlay: the set of graph nodes a diff changed, plus the set of nodes one hop away from those (imports/importers, callers/callees) that weren't changed themselves but are affected by the change.Why this matters: understanding a diff against an already-analyzed codebase is a different problem from understanding the codebase itself — a reviewer doesn't just want "what does this function do," they want "what changed, and what could break because of it." Before this PR, that reasoning only existed informally: the
understand-diffskill has always read/written<ua-dir>/diff-overlay.jsonin this exact shape by convention (SKILL.md step 8), and the dashboard's own diff-mode UI (changedNodeIds/affectedNodeIdsin the store/GraphView) already expects it — but nothing in core actually produced that data as a real type with a real algorithm. It was a shape everyone agreed on informally, computed ad hoc wherever it was needed. This PR makes it a proper first-class part of the graph model, the same waydomain-graph.jsonalready is.This is deliberately engine work, not UI or LLM work — everything here is pure and deterministic (no model calls), so anything that already understands the graph (the dashboard, a skill, a future feature) can compute a diff overlay the same way, instead of each caller reimplementing its own "what's near this changed file" logic.
What this adds
DiffOverlaytype (types.ts):{ changedFiles, changedNodeIds, affectedNodeIds, baseBranch?, generatedAt, version }.saveDiffOverlay/loadDiffOverlay(persistence/index.ts): reads/writes<ua-dir>/diff-overlay.json, mirroring the existingsaveDomainGraph/loadDomainGraphpattern exactly — same directory resolution, same file-path sanitization so absolute local paths never leak into the persisted JSON.computeDiffOverlay(graph, changedFiles, baseBranch?)(newdiff-overlay.ts) — the actual algorithm:filePath(file, function, and class nodes all share it).changedNodeIds, included inaffectedNodeIds.No LLM, no heuristics — same input always produces the same output, which is what lets a downstream caller (like the PR-walkthrough feature in feat: native OpenClaw gateway plugin (in-process analysis pipeline + dashboard route) #575) treat this as ground truth to narrate from, rather than something the model has to infer itself.
Test plan
diff-overlay-persistence.test.ts), 6 traversal-logic cases (diff-overlay.test.ts) — changed file with no graph match, single-hop forward/backward edges, nodes excluded correctly from their own affected set, multi-file diffs.sanitiseFilePathswas refactored to share a newsanitiseFilePathhelper with zero behavior change — covered by the existing domain-graph persistence tests still passing unmodified).Relationship to other PRs
#575 (the OpenClaw gateway plugin) depends on this: its
understand_analyze_prtool callscomputeDiffOverlayto get the changed+affected node sets, then asks an LLM to narrate a walkthrough grounded strictly in what this PR computes — the plugin adds the LLM narration and thegh pr diff/git diffresolution on top of what's purely graph math here.