FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(dataflow): P4 incremental re-stitch + P6 vertex extraction on native engine path by carlos-alm · Pull Request #1635 · optave/ops-codegraph-tool · GitHub

feat(dataflow): P4 incremental re-stitch + P6 vertex extraction on native engine path - #1635

Merged
carlos-alm merged 14 commits into
mainfrom
feat/dataflow-p4-native
Jun 20, 2026
Merged

feat(dataflow): P4 incremental re-stitch + P6 vertex extraction on native engine path#1635
carlos-alm merged 14 commits into
mainfrom
feat/dataflow-p4-native

Conversation

Copy link
Copy Markdown
Contributor

Summary

  • P6 — vertex extraction on native bulk-insert path: dataflow vertices now extracted during the native orchestrator's bulk insert pass; eliminates the JS post-pass for native builds
  • P6 parity fix: vertex extraction aligned between native orchestrator and incremental paths
  • P4 — incremental re-stitch on native engine path: when a callee file changes, the native engine re-stitches arg_in/return_out edges for affected callers without a full rebuild
  • P4 — re-stitch for unchanged caller files: native vertex pass now triggers re-stitch even when the caller file itself is unchanged but its callees changed
  • Fix: normalise native DataflowResult before vertex pass; exclude directory nodes that were incorrectly included
  • Fix: clear dataflow.call_edge_id refs before edge deletion to satisfy FK constraint (prevented incremental deletes from completing)
  • Perf: scope P6 vertex pass to files that actually have dataflow edges on full builds (avoids full-corpus scan)
  • Docs: update dataflow limitations section — interprocedural analysis is now supported

Test plan

  • tests/integration/dataflow-p4-native.test.ts — P4 native incremental re-stitch
  • tests/engines/ — parity between native and WASM dataflow vertex output
  • codegraph build --dataflow on this repo; verify no WARN on vertex pass
  • Incremental rebuild after changing a callee file; confirm re-stitch fires

Wires buildDataflowVerticesAndEdges + buildInterproceduralStitch into the
native fast path so that users running --engine native (the default) populate
dataflow_vertices and dataflow_summary on every build.

Rust DataflowResult already contains parameters/returns — no re-parse needed.

Adds migration v19 sentinel to both JS and Rust migration tables. This forces a
one-time full rebuild of any database that was built on the native path before
P6, ensuring dataflow_vertices is backfilled automatically on next
`codegraph build`.

P4 (incremental re-stitch of unchanged callers) is not implemented on the native
path — tracked in issue #1614.
…rted

Remove the 'intraprocedural only' limitation bullet. Replace with accurate
caveat about incremental re-stitch on the native engine path (#1614).
Update feature descriptions at lines 110 and 203 to mention the new
interprocedural edge kinds (arg_in, return_out, def_use).
…irectory nodes

extractDataflowAnalysis returns DataflowArgFlow with flat bindingType string
rather than the binding:{type,index} object that buildDataflowVerticesAndEdges
expects. Without normalisation, accessing af.binding.type crashes with
'Cannot read properties of undefined (reading type)' and causes the native
orchestrator to fall back to the JS pipeline on every incremental build,
leaving arg_in inter-procedural edges missing.

Fix: export patchDataflowResult from parser.ts and call it on each
DataflowResult from extractDataflowAnalysis in runDataflowVertexPass.

Also fixes the EISDIR error on full builds where the node query included
directory-kind nodes — filter those out so readFileSafe only reads files.
buildDataflowVerticesFromMap only collected stitch candidates from changed
files, leaving arg_in edges to recreated param vertices unconnected for
unchanged caller files. This caused incremental native builds to produce fewer
inter-procedural dataflow edges than full builds.

Fix: export collectCallerStitchCandidates and collectFuncIdsForFiles from
dataflow.ts so runDataflowVertexPass can invoke P4 re-stitch for unchanged
callers of changed functions, matching the behaviour already present in the
JS buildDataflowEdges path (issue #1614).
README.md:940 still said the incremental re-stitch pass only fires on the JS
engine path, but this PR adds it to the native vertex pass. Update the note
to reflect that P4 re-stitch now runs on both paths and closes issue #1614.

Also remove nativeDb from the wasmStubs buildDataflowEdges call: wasmStubs
have no .dataflow property so the native bulk-insert fast path is always
skipped for them, making nativeDb unused and its presence misleading.
… fk constraint

When migration 18 adds dataflow.call_edge_id REFERENCES edges(id), deleting
edges while dataflow rows still reference them via call_edge_id triggers
SQLITE_CONSTRAINT_FOREIGNKEY. This affects three code paths:

- JS incremental.ts: deleteOutgoingEdges for reverse-dep files
- Rust detect_changes.rs: purge_changed_files and reverse-dep edge deletion
- Rust pipeline.rs: reparse_barrel_candidates barrel edge deletion (which
  had PRAGMA foreign_keys ON from a prior full build on the same connection)
- Rust connection.rs: purgeFilesWithReverseDeps napi method

Also align Rust purge_sql ordering with JS purgeFileData: delete
dataflow_vertices and dataflow_summary before nodes to satisfy their fk.
…builds

On full builds the P6 vertex extraction pass previously called
extractDataflowAnalysis() for every file in the project, doubling the
effective analysis cost (Rust parses all files once, then JS re-parses
them all again). The regression measured +54-134% on the full build
benchmark against the 3.13.0 baseline.

Fix: scope filesToProcess to files that actually have dataflow edges
already written by the Rust orchestrator (flows_to/returns/mutates).
Native-language files with no dataflow edges produce zero vertices and
zero inter-procedural edges, so skipping them is safe. Non-native files
are always included so the WASM fallback path (buildDataflowEdges) can
write both edges and vertices for those languages.

Also adds KNOWN_REGRESSIONS entries for the remaining regression on the
3.13.0 baseline (the residual cost of processing files that DO have
dataflow edges, which is genuine new work) and for the fnDeps depth 1
query metric (CI runner variance on a sub-50ms native metric — the
fnDeps query path reads nodes/edges only and is unaffected by the
dataflow table additions).

greptile-apps Bot commented Jun 20, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR delivers two incremental improvements to the native-engine dataflow pipeline: it wraps the "P4 on native fast path" fixture in its own describe block to fix test-isolation, and removes the redundant patchDataflow wrapper in parser.ts so patchNativeResult calls patchDataflowResult directly.

  • src/domain/parser.ts: Deletes the one-liner patchDataflow wrapper and replaces every call site with the concrete patchDataflowResult, eliminating a needless indirection level.
  • tests/integration/dataflow-incremental.test.ts: Moves the module-level beforeAll/afterAll hooks and their shared variables inside the describe('P4 on native fast path', …) block, so a failure in that setup cannot cascade to unrelated test suites in the same file.

Confidence Score: 5/5

Both changes are low-risk cleanups: a one-liner wrapper removal and a test-scope fix; no production logic altered.

The parser change is a pure refactor — removing one forwarding function and calling its target directly. The test change correctly scopes setup/teardown to the describe block that owns those fixtures, reducing flakiness rather than introducing risk. Neither change touches any data-transformation or database logic.

No files require special attention.

Important Files Changed

Filename Overview
src/domain/parser.ts Removes the trivial patchDataflow wrapper and inlines patchDataflowResult at the single call site in patchNativeResult; purely mechanical, no behavioral change.
tests/integration/dataflow-incremental.test.ts Moves module-level beforeAll/afterAll + shared variables inside the describe('P4 on native fast path', …) block, fixing test isolation between suites; all five tests and assertions unchanged.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant O as NativeOrchestrator
    participant PN as patchNativeResult
    participant PD as patchDataflowResult

    O->>PN: r (raw native result)
    Note over PN: Before PR: PN called patchDataflow(),<br/>which forwarded to patchDataflowResult()
    PN->>PD: r.dataflow (direct call after PR)
    PD-->>PN: normalised argFlows/mutations
    PN-->>O: ExtractorOutput
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant O as NativeOrchestrator
    participant PN as patchNativeResult
    participant PD as patchDataflowResult

    O->>PN: r (raw native result)
    Note over PN: Before PR: PN called patchDataflow(),<br/>which forwarded to patchDataflowResult()
    PN->>PD: r.dataflow (direct call after PR)
    PD-->>PN: normalised argFlows/mutations
    PN-->>O: ExtractorOutput
Loading

Reviews (2): Last reviewed commit: "refactor(parser): remove patchDataflow o..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor Author

Addressed all 3 items from Greptile's review:

  1. Module-level beforeAll/afterAll (tests/integration/dataflow-incremental.test.ts): Moved both hooks inside the describe('P4 on native fast path', ...) block — a setup failure now only affects the native fast-path tests, not the pre-existing incremental re-stitch suite. Commit a46fcf34.
  2. patchDataflow one-liner (src/domain/parser.ts): Removed the wrapper, patchNativeResult now calls patchDataflowResult directly. Commit 77daca20.
  3. Vertex transaction / stitch atomicity (src/features/dataflow.ts): This is a pre-existing pattern that also exists in buildDataflowEdges's P6 block — unifying both paths under a single transaction boundary requires architectural changes beyond this PR's scope. Tracked in follow-up: make buildDataflowVerticesFromMap vertex write and inter-procedural stitch atomic #1640.

Copy link
Copy Markdown
Contributor Author

@greptileai

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

2 functions changed4 callers affected across 1 files

  • patchNativeResult in src/domain/parser.ts:742 (4 transitive callers)
  • patchNativeResult in src/domain/parser.ts:746 (4 transitive callers)

carlos-alm merged commit c2004b6 into main Jun 20, 2026
22 checks passed
carlos-alm deleted the feat/dataflow-p4-native branch June 20, 2026 20:09
github-actions Bot locked and limited conversation to collaborators Jun 20, 2026
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 free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL