Issue #109 Auto-create issues for API drift with fingerprint deduplication#113
Issue #109 Auto-create issues for API drift with fingerprint deduplication#113
Conversation
…reation - Add generateFingerprint() to create SHA256-based 7-char hash of API diffs - Add generateSummary() to create markdown summary for issue body - Add hasDifferences() helper method - Update ApiTrackerRunner to write outputs to target/api-tracker/ - Update Java 25 workflow to: - Upload report artifacts - Check for existing issues with matching fingerprint - Create new issue only if no match found
Summary of ChangesHello @simbo1905, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the API tracking system by automating the creation of GitHub issues for detected API drifts. It introduces a robust deduplication strategy using unique fingerprints to ensure that only new and distinct API changes trigger issue creation, thereby streamlining the monitoring process and reducing alert fatigue. Additionally, the changes enhance the output capabilities of the tracker and update the underlying Java environment. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively adds fingerprinting and summary generation for API drift detection, which is a great enhancement for automating issue creation. The code is generally well-written and clear. I've provided a few suggestions to improve robustness, consistency, and reduce code duplication. The most critical feedback is regarding error handling when writing output files, which should be stricter to prevent silent failures in your CI workflow. Other comments focus on making the generated summary deterministic and improving code consistency.
| } catch (IOException e) { | ||
| System.err.println("Warning: Could not write output files: " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Catching IOException and only printing a warning to stderr can mask a critical failure. If the workflow relies on these output files, the build should fail explicitly. It's better to treat this as a fatal error, print an error message, and exit with a non-zero status code, similar to how other exceptions are handled in the main method.
| } catch (IOException e) { | |
| System.err.println("Warning: Could not write output files: " + e.getMessage()); | |
| } | |
| } catch (IOException e) { | |
| System.err.println("Error: Could not write output files: " + e.getMessage()); | |
| //noinspection CallToPrintStackTrace | |
| e.printStackTrace(); | |
| System.exit(1); | |
| } |
There was a problem hiding this comment.
Fixed - now exits with non-zero status code on IOException.
| return hexString.substring(0, 7); | ||
| } catch (NoSuchAlgorithmException e) { | ||
| LOGGER.warning("SHA-256 not available, using fallback fingerprint"); | ||
| return String.format("%07d", stableString.hashCode() & 0xFFFFFFF); |
There was a problem hiding this comment.
The fallback fingerprint is generated as a decimal string using String.format("%07d", ...), while the primary SHA-256 fingerprint is a hexadecimal string. For consistency, it's better if the fallback also produces a hexadecimal string. This avoids potential issues in consumers of the fingerprint that might expect a hex value.
| return String.format("%07d", stableString.hashCode() & 0xFFFFFFF); | |
| return String.format("%07x", stableString.hashCode() & 0xFFFFFFF); |
There was a problem hiding this comment.
Fixed - fallback now uses hex format: %07x
| } | ||
|
|
||
| sb.append("---\n"); | ||
| sb.append("*Generated by API Tracker on ").append(Instant.now().toString().split("T")[0]).append("*\n"); |
There was a problem hiding this comment.
The summary generation uses Instant.now() for its timestamp. This makes the generated summary non-deterministic if called at different times for the same report. The report object already contains a timestamp field which should be used instead to ensure the summary output is deterministic and accurately reflects the time of the report generation.
| sb.append("*Generated by API Tracker on ").append(Instant.now().toString().split("T")[0]).append("*\n"); | |
| final var timestamp = ((JsonString) report.members().get("timestamp")).value(); | |
| sb.append("*Generated by API Tracker on ").append(timestamp.split("T")[0]).append("*\n"); |
There was a problem hiding this comment.
Fixed - now uses report timestamp instead of Instant.now()
| static boolean hasDifferences(JsonObject report) { | ||
| final var summary = (JsonObject) report.members().get("summary"); | ||
| final var differentApi = ((JsonNumber) summary.members().get("differentApi")).toNumber().longValue(); | ||
| return differentApi > 0; | ||
| } |
There was a problem hiding this comment.
The logic to extract the differentApi count from the report summary is duplicated across generateFingerprint, generateSummary, and this method (hasDifferences). This duplication can lead to maintenance issues. Consider refactoring this logic into a private static helper method to improve code reuse and robustness.
For example:
private static long getDifferentApiCount(JsonObject report) {
final var summary = (JsonObject) report.members().get("summary");
if (summary == null) {
return 0;
}
final var differentApiValue = summary.members().get("differentApi");
if (differentApiValue instanceof JsonNumber num) {
return num.toNumber().longValue();
}
return 0;
}This method could then be simplified to return getDifferentApiCount(report) > 0;, and the other methods could use getDifferentApiCount as well.
There was a problem hiding this comment.
Fixed - extracted getDifferentApiCount() helper method to reduce duplication
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 206ad18c81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Read outputs into environment | ||
| echo "fingerprint=$(cat target/api-tracker/fingerprint.txt)" >> $GITHUB_OUTPUT | ||
| echo "has_differences=$(cat target/api-tracker/has-differences.txt)" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Read tracker outputs from module target dir
In this workflow step the outputs are read from target/api-tracker/..., but ApiTrackerRunner.writeOutputFiles writes to target/api-tracker relative to the JVM working directory, which under mvn exec:java -pl json-java21-api-tracker is the module base. That means the files land in json-java21-api-tracker/target/api-tracker, so cat target/api-tracker/*.txt (and the artifact upload path) will be missing in CI and the step will fail before issue creation. Consider using json-java21-api-tracker/target/api-tracker or setting a working-directory so the paths align.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This was tested and works correctly. Maven exec:java runs from the project root, not the module directory. The logs from CI show: 'Output files written to: /home/runner/work/java.util.json.Java21/java.util.json.Java21/target/api-tracker' which is the project root's target/api-tracker. Issue #114 was created successfully using these paths.
| final var changeObj = (JsonObject) change; | ||
| final var type = ((JsonString) changeObj.members().get("type")).value(); | ||
| final var methodValue = changeObj.members().get("method"); | ||
| final var method = methodValue instanceof JsonString js ? js.value() : ""; | ||
| // Create stable line: "ClassName:changeType:methodName" | ||
| stableLines.add(className + ":" + type + ":" + method); |
There was a problem hiding this comment.
Include non-method diff details in fingerprint
The fingerprint line only uses className:type:method, but for diff types like fieldsChanged, constructorsChanged, inheritanceChanged, attributeChanged, and modifiersChanged there is no method and the specific field/attribute lists or counts are ignored. That means a class can change again within the same diff type (e.g., different field set or different attribute mismatch) and still hash to the same fingerprint, so the workflow will treat a new drift as a duplicate and skip opening a new issue. Including the relevant detail (attribute name, field list, counts, etc.) in the fingerprint avoids missing updated drift.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged - for now the fingerprint covers className:changeType:methodName which handles the common case of method changes. For non-method changes (fieldsChanged, inheritanceChanged, etc.) the type is still included which will differentiate them. Adding full detail extraction would add complexity - deferring to a follow-up if needed.
- Exit non-zero on IOException when writing output files - Use hex format for fallback fingerprint (%07x) - Use report timestamp instead of Instant.now() in summary - Extract getDifferentApiCount() helper to reduce duplication - Use helper in generateFingerprint(), generateSummary(), hasDifferences()
Summary
Enhances the Daily API Tracker workflow to automatically create GitHub issues when upstream API drift is detected, with intelligent deduplication to prevent issue spam.
Changes
API Tracker Code (
ApiTracker.java,ApiTrackerRunner.java)generateFingerprint()- creates SHA256-based 7-char hash from sorted list of changesgenerateSummary()- creates markdown summary with emojis for issue bodyhasDifferences()- helper to check if any API differences existtarget/api-tracker/(report.json, fingerprint.txt, summary.md, has-differences.txt)Workflows
hash:XXXXXXXin issue titles)Local Development
mise.tomlfor Java 25 setup via miseTesting
Verified on the Java 25 test workflow:
Closes #109