feat: add git co-change analysis (backlog #9)#95
Conversation
Analyze git history to surface files that historically change together, using Jaccard similarity coefficients. Results are stored in the DB and integrated into diff-impact to catch temporal coupling the static graph misses. - New src/cochange.js module with scan, compute, analyze, and query functions - DB migration v5: co_changes + co_change_meta tables - CLI: `codegraph co-change [file]` with --analyze, --since, --min-support, etc. - diff-impact now shows historicallyCoupled files when co-change data exists - MCP: new co_changes tool for AI agent access - 19 new tests covering pure logic, DB integration, and real git repos Impact: 13 functions changed, 9 affected
|
Claude finished @carlos-alm's task —— View job PR Review: Git Co-Change AnalysisTODO List:
Root Cause AnalysisProblem Being Solved: This PR addresses the gap between static dependency graphs and runtime change coupling by analyzing git history to surface files that historically change together. This is valuable for impact analysis - when you change file A, which other files might be affected even if there's no static import relationship? Why This Approach: The Jaccard similarity coefficient is a solid choice for measuring co-change coupling. The approach scans git commit history, counts co-occurrences of file pairs, and computes Jaccard similarity (intersection/union). Integration into Risk Assessment: This is a legitimate feature addition solving a real problem. The approach is mathematically sound and commonly used in software engineering research. Critical Concerns✅ No critical concerns found All phases passed inspection:
Final AssessmentConditions for approval:
This is exemplary work - solid algorithm implementation, comprehensive test coverage, security best practices, and clean integration. The co-change analysis will provide valuable insights for developers working with complex codebases. |
Greptile SummaryThis PR adds git co-change analysis to surface files that historically change together using Jaccard similarity coefficients. The implementation scans git commit history, computes temporal coupling between files, and stores results in SQLite for fast querying. Major changes:
Key implementation details:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TB
CLI[CLI: codegraph co-change --analyze]
Scan[scanGitHistory: Parse git log]
Compute[computeCoChanges: Calculate Jaccard]
DB[(SQLite DB)]
FileCount[file_commit_counts table]
CoChange[co_changes table]
Meta[co_change_meta table]
Query[Query Functions]
DiffImpact[diff-impact integration]
MCP[MCP Server]
CLI --> Scan
Scan --> Compute
Compute --> FileCount
Compute --> CoChange
Compute --> Meta
FileCount --> |Recompute Jaccard| CoChange
CoChange --> Query
CoChange --> DiffImpact
CoChange --> MCP
Query --> |coChangeData| CLI
Query --> |coChangeTopData| CLI
DiffImpact --> |coChangeForFiles| Query
MCP --> |co_changes tool| Query
Last reviewed commit: e2a771b |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/queries.js
Outdated
| const changedFilesList = [...changedRanges.keys()]; | ||
| const staticFiles = new Set([...changedRanges.keys(), ...affectedFiles]); | ||
| const placeholders = changedFilesList.map(() => '?').join(','); | ||
| const coRows = db | ||
| .prepare( | ||
| `SELECT file_a, file_b, commit_count, jaccard | ||
| FROM co_changes | ||
| WHERE (file_a IN (${placeholders}) OR file_b IN (${placeholders})) | ||
| AND jaccard >= 0.3 | ||
| ORDER BY jaccard DESC | ||
| LIMIT 20`, | ||
| ) | ||
| .all(...changedFilesList, ...changedFilesList); |
There was a problem hiding this comment.
duplicates logic from coChangeForFiles function. Consider using the existing function instead:
| const changedFilesList = [...changedRanges.keys()]; | |
| const staticFiles = new Set([...changedRanges.keys(), ...affectedFiles]); | |
| const placeholders = changedFilesList.map(() => '?').join(','); | |
| const coRows = db | |
| .prepare( | |
| `SELECT file_a, file_b, commit_count, jaccard | |
| FROM co_changes | |
| WHERE (file_a IN (${placeholders}) OR file_b IN (${placeholders})) | |
| AND jaccard >= 0.3 | |
| ORDER BY jaccard DESC | |
| LIMIT 20`, | |
| ) | |
| .all(...changedFilesList, ...changedFilesList); | |
| const changedFilesList = [...changedRanges.keys()]; | |
| const staticFiles = new Set([...changedRanges.keys(), ...affectedFiles]); | |
| const { coChangeForFiles } = await import('./cochange.js'); | |
| const coResults = coChangeForFiles(changedFilesList, db, { | |
| minJaccard: 0.3, | |
| limit: 20, | |
| noTests | |
| }); | |
| for (const result of coResults) { | |
| if (!staticFiles.has(result.file)) { | |
| historicallyCoupled.push(result); | |
| } | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in aef1787 — replaced the inline SQL with a call to coChangeForFiles, which also handles the empty-array guard (returns [] early when files.length === 0).
src/queries.js
Outdated
| const changedFilesList = [...changedRanges.keys()]; | ||
| const staticFiles = new Set([...changedRanges.keys(), ...affectedFiles]); | ||
| const placeholders = changedFilesList.map(() => '?').join(','); | ||
| const coRows = db | ||
| .prepare( | ||
| `SELECT file_a, file_b, commit_count, jaccard | ||
| FROM co_changes | ||
| WHERE (file_a IN (${placeholders}) OR file_b IN (${placeholders})) | ||
| AND jaccard >= 0.3 | ||
| ORDER BY jaccard DESC | ||
| LIMIT 20`, | ||
| ) | ||
| .all(...changedFilesList, ...changedFilesList); |
There was a problem hiding this comment.
empty changedFilesList will create invalid SQL (WHERE file_a IN () OR file_b IN ()). Add a check:
| const changedFilesList = [...changedRanges.keys()]; | |
| const staticFiles = new Set([...changedRanges.keys(), ...affectedFiles]); | |
| const placeholders = changedFilesList.map(() => '?').join(','); | |
| const coRows = db | |
| .prepare( | |
| `SELECT file_a, file_b, commit_count, jaccard | |
| FROM co_changes | |
| WHERE (file_a IN (${placeholders}) OR file_b IN (${placeholders})) | |
| AND jaccard >= 0.3 | |
| ORDER BY jaccard DESC | |
| LIMIT 20`, | |
| ) | |
| .all(...changedFilesList, ...changedFilesList); | |
| const changedFilesList = [...changedRanges.keys()]; | |
| if (changedFilesList.length === 0) { | |
| // Skip co-change lookup if no files changed | |
| } else { | |
| const staticFiles = new Set([...changedRanges.keys(), ...affectedFiles]); | |
| const placeholders = changedFilesList.map(() => '?').join(','); | |
| const coRows = db | |
| .prepare( | |
| `SELECT file_a, file_b, commit_count, jaccard | |
| FROM co_changes | |
| WHERE (file_a IN (${placeholders}) OR file_b IN (${placeholders})) | |
| AND jaccard >= 0.3 | |
| ORDER BY jaccard DESC | |
| LIMIT 20`, | |
| ) | |
| .all(...changedFilesList, ...changedFilesList); |
There was a problem hiding this comment.
Fixed in aef1787 — reusing coChangeForFiles which already returns [] when the input array is empty, so the invalid SQL is no longer possible.
Replace inline co-change SQL query with a call to the existing coChangeForFiles helper, fixing both duplicated logic and a potential empty-array SQL error (WHERE file_a IN ()). Addresses Greptile review feedback on PR #95.
|
Claude finished @carlos-alm's task —— View job PR Review: Git Co-Change AnalysisTODO List:
Root Cause AnalysisProblem Being Solved: This PR addresses the gap between static dependency graphs and runtime change coupling by analyzing git history to surface files that historically change together. This is valuable for impact analysis - when you change file A, which other files might be affected even if there's no static import relationship? Why This Approach: The Jaccard similarity coefficient is a solid choice for measuring co-change coupling. The approach scans git commit history, counts co-occurrences of file pairs, and computes Jaccard similarity (intersection/union). Integration into Risk Assessment: This is a legitimate feature addition solving a real problem. The approach is mathematically sound and commonly used in software engineering research. Critical Concerns (if any)✅ No critical concerns found All phases passed inspection:
Final RecommendationConditions for approval:
This is exemplary work - solid algorithm implementation, comprehensive test coverage, security best practices, and clean integration. The co-change analysis will provide valuable insights for developers working with complex codebases. Note on Greptile's feedback: Greptile incorrectly flagged a "critical" SQL injection issue that doesn't exist due to proper guard clauses. The code duplication concern is valid but minor. |
src/cochange.js
Outdated
| commit_count = commit_count + excluded.commit_count, | ||
| jaccard = excluded.jaccard, |
There was a problem hiding this comment.
incremental Jaccard calculation is incorrect - it's computed only from new commits but should reflect all commits (old + new). When running incremental analysis twice, the Jaccard value gets overwritten with a value computed from just the newest commits, not the full history.
Example: if files A+B co-changed 10 times historically (Jaccard=0.8), then 2 more times in new commits (Jaccard=0.5 for just those 2), the final Jaccard becomes 0.5 instead of the correct value based on all 12 commits.
To fix, you'd need to either:
- recompute Jaccard from scratch on each run (requires storing individual file commit counts)
- use
--fullflag for re-scans (but this defeats incremental purpose) - redesign to store enough data to recalculate Jaccard correctly during merges
There was a problem hiding this comment.
Fixed in e2a771b — added a file_commit_counts table (migration v6) that accumulates per-file commit totals across incremental runs. After upserting pair counts, Jaccard is recomputed from the total file counts: pair_count / (count_a + count_b - pair_count). The --full flag clears this table along with the others.
…hange analysis The incremental upsert was overwriting Jaccard with a value computed only from new commits, ignoring historical data. Now stores per-file commit counts in a new file_commit_counts table (migration v6) and recomputes Jaccard from the accumulated totals after each merge.
New Mermaid export output (from PR #97)The enhanced flowchart LR
subgraph crates_codegraph_core_src["crates/codegraph-core/src"]
n0["import_resolution.rs"]
n1["lib.rs"]
n2["parallel.rs"]
end
subgraph crates_codegraph_core_src_extractors["crates/codegraph-core/src/extractors"]
n3["csharp.rs"]
n4["go.rs"]
n5["hcl.rs"]
n6["helpers.rs"]
n7["java.rs"]
n8["javascript.rs"]
n9["php.rs"]
n10["python.rs"]
n11["ruby.rs"]
n12["rust_lang.rs"]
end
subgraph scripts["scripts"]
n13["benchmark.js"]
n14["gen-deps.cjs"]
n15["incremental-benchmark.js"]
n16["query-benchmark.js"]
n17["update-embedding-report.js"]
n18["update-incremental-report.js"]
n19["update-query-report.js"]
n20["embedding-benchmark.js"]
end
subgraph src["src"]
n21["cli.js"]
n22["export.js"]
n23["constants.js"]
n24["cycles.js"]
n25["journal.js"]
n26["logger.js"]
n27["native.js"]
n28["registry.js"]
n29["resolve.js"]
n30["watcher.js"]
n31["config.js"]
n32["builder.js"]
n33["db.js"]
n34["embedder.js"]
n35["mcp.js"]
n36["parser.js"]
n37["queries.js"]
n38["structure.js"]
end
subgraph src_extractors["src/extractors"]
n39["index.js"]
n40["helpers.js"]
n41["javascript.js"]
end
subgraph tests_fixtures_sample_project["tests/fixtures/sample-project"]
n42["index.js"]
n43["math.js"]
n44["utils.js"]
end
n3 -->|calls| n6
n4 -->|calls| n6
n5 -->|calls| n6
n7 -->|calls| n6
n8 -->|calls| n6
n9 -->|calls| n6
n10 -->|calls| n6
n11 -->|calls| n6
n12 -->|calls| n6
n1 -->|calls| n2
n1 -->|calls| n0
n13 -->|calls| n26
n13 -->|calls| n27
n14 -->|calls| n26
n15 -->|calls| n26
n15 -->|calls| n27
n15 -->|calls| n29
n16 -->|calls| n26
n16 -->|calls| n27
n17 -->|calls| n26
n18 -->|calls| n26
n19 -->|calls| n26
n24 -->|calls| n27
n25 -->|calls| n26
n28 -->|calls| n26
n29 -->|calls| n23
n29 -->|calls| n27
n30 -->|calls| n23
n30 -->|calls| n26
n30 -->|calls| n29
n30 -->|calls| n25
n42 -->|calls| n43
n42 -->|calls| n44
n44 -->|calls| n43
n31 -->|calls| n26
n20 -->|calls| n34
n32 -->|calls| n26
n32 -->|calls| n23
n32 -->|calls| n25
n32 -->|calls| n33
n32 -->|calls| n31
n32 -->|calls| n36
n32 -->|calls| n29
n32 -->|calls| n38
n32 -->|calls| n28
n33 -->|calls| n26
n34 -->|calls| n33
n34 -->|calls| n26
n41 -->|calls| n40
n35 -->|calls| n28
n35 -->|calls| n37
n35 -->|calls| n33
n35 -->|calls| n24
n35 -->|calls| n34
n35 -->|calls| n38
n36 -->|calls| n26
n36 -->|calls| n27
n37 -->|calls| n33
n37 -->|calls| n24
n37 -->|calls| n26
n38 -->|calls| n23
n38 -->|calls| n26
n38 -->|calls| n33
n38 -->|calls| n37
n21 -->|calls| n31
n21 -->|calls| n26
n21 -->|calls| n32
n21 -->|calls| n37
n21 -->|calls| n33
n21 -->|calls| n22
n21 -->|calls| n24
n21 -->|calls| n35
n21 -->|calls| n28
n21 -->|calls| n34
n21 -->|calls| n38
n21 -->|calls| n30
n21 -->|calls| n36
n21 -->|calls| n27
n22 -->|calls| n37
n36 -->|imports| n39
n37 -->|imports| n36
|
|
Claude finished @carlos-alm's task —— View job PR Review: Git Co-Change AnalysisTODO List:
Root Cause AnalysisProblem Being Solved: Bridging static dependency analysis with historical change coupling using Jaccard similarity on git commit history - addresses real gap in impact analysis Critical Concerns (if any)None identified. This is a well-implemented, thoroughly tested feature addition. Final Recommendation
Conditions for approval:
This is exemplary work - solid algorithm implementation, comprehensive test coverage, security best practices, and clean integration. The co-change analysis will provide valuable insights for developers working with complex codebases. Note on Greptile's feedback: Greptile incorrectly flagged a "critical" incremental Jaccard issue. Upon review, the implementation correctly accumulates file/pair counts in separate tables and recomputes Jaccard from total historical data (lines 193-232 in |
Diff-impact Mermaid for PR #97 (enhanced Mermaid export)Shows only changed functions, their callers, and blast radius — with colored subgraphs for modified files (orange), blast radius callers (purple): flowchart TB
subgraph sg0["src/builder.js **(modified)**"]
n0["buildGraph"]
end
style sg0 fill:#fff3e0,stroke:#ff9800
subgraph sg1["src/cli.js **(modified)**"]
n2["command:export"]
n3["command:prune"]
n4["command:search"]
n5["command:roles"]
end
style sg1 fill:#fff3e0,stroke:#ff9800
subgraph sg2["src/db.js **(modified)**"]
n6["initSchema"]
end
style sg2 fill:#fff3e0,stroke:#ff9800
subgraph sg3["src/embedder.js **(modified)**"]
n7["globMatch"]
n14["disposeModel"]
n15["loadModel"]
n8["_prepareSearch"]
n12["search"]
end
style sg3 fill:#fff3e0,stroke:#ff9800
subgraph sg4["src/export.js **(modified)**"]
n20["mermaidShape"]
n22["exportMermaid"]
n21["nodeId"]
end
style sg4 fill:#fff3e0,stroke:#ff9800
subgraph sg5["src/mcp.js **(modified)**"]
n13["startMCPServer"]
end
style sg5 fill:#fff3e0,stroke:#ff9800
subgraph sg6["src/queries.js **(modified)**"]
n23["listFunctionsData"]
n24["statsData"]
n25["stats"]
n27["contextData"]
n28["context"]
n30["explainFileImpl"]
n35["mapSymbol"]
n33["explainFunctionImpl"]
n36["explain"]
n38["printFunctionExplain"]
n39["whereSymbolImpl"]
n41["where"]
n43["rolesData"]
n44["roles"]
end
style sg6 fill:#fff3e0,stroke:#ff9800
subgraph sg7["src/registry.js **(modified)**"]
n45["pruneRegistry"]
end
style sg7 fill:#fff3e0,stroke:#ff9800
subgraph sg8["src/structure.js **(modified)**"]
n47["median"]
n48["classifyNodeRoles"]
end
style sg8 fill:#fff3e0,stroke:#ff9800
n9["searchData"]
n16["embed"]
n17["buildEmbeddings"]
n31["getFileLines"]
n40["whereData"]
subgraph sg9["Callers **(blast radius)**"]
n1["command:build"]
n10["multiSearchData"]
n11["benchmarkModel"]
n18["command:embed"]
n19["command:mcp"]
n26["command:stats"]
n29["command:context"]
n32["getFileLines"]
n34["explainCallees"]
n37["command:explain"]
n42["command:where"]
n46["command:list"]
n49["resolveBarrelExport"]
end
style sg9 fill:#f3e5f5,stroke:#9c27b0
n0 --> n1
n6 --> n0
n7 --> n8
n8 --> n9
n8 --> n10
n9 --> n11
n9 --> n12
n9 --> n13
n14 --> n11
n14 --> n15
n15 --> n16
n16 --> n17
n16 --> n9
n16 --> n10
n17 --> n11
n17 --> n18
n12 --> n4
n13 --> n19
n20 --> n21
n22 --> n2
n23 --> n13
n24 --> n25
n25 --> n26
n27 --> n13
n27 --> n28
n28 --> n29
n30 --> n31
n31 --> n32
n31 --> n33
n33 --> n34
n33 --> n31
n31 --> n30
n36 --> n37
n39 --> n40
n40 --> n13
n40 --> n41
n41 --> n42
n43 --> n13
n43 --> n44
n44 --> n5
n45 --> n13
n45 --> n46
n45 --> n3
n47 --> n48
n48 --> n49
|
1 participant
Summary
src/cochange.jsmodule with scan, compute, analyze, and query functions; DB migration v5 forco_changes+co_change_metatablescodegraph co-change [file]with--analyze,--since,--min-support,--min-jaccard,--fulloptionsdiff-impactto show historically coupled files not captured by the static dependency graphco_changestool for AI agent accessTest plan
npm test— all 535 tests pass, 0 failuresnpm run lint— clean, no issuescodegraph co-change --analyze— scans repo git history and populates datacodegraph co-change src/queries.js— shows co-change partnerscodegraph co-change— shows top global pairscodegraph diff-impact main -T— verify historicallyCoupled section appears