Skip to content

core: add DiffOverlay type + persistence + blast-radius computation - #571

Closed
zeroaltitude wants to merge 1 commit into
Egonex-AI:mainfrom
zeroaltitude:feat/diff-overlay-core-upstream
Closed

core: add DiffOverlay type + persistence + blast-radius computation#571
zeroaltitude wants to merge 1 commit into
Egonex-AI:mainfrom
zeroaltitude:feat/diff-overlay-core-upstream

Conversation

@zeroaltitude

Copy link
Copy Markdown

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 a new diff-overlay.ts — pure and deterministic: maps changed file paths to file/function/class nodes (graph-builder stamps the same filePath on all three), then walks 1-hop edges in both directions for the blast radius, excluding the changed nodes themselves.

11 new tests (5 persistence round-trip/sanitization, 6 traversal logic). Full suite: 942/942 pass.

This is a companion PR to #569 (the OpenClaw gateway plugin), which consumes this to add PR/diff-walkthrough tours — but the type/persistence/traversal logic here is general-purpose and useful independent of that integration, so splitting it out as its own contribution.

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).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid relativizing sibling absolute paths

When saveDiffOverlay is given absolute changedFiles outside the project but sharing the same prefix as projectRoot (for example project root /tmp/app and changed file /tmp/app-copy/src/auth.ts), sanitiseFilePath classifies it as inside the project and writes ../app-copy/src/auth.ts instead of falling back to the basename. That leaves outside-project directory information in diff-overlay.json and creates overlay paths that will not match graph file paths; use a real containment check (for example path.relative plus !rel.startsWith('..')) before relativizing.

Useful? React with 👍 / 👎.

@zeroaltitude

Copy link
Copy Markdown
Author

Closing in favor of #574 — same content, reopened from a correctly-named branch (feat/diff-overlay-core instead of feat/diff-overlay-core-upstream) as part of a branch-naming cleanup in our fork.

@zeroaltitude
zeroaltitude deleted the feat/diff-overlay-core-upstream branch July 13, 2026 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant