research(bench): add javac compilation step to Java fixture for javacg-static comparison#1329
research(bench): add javac compilation step to Java fixture for javacg-static comparison#1329carlos-alm wants to merge 1 commit into
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review: research(bench): add javac compilation step + javacg-static comparison
Root Cause AnalysisProblem Being Solved: The existing Java benchmark ( Why This Approach: javacg-static requires compiled bytecode ( Risk Assessment: Low. No production code changed. No tests weakened. The new script is a standalone research/dev tool, not part of the CI pipeline. Phase 2: Configuration ChangesNone. No linting rules, TypeScript flags, test timeouts, or build config were modified. Phase 3: Test IntegrityNo test assertions weakened. No test files modified. The only doc update is additive (filling out the Java section that previously said "run Phase 4: Code QualityOverall the script is well-structured and follows the pattern of
Correctly excludes
The placeholder javacg-static row ( Phase 5: Backlog ComplianceZero-dep ✓ — No new entries in Foundation-aligned ✓ — This is a research/benchmark script in Problem-fit: 2/5 — This is calibration/research tooling for understanding codegraph's Java recall gaps. It doesn't directly reduce token waste for AI agents or prevent hallucinated deps. It feeds the diagnostic work that will eventually improve codegraph (e.g., fixing the Breaking: No — Purely additive. No API, DB schema, CLI output, or MCP contract changes. Tier: 1 — Zero new deps, fully foundation-aligned. Minor Suggestions (non-blocking)
Final Assessment |
Greptile SummaryThis PR adds a
Confidence Score: 5/5Safe to merge — this is a self-contained benchmark tooling addition with no changes to production code paths. All changed files are benchmark fixtures, a new standalone comparison script, and documentation. The core parsing and metrics logic in compare-javacg.mjs correctly maps javacg-static output to the expected-edges.json convention, and precision/recall computation is correct. No production code is touched. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Start: node compare-javacg.mjs] --> B{Find javacg JAR}
B -->|--jar arg| C[Use provided path]
B -->|JAVACG_JAR env| D[Use env var path]
B -->|scripts/lib/*.jar| E[Use auto-discovered jar]
B -->|none found| F[Exit: JAR not found]
C --> G[buildClassFileMap\nScan .java files]
D --> G
E --> G
G --> H[loadGroundTruth\nParse expected-edges.json]
H --> I{fixture.jar exists?}
I -->|No| J[Exit: run make first]
I -->|Yes| K[runJavacg\njava -jar javacgJar fixtureJar]
K --> L[parseJavacgOutput\nFilter JDK edges, map names]
L --> M[computeMetrics\nTP / FP / FN]
M --> N{--json flag?}
N -->|Yes| O[stdout: JSON]
N -->|No| P[stdout: Markdown table]
Reviews (3): Last reviewed commit: "chore(bench): add javac fixture compilat..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function buildClassFileMap(fixtureDir) { | ||
| const map = new Map(); | ||
| for (const filename of fs.readdirSync(fixtureDir)) { | ||
| if (!filename.endsWith('.java')) continue; | ||
| const src = fs.readFileSync(path.join(fixtureDir, filename), 'utf8'); | ||
| const m = src.match(/(?:public\s+)?(?:abstract\s+)?(?:class|interface)\s+(\w+)/); | ||
| if (m) map.set(m[1], filename); | ||
| } | ||
| return map; |
There was a problem hiding this comment.
buildClassFileMap silently drops classes with unhandled modifiers
The regex only handles optional public and abstract prefixes before class/interface. Any Java file using final class, sealed class, non-sealed class, record, or enum won't match, so the class name won't be added to classFileMap. All edges involving such a class are then silently filtered out in parseJavacgOutput (the !sourceFile || !targetFile guard), which deflates the predicted-edge count and inflates measured precision without any warning. A simple validation — asserting that classFileMap contains one entry per .java file before running the comparison — would catch this class of silent failure.
There was a problem hiding this comment.
Fixed — broadened the regex in buildClassFileMap to handle all Java modifier combinations (final, sealed, non-sealed, protected, private, strictfp) as well as additional type keywords (enum, record). Also added a validation that warns (with a clear message) if any .java file doesn't yield a class name, and a final assertion that map.size === javaFiles.length so skewed precision/recall is surfaced immediately.
| // M: caller (T) callee — the space between caller and (T) may vary | ||
| const lineRe = /^M:(\S+)\s+\(([CSOIDE])\)(\S+)$/; |
There was a problem hiding this comment.
[CSOIDE] also includes E, which isn't documented anywhere. javacg-static doesn't emit an E type, so it's harmless, but a stale character in the regex is confusing. Adding an inline legend makes it easy to verify the regex is exhaustive.
| // M: caller (T) callee — the space between caller and (T) may vary | |
| const lineRe = /^M:(\S+)\s+\(([CSOIDE])\)(\S+)$/; | |
| // M: caller (T) callee — the space between caller and (T) may vary | |
| // T values: C=virtual, S=static, O=special, I=interface, D=dynamic | |
| const lineRe = /^M:(\S+)\s+\(([CSOIDE])\)(\S+)$/; |
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 — removed the undocumented E from the character class and added an inline legend for all call-type codes: // T values: C=virtual, S=static, O=special (constructors/super), I=interface, D=dynamic (invokedynamic). The regex is now [CSOID] which exactly matches what javacg-static documents and emits.
Codegraph Impact Analysis9 functions changed → 2 callers affected across 1 files
|
carlos-alm
force-pushed
the
research/bench-javacg-static-1307
branch
from
7e3a5ac to
b12b579
Compare
|
Addressed the feedback from this review:
|
|
Superseded by #1332 — branch renamed from |
|
Superseded by #1332 — branch renamed to |
1 participant
Summary
tests/benchmarks/resolution/fixtures/java/Makefile— compiles all.javasources tofixture.jarviajavac -d . *.java && jar cf fixture.jar benchmark/scripts/compare-javacg.mjs— runs javacg-static on the compiled JAR, parses itsM:pkg.Class:method (T)pkg.Class:methodoutput, maps toClassName.methodform, and computes precision/recall vsexpected-edges.jsondocs/benchmarks/RESOLUTION-COMPARISON.mdwith the javacg-static methodology, a placeholder comparison table, and reproduction instructionsReproducing
Name mapping
javacg-static uses
pkg.ClassName:method(JVM-descriptors)form. The script maps:<init>→ClassName.ClassName(constructor-as-method, matching expected-edges.json)<init>→ClassName(constructor target = class name only)HashMap,String,System.out, …) are filtered out — only edges between fixture classes countCloses #1307