iframe-proxy

carlos-alm · 2026-06-04T07:57:16Z

Summary

DB migration v17: adds technique TEXT nullable column + index to edges table
Edge tagging at insertion: call edges are tagged with the resolution technique that produced them:
- ts-native — direct static/same-file calls (native engine + JS direct resolution)
- points-to — parameter-flow pts post-pass and pts-fallback edges
- null — import, reexport, extends, implements, and receiver edges (not call edges)
PTS upgrade handling: when a pts-resolved edge is later superseded by a direct call, the technique is promoted from points-to to ts-native in-place
Native bulkInsertEdges path: after bulkInsertEdges (which doesn't write technique), a post-insert SQL UPDATE backfills techniques — pts edges by (source_id, target_id) first, then remaining NULL calls edges as ts-native baseline
Incremental builds: technique is saved and restored through reverse-dep edge reconnection
Stats output: callerCoverage.byTechnique is now included in codegraph stats --json and displayed in the human-readable output under the caller coverage line

JSON output shape

"caller_coverage": {
  "total": 5122,
  "resolved": 3585,
  "percentage": 70,
  "byTechnique": {
    "ts-native": 2940,
    "points-to": 645
  }
}

(type-propagation, cha-rta, and barrel are reserved labels for future work — they require threading resolution-source information through resolveCallTargets.)

Test plan

All 2832 tests pass (npx vitest run)
Build passes (npm run build)
Lint clean on changed files (npx biome check on staged files)

…-to analysis Walk assignment_expression nodes in extractTypeMapWalk (WASM) and match_js_type_map (native). When LHS is a simple member_expression (obj.prop) and RHS is an identifier, seed typeMap['obj.prop'] = { type: fn, confidence: 0.85 }. This covers patterns like: handlers.auth = authMiddleware; router.use(handlers.auth); // -> edge: caller -> authMiddleware Extend resolveByMethodOrGlobal (TS) and resolve_call_targets (Rust) with a composite key lookup (step 4.5): when a call has receiver + name, check typeMap['receiver.name'] for a direct pts target before falling through to the no-match return. Skips BUILTIN_GLOBALS / builtin objects (console, Math, etc.) and chained writes (a.b.c = x). Closes #1292

…BALS (#1295) Adds the four names present in Rust's is_js_builtin_global but absent from the TypeScript BUILTIN_GLOBALS set, restoring dual-engine parity for property-write pts tracking. Also adds prop.type guard in handlePropWriteTypeMap consistent with the adjacent fnRefBindings block.

…romotion test (#1295) Renames 'higher-confidence entry wins' to accurately describe equal-confidence first-write behavior. Removes stale inner comment. Adds explicit test for strict-higher-confidence promotion via setTypeMapEntry. Extends BUILTIN_GLOBALS test to cover process/window/document/globalThis.

…1295)

…S set (#1295)

…low-up) Closes #1300 - DB migration v17: adds `technique TEXT` column + index to `edges` table - Tags call edges at insertion: `ts-native` for direct/native-path calls, `points-to` for pts post-pass and pts-fallback edges; non-call edges get NULL - Upgrades pts edges to `ts-native` in-place when a direct call supersedes them - Native bulkInsertEdges path: technique is backfilled via a post-insert SQL UPDATE (pts edges targeted first; remaining NULL calls tagged ts-native as baseline) - Preserves technique through incremental-build reverse-dep edge reconnection - `computeQualityMetrics` / `buildStatsFromNative` expose `byTechnique` in `callerCoverage`; the JSON output and `printQuality` display it when present docs check acknowledged Impact: 20 functions changed, 23 affected

claude · 2026-06-04T07:57:35Z

Claude finished @carlos-alm's task —— View job

PR Review: feat(stats): by_technique breakdown in codegraph stats

Tasks

Root Cause Analysis

Problem Being Solved: The existing codegraph stats command reports aggregate caller coverage but gives no visibility into how calls were resolved — whether by direct static analysis (ts-native) or via the points-to alias resolution introduced in Phase 8.3 (points-to). This PR adds a technique TEXT column to edges and propagates technique labels through all build paths, then surfaces per-technique breakdowns in stats --json and the human-readable output.

Why This Approach: Tagging at insertion time (with a post-fill pass for the native bulk-insert path) is the right place to capture this — it is a property of the resolution method, not of the edge itself. The EdgeRowTuple is already the canonical in-memory representation flowing into every insert path, so extending it with a 6th element (technique) threads cleanly through the pipeline without touching query-layer code.

Risk Assessment: Medium. The primary risk is the applyEdgeTechniquesAfterNativeInsert post-fill pattern: it runs UPDATEs over potentially large edge sets after a bulk-insert and relies on the order of taggedRows iteration to avoid a race (pts edges must be stamped before the ts-native catch-all). There is a correctness bug here (detailed below). The incremental path via savedReverseDepEdges is correct: technique is now saved and restored through the full serialise/reconnect cycle.

Findings

🔴 Bug: `applyEdgeTechniquesAfterNativeInsert` — UPDATE-order race on duplicate `(source_id, target_id)` pairs

build-edges.ts:945-962

const stmt = db.prepare(
  "UPDATE edges SET technique = ? WHERE kind = 'calls' AND source_id = ? AND target_id = ? AND technique IS NULL",
);
for (const r of taggedRows) stmt.run(r[5], r[0], r[1]);

db.prepare(
  "UPDATE edges SET technique = 'ts-native' WHERE kind = 'calls' AND technique IS NULL",
).run();

The pts-edge UPDATE uses AND technique IS NULL. The native bulkInsertEdges call inserts all edges without technique, so every row starts with technique IS NULL. If the same (source_id, target_id) pair appears in taggedRows more than once (e.g. a pts edge for a call site that also has a promoted ts-native entry — the upgrade path in buildFileCallEdges at line 815 mutates the in-memory tuple, but in the native path the tuple is iterated over all calls in buildCallEdgesNative + buildParamFlowPtsPostPass), the first stmt.run(r[5], r[0], r[1]) stamps points-to, and the second call for the same pair (after the pts-edge was promoted to ts-native in the in-memory row) would be a no-op because technique IS NULL is now false. So the in-memory promotion of the technique from points-to to ts-native is not reflected in the DB for the native path.

Concretely: the JS path has a proper upgrade mechanism (ptsRow[5] = 'ts-native' at line 815) that fixes both the in-memory tuple and the DB value in a single insert. The native path's post-fill approximation does not replicate this promotion logic. The result is that the technique column in the native engine will occasionally report points-to for edges that were actually superseded by a direct static call.

🟡 Issue: `callerCoverage.byTechnique` counts edges, not callers

module-map.ts:166-175

function countCallEdgesByTechnique(db: BetterSqlite3Database): Record<string, number> {
  const rows = db
    .prepare(
      "SELECT technique, COUNT(*) as c FROM edges WHERE kind = 'calls' AND technique IS NOT NULL GROUP BY technique",
    )
    .all() as Array<{ technique: string; c: number }>;

The JSON key is callerCoverage.byTechnique, but this function counts call edges by technique — not functions with callers. So the label is misleading. If function A calls function B from two call sites, the ts-native count increases by 2, but the "callerCoverage" semantics should increase by 1 (B has at least one caller). The PR description's own example:

"byTechnique": { "ts-native": 2940, "points-to": 645 }

…sums to 3585, which matches resolved (the edge count), not covered (the unique-callee count). The field should be renamed to byTechnique under callEdges or a comment should clarify the unit.

🟡 Semantic issue: `byTechnique` count does not respect the `noTests` filter

module-map.ts:199-219

computeQualityMetrics receives a testFilter argument and passes it to callableWithCallers and totalCallable queries — but countCallEdgesByTechnique is a bare query with no test filter. When called with --no-tests, the callerCoverage ratio and byTechnique will be computed from different populations, making the technique counts inconsistent with the coverage percentages above them.

Fix: pass testFilter to countCallEdgesByTechnique.

🟡 `applyEdgeTechniquesAfterNativeInsert` — unbounded UPDATE without `LIMIT`

build-edges.ts:957-960

db.prepare(
  "UPDATE edges SET technique = 'ts-native' WHERE kind = 'calls' AND technique IS NULL",
).run();

On a large codebase this is a full-table scan updating every untagged calls edge. There is no LIMIT or batching. For a codebase with tens of thousands of call edges (not uncommon given the stats sample in the PR — 3585 resolved out of 5122), this will briefly write-lock the SQLite DB and produce a large journal entry. The JS path does not have this issue because technique is set per-row at insert time. This is a performance concern, not a correctness one, but worth noting.

🟢 DB migration: correct

Migration v17 (migrations.ts:260-265) is correct: ALTER TABLE edges ADD COLUMN technique TEXT (nullable, no DEFAULT) and a covering index. No DEFAULT means existing rows are NULL, which is the correct baseline — older edges have no known technique. ensureEdgeColumns was not updated to backfill the legacy compat path, but that is acceptable since old DBs are migrated via applyMigrations before ensureLegacyColumns runs, and the new column is nullable.

🟢 `EdgeRowTuple` shape change

helpers.ts:316 — The 6th element was previously absent; it is now always present as technique: string | null. The getEdgeStmt prepared statement correctly uses (?,?,?,?,?,?) and batchInsertEdges passes r[5] ?? null. This is a clean, correct extension.

🟢 Test for issue #1292

tests/integration/issue-1292-property-write-pts.test.ts — The new integration test correctly exercises the property-write pts path introduced in Phase 8.3d. The dynamic: 1 assertion is appropriate (calls go through the pts fallback, which uses isDynamic from the original call site). The test does not assert on technique, which is fine (that would be testing the stats output layer, not the resolution layer).

🟢 Incremental build: `technique` round-tripped correctly

detect-changes.ts:418 — The saveEdgesStmt SELECT now includes e.technique. context.ts:92 adds technique: string | null to the saved edge shape. build-edges.ts:997 restores it in reconnectReverseDepEdges. This is thorough and correct.

🟢 No config-rule relaxation, no test weakening

No linting rules were disabled, no test expectations were lowered, no coverage thresholds were changed. Pass.

Backlog Compliance

Zero-dep: ✓ — no new runtime dependencies; extends tree-sitter/SQLite/existing AST pipeline
Foundation-aligned: ✓ — purely local, no cloud calls, no code generation, no infrastructure requirements; adds diagnostic data to the existing stats output without changing the programmatic API contract
Problem-fit: 3/5 — useful observability for understanding resolution quality, but doesn't directly reduce token waste for AI agents or catch blast-radius breakage. It's useful metadata that helps maintainers tune the resolver.
Breaking: No — byTechnique is additive to the JSON output (optional field). The edges schema change is purely additive (nullable column, existing rows NULL). DB schema version bump from 16→17 is handled gracefully.
Tier: 1 (zero-dep + foundation-aligned)

Critical Concerns

applyEdgeTechniquesAfterNativeInsert promotion bug (medium severity): When a pts edge is promoted to ts-native in the native path, the post-fill UPDATE does not replicate the in-memory promotion. Edges that were promoted from points-to to ts-native will be incorrectly tagged as points-to in the DB. This means the byTechnique stat will over-count points-to for the native engine path.
byTechnique ignores noTests filter (low-medium severity): The technique counts in callerCoverage are computed without the test file filter, making them inconsistent with the other stats when --no-tests is used.
Misleading label (low severity): callerCoverage.byTechnique counts call edges, not callers. Consider renaming to resolvedEdges.byTechnique or clarifying in docs.

Final Recommendation

Rating: ⭐⭐⭐ (3/5)
Action: REQUEST CHANGES
Reasoning: The core idea is solid and the implementation is mostly correct. The JS path (direct insert) is clean. The DB migration, incremental path, and stats presentation are all well-done. However, the native path's technique post-fill has a real correctness bug (pts→ts-native promotions are lost), and the byTechnique filter inconsistency with --no-tests will silently produce misleading stats. These should be fixed before merge. The misleading label is a cosmetic concern that can be addressed alongside.

greptile-apps · 2026-06-04T08:03:03Z

Greptile Summary

This PR adds a technique column (migration v17) to the edges table and uses it to tag call edges with their resolution strategy (ts-native, points-to, or null for non-call edges). The callerCoverage.byTechnique breakdown is surfaced in both the JSON and human-readable codegraph stats output.

Schema + tagging: EdgeRowTuple gains a 6th slot; all edge-emitting sites are updated inline. batchInsertEdges writes technique directly; the native bulkInsertEdges path backfills via applyEdgeTechniquesAfterNativeInsert, scoped to the batch's source IDs to avoid mis-tagging pre-migration edges from unchanged files.
Native orchestrator path: backfillEdgeTechniquesAfterNativeOrchestrator applies a global UPDATE on full builds, a no-op on quiet incrementals, and a changed-file-scoped subquery UPDATE on normal incrementals — correctly addressing both previously identified global-UPDATE mis-tagging issues.
Incremental safety: technique is now saved and restored through the reverse-dep edge purge/reconnect cycle, so technique labels survive incremental builds without reparsing unchanged files.

Confidence Score: 5/5

Safe to merge. The technique backfill logic is well-scoped, the two global-UPDATE mis-tagging issues from the prior review cycle are both resolved, and technique values survive incremental purge/reconnect correctly.

All three insert paths (JS batch, native bulkInsert in build-edges, native Rust orchestrator) now correctly tag or backfill technique on call edges. The incremental-safety properties (quiet-incremental no-op, changed-file scoping, reverse-dep technique round-trip) are all addressed. The migration is additive and nullable with no risk of breaking existing data.

No files require special attention.

Important Files Changed

Filename	Overview
src/db/migrations.ts	Adds migration v17: nullable technique TEXT column and index on edges. Consistent with existing migration pattern; index supports GROUP BY in stats queries.
src/domain/graph/builder/stages/build-edges.ts	EdgeRowTuple extended to 6-tuple; all push sites updated; pts supersession promotion correct; applyEdgeTechniquesAfterNativeInsert scoped to current-batch source IDs, preventing mis-tagging of pre-migration edges from unchanged files.
src/domain/graph/builder/stages/native-orchestrator.ts	backfillEdgeTechniquesAfterNativeOrchestrator correctly distinguishes full builds (global UPDATE), quiet incrementals (early return), and file-changing incrementals (subquery-scoped UPDATE). Called after all edge-writing phases.
src/domain/analysis/module-map.ts	Adds countCallEdgesByTechnique helper; byTechnique correctly omitted when empty; buildStatsFromNative now receives noTests parameter and constructs testFilter consistently with the JS path.
src/domain/graph/builder/context.ts	Adds technique: string
src/domain/graph/builder/stages/detect-changes.ts	Adds e.technique to the reverse-dep edge save query and propagates it through the row type so technique survives the purge/reconnect cycle.
src/domain/graph/builder/helpers.ts	Extends batchInsertEdges and prepared-statement cache to include the 6th technique column; uses r[5] ?? null safely.
src/presentation/queries-cli/overview.ts	Extends QualityInfo type with optional byTechnique and prints the technique breakdown sorted by count, guarded by Object.keys().length > 0.
tests/integration/issue-1292-property-write-pts.test.ts	Adds technique assertions that correctly branch on engine (ts-native for native, points-to for JS). Both test cases now verify technique tagging end-to-end.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Edge rows built in-memory"] --> B{Insert path?}
    B -->|JS batchInsertEdges| C["technique written directly to DB"]
    B -->|Native bulkInsertEdges| D["Edges inserted, technique = NULL"]
    D --> E["applyEdgeTechniquesAfterNativeInsert scoped to batch sourceIds"]
    E --> F["UPDATE: explicit technique rows first"]
    F --> G["Catch-all UPDATE: remaining NULL calls tagged ts-native"]
    H["Native Rust Orchestrator"] --> I["backfillEdgeTechniquesAfterNativeOrchestrator"]
    I --> J{Build type?}
    J -->|Full build| K["Global UPDATE all NULL calls"]
    J -->|Quiet incremental| L["No-op"]
    J -->|Incremental with changes| M["Scoped UPDATE via subquery"]
    N["Reverse-dep edge save"] --> O["technique saved"]
    O --> P["Reconnect: technique restored"]
    P --> B
    C --> Q["countCallEdgesByTechnique"]
    G --> Q
    K --> Q
    M --> Q
    Q --> R["callerCoverage.byTechnique in stats"]

_{Reviews (5): Last reviewed commit: "fix(stats): guard quiet incremental in b..." | Re-trigger Greptile}

github-actions · 2026-06-04T08:05:10Z

Codegraph Impact Analysis

23 functions changed → 24 callers affected across 13 files

countCallEdgesByTechnique in src/domain/analysis/module-map.ts:166 (5 transitive callers)
computeQualityMetrics in src/domain/analysis/module-map.ts:186 (3 transitive callers)
buildStatsFromNative in src/domain/analysis/module-map.ts:410 (3 transitive callers)
statsData in src/domain/analysis/module-map.ts:524 (2 transitive callers)
PipelineContext in src/domain/graph/builder/context.ts:21 (5 transitive callers)
getEdgeStmt in src/domain/graph/builder/helpers.ts:308 (6 transitive callers)
batchInsertEdges in src/domain/graph/builder/helpers.ts:349 (6 transitive callers)
emitTypeOnlySymbolEdges in src/domain/graph/builder/stages/build-edges.ts:117 (3 transitive callers)
emitEdgesForImport in src/domain/graph/builder/stages/build-edges.ts:143 (3 transitive callers)
buildBarrelEdges in src/domain/graph/builder/stages/build-edges.ts:188 (3 transitive callers)
buildImportEdgesNative in src/domain/graph/builder/stages/build-edges.ts:370 (3 transitive callers)
buildCallEdgesNative in src/domain/graph/builder/stages/build-edges.ts:458 (3 transitive callers)
buildParamFlowPtsPostPass in src/domain/graph/builder/stages/build-edges.ts:541 (3 transitive callers)
buildFileCallEdges in src/domain/graph/builder/stages/build-edges.ts:783 (3 transitive callers)
buildClassHierarchyEdges in src/domain/graph/builder/stages/build-edges.ts:912 (3 transitive callers)
applyEdgeTechniquesAfterNativeInsert in src/domain/graph/builder/stages/build-edges.ts:960 (4 transitive callers)
reconnectReverseDepEdges in src/domain/graph/builder/stages/build-edges.ts:997 (3 transitive callers)
buildEdges in src/domain/graph/builder/stages/build-edges.ts:1130 (4 transitive callers)
purgeAndAddReverseDeps in src/domain/graph/builder/stages/detect-changes.ts:379 (4 transitive callers)
backfillEdgeTechniquesAfterNativeOrchestrator in src/domain/graph/builder/stages/native-orchestrator.ts:751 (4 transitive callers)

…lter (#1303) - applyEdgeTechniquesAfterNativeInsert: scope the catch-all 'ts-native' UPDATE to source_ids in the current batch — prevents mis-tagging pre-migration NULL-technique edges from unchanged files on the first incremental build after a v16->v17 migration - countCallEdgesByTechnique: accept testFilter (JOIN on source node) so byTechnique counts are consistent with --no-tests - buildStatsFromNative: thread noTests into countCallEdgesByTechnique

) The Rust orchestrator writes edges without the technique column. Add a post-orchestrator step that tags all new 'calls' edges as 'ts-native': full builds use a global UPDATE, incremental builds scope to changed-file source nodes to avoid overwriting existing technique values.

- readCallEdges: SELECT e.technique in addition to existing columns - readEngine: read build_meta engine to detect native vs WASM path - Assertions check technique is 'ts-native' (native orchestrator) or 'points-to' (JS path), confirming the technique backfill is working

carlos-alm · 2026-06-04T09:16:06Z

Addressed all findings from the review:

Bug: Global UPDATE mis-tags pre-migration pts edges (P1 from Greptile, Red from Claude)
Fixed in two commits (d9947a6, 1636527):

applyEdgeTechniquesAfterNativeInsert: the catch-all 'ts-native' UPDATE is now scoped to source_id IN (batch sourceIds) — prevents contaminating unchanged files' edges on incremental builds after v16→v17 migration
Added backfillEdgeTechniquesAfterNativeOrchestrator in native-orchestrator.ts: the full Rust orchestrator (which handles most builds) was never calling the technique backfill at all, leaving all edges with technique = NULL. This new step runs after all edge-writing phases (including WASM backfill for dropped languages) and tags new 'calls' edges as 'ts-native' — global for full builds, scoped to changed-file source nodes for incremental builds

Issue: byTechnique ignores noTests filter (Yellow from Claude)
Fixed in d9947a6:

countCallEdgesByTechnique now accepts a testFilter string parameter and JOINs source nodes to apply the file filter
buildStatsFromNative now receives noTests and derives testFilter before calling countCallEdgesByTechnique

Issue: Integration test missing technique assertions (P2 from Greptile)
Fixed in 2769436:

readCallEdges now SELECT e.technique
Added readEngine helper to detect native vs JS build path from build_meta
Both test cases assert technique is 'ts-native' (native orchestrator) or 'points-to' (JS path)

Misleading label callerCoverage.byTechnique (Low/cosmetic, Claude)
The current placement under callerCoverage is intentional — it shows how the 'covered' subset was resolved, making it contextually useful. Renaming to resolvedEdges.byTechnique would break the JSON output shape that was just introduced. Tracked as a follow-up to address in a future stats restructuring PR.

carlos-alm · 2026-06-04T09:16:25Z

@greptileai

carlos-alm · 2026-06-04T09:16:29Z

@claude

claude · 2026-06-04T09:16:45Z

Claude finished @carlos-alm's task —— View job

Re-review: feat(stats): by_technique breakdown in codegraph stats

Tasks

Gather PR context
Read updated source files
Verify fixes address reported issues
Write final review

Follow-up Review

All three substantive issues from the initial review have been correctly addressed. Here is a targeted assessment of each fix:

Fix 1: Global UPDATE scoped in `applyEdgeTechniquesAfterNativeInsert`

build-edges.ts:947-971

Correct. The catch-all ts-native UPDATE now collects sourceIds from the current batch and adds AND source_id IN (${placeholders}). Pre-existing NULL-technique edges from unchanged files are no longer touched on incremental builds after a v16→v17 migration.

One minor observation: the taggedRows first-pass UPDATE (AND technique IS NULL) still carries the upgrade-promotion gap from the original review (pts-promoted-to-ts-native tuples). However, since applyEdgeTechniquesAfterNativeInsert is only called from reconnectReverseDepEdges, not from the main native path (the Rust orchestrator path now uses the separate backfillEdgeTechniquesAfterNativeOrchestrator), the scenario where an in-memory promotion is not reflected in the DB is significantly less likely. The reconnect path operates on savedReverseDepEdges, which are pure restore-from-DB entries (not pts-promoted tuples from the call resolver), so the practical impact is minimal.

Fix 2: `backfillEdgeTechniquesAfterNativeOrchestrator` in native-orchestrator.ts

native-orchestrator.ts:751-772 and line 975

Correct and well-structured. The function correctly distinguishes full builds (global UPDATE — safe, all rows are newly inserted) from incremental builds (scoped UPDATE via source_id IN (SELECT id FROM nodes WHERE file IN (...)) subquery). The placement at line 975 — after both backfillNativeDroppedFiles and any structure/analysis fallbacks — ensures every edge-writing phase in the native path is covered before the technique stamp runs.

The subquery approach (AND source_id IN (SELECT id FROM nodes WHERE file IN (?))) is preferable to the batch-of-IDs approach used in applyEdgeTechniquesAfterNativeInsert, as it avoids loading all node IDs into JS for large changesets.

Fix 3: `countCallEdgesByTechnique` respects `testFilter`

module-map.ts:166-184

Correct. The function now accepts testFilter: string and joins nodes n ON e.source_id = n.id to apply n.file-based filtering. Both computeQualityMetrics (JS path) and buildStatsFromNative (native path) now derive testFilter consistently before calling this function. The --no-tests filter is now consistent across the callerCoverage ratio and the byTechnique breakdown.

Fix 4: Integration test technique assertions

tests/integration/issue-1292-property-write-pts.test.ts:49-111

Correct. readCallEdges now selects e.technique. The readEngine helper correctly reads build_meta.engine. The test branches on engine === 'native' and asserts ts-native vs points-to accordingly, which reflects the actual behavior difference between the two paths (the native Rust orchestrator doesn't distinguish pts from direct calls at the edge-tagging level).

The comment at line 93-94 is accurate: "The native orchestrator resolves pts edges via the typeMap (which includes the property-write seed) but labels them 'ts-native' since Rust doesn't distinguish resolution strategy." This is honest documentation of an engine behavioral difference, not a parity gap being papered over.

Remaining Low-Severity Issue

Misleading label callerCoverage.byTechnique (unchanged from initial review)

The field counts call edges, not callers. The PR author noted this is intentional for now and will be addressed in a future stats restructuring PR. This is acceptable — the JSON shape was just introduced in this PR and there's no external API contract yet, so deferring the rename is a reasonable call.

Verdict

The critical bug (global UPDATE mis-tagging) and the filter-consistency issue are both fixed correctly. The integration test now has meaningful technique assertions. No new issues introduced.

Rating: ✅ Approved

The PR is ready to merge.

View job

Quiet incremental builds (no files changed) insert no new edges — running the global UPDATE would mis-tag pre-migration NULL-technique edges from unchanged files as 'ts-native'. Return early when changedFiles is defined but empty.

carlos-alm · 2026-06-04T19:19:29Z

@greptileai

carlos-alm · 2026-06-04T19:19:39Z

carlos-alm added 7 commits June 3, 2026 01:30

fix: resolve merge conflicts with main

d069a13

fix(resolver): fall through on empty pts results in step 4.5 TS path (#…

db1beb6

…1295)

fix(extractor): sync is_js_builtin_global with full TS BUILTIN_GLOBAL…

be99382

…S set (#1295)

greptile-apps Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread src/domain/graph/builder/stages/build-edges.ts Outdated

carlos-alm added 3 commits June 4, 2026 03:15

chore: merge origin/main into feat/phase-8-3d-property-write-pts (#1303)

59bd571

greptile-apps Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread src/domain/graph/builder/stages/native-orchestrator.ts Outdated

carlos-alm added 2 commits June 4, 2026 13:16

fix: resolve merge conflicts with main

f79a7e7

carlos-alm merged commit c2959a1 into main Jun 4, 2026
26 checks passed

carlos-alm deleted the feat/phase-8-3d-property-write-pts branch June 4, 2026 22:56

github-actions Bot locked and limited conversation to collaborators Jun 4, 2026

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(stats): by_technique breakdown in codegraph stats (Phase 8.6 follow-up)#1303

feat(stats): by_technique breakdown in codegraph stats (Phase 8.6 follow-up)#1303
carlos-alm merged 13 commits into
mainfrom
feat/phase-8-3d-property-write-pts

carlos-alm commented Jun 4, 2026

Uh oh!

claude Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

github-actions Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

claude Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

carlos-alm commented Jun 4, 2026

Summary

JSON output shape

Test plan

Uh oh!

claude Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: feat(stats): by_technique breakdown in codegraph stats

Root Cause Analysis

Findings

🔴 Bug: applyEdgeTechniquesAfterNativeInsert — UPDATE-order race on duplicate (source_id, target_id) pairs

🟡 Issue: callerCoverage.byTechnique counts edges, not callers

🟡 Semantic issue: byTechnique count does not respect the noTests filter

🟡 applyEdgeTechniquesAfterNativeInsert — unbounded UPDATE without LIMIT

🟢 DB migration: correct

🟢 EdgeRowTuple shape change

🟢 Test for issue #1292

🟢 Incremental build: technique round-tripped correctly

🟢 No config-rule relaxation, no test weakening

Backlog Compliance

Critical Concerns

Final Recommendation

Uh oh!

greptile-apps Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

github-actions Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codegraph Impact Analysis

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

claude Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-review: feat(stats): by_technique breakdown in codegraph stats

Follow-up Review

Fix 1: Global UPDATE scoped in applyEdgeTechniquesAfterNativeInsert

Fix 2: backfillEdgeTechniquesAfterNativeOrchestrator in native-orchestrator.ts

Fix 3: countCallEdgesByTechnique respects testFilter

Fix 4: Integration test technique assertions

Remaining Low-Severity Issue

Verdict

Uh oh!

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

carlos-alm commented Jun 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jun 4, 2026 •

edited

Loading

🔴 Bug: `applyEdgeTechniquesAfterNativeInsert` — UPDATE-order race on duplicate `(source_id, target_id)` pairs

🟡 Issue: `callerCoverage.byTechnique` counts edges, not callers

🟡 Semantic issue: `byTechnique` count does not respect the `noTests` filter

🟡 `applyEdgeTechniquesAfterNativeInsert` — unbounded UPDATE without `LIMIT`

🟢 `EdgeRowTuple` shape change

🟢 Incremental build: `technique` round-tripped correctly

greptile-apps Bot commented Jun 4, 2026 •

edited

Loading

github-actions Bot commented Jun 4, 2026 •

edited

Loading

claude Bot commented Jun 4, 2026 •

edited

Loading

Fix 1: Global UPDATE scoped in `applyEdgeTechniquesAfterNativeInsert`

Fix 2: `backfillEdgeTechniquesAfterNativeOrchestrator` in native-orchestrator.ts

Fix 3: `countCallEdgesByTechnique` respects `testFilter`