PPLT-5495: Add file diff data to generate graph

packages/cli-command/src/smartsnap.js

@@ -107,6 +107,58 @@ function gitProjectRoot() {
   return git(['rev-parse', '--show-toplevel']).trim();
 }
 
+// Parse `git diff --unified=0` hunk headers to find which line ranges are new
+// or changed (on the HEAD side) for each file, then key them by the file's
+// index in the stats `files` array — the same index module/source refs use, so
+// the graph can join them without a path lookup. `--unified=0` drops context
+// lines so every hunk header bounds an actual change; `--no-renames` makes a
+// rename surface as add+delete so the new path is always concrete.
+//
+// Files in the diff that aren't tracked in `files` (e.g. node_modules,
+// .storybook, or anything the stats compactor didn't index) are skipped — the
+// graph can only reason about indexed files. Returns { <fileIndex>: [[start, end], ...] }.
+export function getAffectedFileLocations(baseRef, files) {
+  assertSafeRef(baseRef);
+  const diff = git(['diff', '--unified=0', '--no-color', '--no-renames', baseRef, 'HEAD', '--']);
+
+  // The `files` array uses the platform separator (path.relative → back-slash
+  // on Windows); git diff always emits forward slashes. Normalize both to
+  // forward-slash for the path→index lookup.
+  const toPosix = p => p.split(path.sep).join('/');
+  const indexByPath = new Map(files.map((f, i) => [toPosix(f), i]));
+
+  // @@ -a,b +c,d @@ — the new-side `+c,d` is what we want. `d` defaults to 1
+  // when omitted; `d === 0` is a pure deletion anchored at line c (no added
+  // lines), so it contributes no range.
+  const HUNK = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/;
+
+  const locations = {};
+  let currentIdx;
+  for (const line of diff.split('\n')) {
+    // `+++ b/<path>` opens a file's hunks with its new-side path; a pure
+    // deletion shows `+++ /dev/null`, so leave currentIdx unset and skip it.
+    if (line.startsWith('+++ ')) {
+      const p = line.slice(4);
+      if (p === '/dev/null') { currentIdx = undefined; continue; }
+      /* istanbul ignore next: git always prefixes the new-side path with `b/`
+         (anything else is `/dev/null`, handled above), so the bare `: p`
+         fallback is defensive and unreachable in real diff output */
+      const rel = p.startsWith('b/') ? p.slice(2) : p;
+      currentIdx = indexByPath.get(rel);
+      continue;
+    }
+    if (currentIdx === undefined) continue;
+    const m = HUNK.exec(line);
+    if (!m) continue;
+    const start = parseInt(m[1], 10);
+    const count = m[2] === undefined ? 1 : parseInt(m[2], 10);
+    if (count === 0) continue;
+    if (!locations[currentIdx]) locations[currentIdx] = [];
+    locations[currentIdx].push([start, start + count - 1]);
+  }
+  return locations;
+}
+
 // Paths under these directories are dependencies / framework wiring rather
 // than first-party source, so we don't track them in the file index.
 const EXCLUDED_DIRS = new Set(['node_modules', '.storybook']);
@@ -147,6 +199,11 @@ function transformModule(m, fileIndex, projectRoot) {
     if (copy.type === 'src' && typeof copy.source === 'string') {
       copy.source = resolveAndIndex(copy.source, fileIndex, projectRoot);
     }
+    // `loc` (when present) is an array of `{ start, end }` source spans; the
+    // graph expects each span as a `[start, end]` tuple instead.
+    if (Array.isArray(copy.loc)) {
+      copy.loc = copy.loc.map(l => [l.start, l.end]);
+    }
     return copy;
   };
 
@@ -216,7 +273,7 @@ async function pollGraphStatus(percy, buildId, log) {
     const res = await percy.client.getStatus('smartsnap_graph', [buildId]);
     const status = res?.status;
     log.debug(`SmartSnap: graph status (attempt ${i + 1}) = ${status}`);
-    if (status === 'done' || status === 'failure') return { status, data: res?.data };
+    if (status === 'done' || status === 'failed') return { status, data: res?.data };
     if (i < POLL_ATTEMPTS - 1) await new Promise(r => setTimeout(r, POLL_INTERVAL_MS));
   }
   return { status: null };
@@ -430,10 +487,12 @@ export function extractStorybookPaths(snapshots, normalizeImportPath, log) {
 // response body that used to come from `getSmartsnapGraphData`. Bails to a
 // full snapshot set if the job doesn't reach `done`.
 export async function runGraphGeneration(percy, buildId, payload, log) {
-  const { files, modules, storybookPaths, affectedNodes } = payload;
-  log.debug(`SmartSnap: starting graph generation job ${JSON.stringify({ buildId, files, modules, storybookPaths, affectedNodes })}`);
+  const { files, modules, storybookPaths, affectedNodes, affectedFileLocations } = payload;
+  log.debug(`SmartSnap: starting graph generation job ${JSON.stringify({ buildId, files, modules, storybookPaths, affectedNodes, affectedFileLocations })}`);
+  // Pass camelCase to the client, which snake_cases it to `affected_file_locations`
+  // for the API (same convention as storybookPaths → storybook_paths).
   await percy.client.generateSmartsnapGraph(buildId, {
-    files, modules, storybookPaths, affectedNodes
+    files, modules, storybookPaths, affectedNodes, affectedFileLocations
   });
 
   const { status, data } = await pollGraphStatus(percy, buildId, log);
@@ -583,7 +642,10 @@ export async function applySmartSnap(percy, snapshots, smartSnapConfig, buildDir
     affectedNodes = [...affectedNodes, ...packageAffectedNodes];
   }
 
-  const data = await runGraphGeneration(percy, buildId, { files, modules, storybookPaths, affectedNodes }, log);
+  // Line-level diff ranges for the indexed files, keyed by their `files` index.
+  const affectedFileLocations = getAffectedFileLocations(baseRef, files);
+
+  const data = await runGraphGeneration(percy, buildId, { files, modules, storybookPaths, affectedNodes, affectedFileLocations }, log);
 
   maybeWriteTrace(trace, data, log);
 

packages/cli-command/test/smartsnap.test.js

@@ -11,6 +11,7 @@ import {
   assertNoBailOnChanges,
   enforceUntraced,
   getAffectedPackages,
+  getAffectedFileLocations,
   extractStorybookPaths,
   runGraphGeneration,
   maybeWriteTrace,
@@ -148,12 +149,12 @@ describe('smartsnap', () => {
             {
               id: '/root/src/A.js',
               imports: [
-                { type: 'src', source: '/root/src/B.js' },
+                { type: 'src', source: '/root/src/B.js', loc: [{ start: 38, end: 38 }, { start: 40, end: 42 }] },
                 { type: 'src', source: '/root/src/B.js' }, // duplicate → reuses the existing index
                 { type: 'src', source: 'lib/rel.js' }, // non-absolute → returned as-is, not indexed
                 { type: 'module', source: 'react' }
               ],
-              passThroughExports: [{ type: 'src', source: '/root/src/C.js' }],
+              passThroughExports: [{ type: 'src', source: '/root/src/C.js', loc: [{ start: 5, end: 5 }] }],
               nonPassThroughExports: [{ type: 'module', source: 'lodash' }]
             },
             { id: '/root/node_modules/dep/index.js' }, // excluded → string id → dropped
@@ -174,7 +175,9 @@ describe('smartsnap', () => {
       expect(res.modules[0].imports[1].source).toEqual(1); // duplicate → same index
       expect(res.modules[0].imports[2].source).toEqual('lib/rel.js'); // non-absolute → as-is
       expect(res.modules[0].imports[3].source).toEqual('react'); // module ref → untouched
+      expect(res.modules[0].imports[0].loc).toEqual([[38, 38], [40, 42]]); // {start,end} spans → [start,end] tuples
       expect(res.modules[0].passThroughExports[0].source).toEqual(2);
+      expect(res.modules[0].passThroughExports[0].loc).toEqual([[5, 5]]);
       expect(res.modules[0].nonPassThroughExports).toEqual([{ type: 'module', source: 'lodash' }]);
       expect(res.modules[1]).toEqual({});
     });
@@ -363,7 +366,14 @@ describe('smartsnap', () => {
           getStatus: async () => ({ status: 'done', data })
         }
       };
-      let payload = { files: ['f'], modules: [{ id: 0 }], storybookPaths: ['p'], affectedNodes: ['a'] };
+      // affectedFileLocations is forwarded verbatim; the client snake_cases it.
+      let payload = {
+        files: ['f'],
+        modules: [{ id: 0 }],
+        storybookPaths: ['p'],
+        affectedNodes: ['a'],
+        affectedFileLocations: { 0: [[3, 3], [6, 7]] }
+      };
 
       let res = await runGraphGeneration(percy, 'bld-1', payload, log);
 
@@ -376,7 +386,7 @@ describe('smartsnap', () => {
       let percy = {
         client: {
           generateSmartsnapGraph: async () => {},
-          getStatus: async () => ({ status: 'failure' })
+          getStatus: async () => ({ status: 'failed' })
         }
       };
       await expectBail(
@@ -609,6 +619,65 @@ describe('smartsnap', () => {
     });
   });
 
+  describe('getAffectedFileLocations()', () => {
+    let origCwd = process.cwd();
+    let repos = [];
+    afterEach(() => {
+      process.chdir(origCwd);
+      for (let d of repos.splice(0)) fs.rmSync(d, { recursive: true, force: true });
+    });
+
+    function setup(seed, changed) {
+      let info = makeRepo(seed, changed);
+      repos.push(info.dir);
+      process.chdir(info.dir);
+      return info;
+    }
+
+    it('maps changed line ranges to file index, skipping unindexed and deleted files', () => {
+      let { dir, baseSha } = setup(
+        {
+          'src/A.js': 'a\nb\nc\nd\ne\n', // indexed; line 3 changed + lines 6-7 appended
+          'src/del.js': 'x\n' // indexed; deleted at HEAD → no new-side lines
+        },
+        {
+          'src/A.js': 'a\nb\nC\nd\ne\nf\ng\n',
+          'src/B.js': 'new\n' // NOT in the files index → skipped
+        });
+      // makeRepo's writeAll can't delete; drop del.js in a follow-up commit so the
+      // baseSha→HEAD diff carries its `+++ /dev/null` deletion hunk.
+      fs.rmSync(path.join(dir, 'src/del.js'));
+      git(['add', '-A'], dir);
+      git(['commit', '-qm', 'remove del'], dir);
+
+      // files index: A=0, del=1; B.js is absent (not indexed).
+      let res = getAffectedFileLocations(baseSha, ['src/A.js', 'src/del.js']);
+
+      // A.js: `@@ -3 +3 @@` → [3,3] and `@@ -5,0 +6,2 @@` → [6,7].
+      // del.js shows `+++ /dev/null` (pure deletion) → no entry. B.js unindexed → no entry.
+      expect(res).toEqual({ 0: [[3, 3], [6, 7]] });
+    });
+
+    it('ignores pure-deletion hunks that add no new lines', () => {
+      let { baseSha } = setup(
+        { 'src/A.js': '1\n2\n3\n' }, // line 2 removed, file kept
+        { 'src/A.js': '1\n3\n' });
+      // `@@ -2 +1,0 @@` → new-side count 0 → no range contributed.
+      expect(getAffectedFileLocations(baseSha, ['src/A.js'])).toEqual({});
+    });
+
+    it('returns an empty map when there is no diff', () => {
+      let { baseSha } = setup({ 'src/A.js': 'a\n' });
+      // baseSha === HEAD (no second commit) → `git diff <sha> HEAD` is empty.
+      expect(getAffectedFileLocations(baseSha, ['src/A.js'])).toEqual({});
+    });
+
+    it('bails on an unsafe base ref before shelling out to git', () => {
+      expect(() => getAffectedFileLocations('--upload-pack=evil', []))
+        .toThrowMatching(e => e instanceof SmartSnapBailError && e.message.includes('unsafe baseline ref'));
+    });
+  });
+
   describe('applySmartSnap() [integration]', () => {
     let origCwd = process.cwd();
     let repos = [];

packages/client/src/client.js

@@ -457,15 +457,17 @@ export class PercyClient {
     files,
     modules,
     storybookPaths,
-    affectedNodes
+    affectedNodes,
+    affectedFileLocations
   } = {}) {
     this.log.debug(`Smartsnap: enqueueing graph build for build ${buildId}...`);
     return this.post('smartsnap/generate-graph', {
       build_id: buildId,
       files,
       modules,
       storybook_paths: storybookPaths,
-      affected_nodes: affectedNodes
+      affected_nodes: affectedNodes,
+      affected_file_locations: affectedFileLocations
     }, { identifier: 'smartsnap.generate_graph' });
   }
 

packages/client/test/client.test.js

@@ -901,7 +901,8 @@ describe('PercyClient', () => {
         files: ['a.js', 'b.js'],
         modules: [{ id: 1, name: 'mod' }],
         storybookPaths: ['stories/a.js'],
-        affectedNodes: ['node-1']
+        affectedNodes: ['node-1'],
+        affectedFileLocations: { 0: [[3, 3], [6, 7]], 1: [[1, 1]] }
       })).toBeResolvedTo({ status: 'queued' });
 
       expect(api.requests['/smartsnap/generate-graph']).toBeDefined();
@@ -911,7 +912,8 @@ describe('PercyClient', () => {
         files: ['a.js', 'b.js'],
         modules: [{ id: 1, name: 'mod' }],
         storybook_paths: ['stories/a.js'],
-        affected_nodes: ['node-1']
+        affected_nodes: ['node-1'],
+        affected_file_locations: { 0: [[3, 3], [6, 7]], 1: [[1, 1]] }
       });
     });