mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-05-30 19:46:34 +00:00
Code Issues Releases Wiki Activity

Apply continuous self-improvement to skills (#3072)

Browse Source 
- pr-polisher: Relaxed the package-lock.json strictness. If package.json or dependencies.gradle are modified, changes to package-lock.json now correctly trigger a WARNING rather than a fatal ERROR, streamlining intentional dependency updates.
- java-ast-refactoring: Replaced the reliance on a local google-java-format binary with the project's native ./gradlew javaIncrementalFormatApply task for post-AST format fixes.
- Updated GEMINI.md and skill instructions to explicitly authorize and mandate the agent to proactively propose systemic infrastructure fixes to the user when it encounters recurring friction, false positives, or brittle workarounds.
- Overhauled the PR polisher "When to Use" instructions in GEMINI.md and SKILL.md into a critical mandate explicitly tying the execution of the polisher to the action of making or amending a commit to prevent agent forgetfulness.
This commit is contained in:
Ben McIlwain
2026-05-28 18:32:18 -04:00
committed by GitHub
parent beb7fcc16e
commit cd65a4c9d8
5 changed files with 27 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
.gemini/skills/java-ast-refactoring/SKILL.md
View File

@@ -22,9 +22,9 @@ python3 .gemini/skills/java-ast-refactoring/scripts/safe_rename.py <filepath> <o
```bash
./.gemini/skills/java-ast-refactoring/scripts/run_rewrite.sh rewrite.yml
```
3. The script will safely apply the AST transformations and then automatically run `./gradlew spotlessApply` and `google-java-format --replace` on the modified files to automatically fix any Checkstyle line-length and import ordering issues caused by longer/shorter identifiers. Verify the output using `git diff`.
3. The script will safely apply the AST transformations and then automatically run `./gradlew spotlessApply` and `./gradlew javaIncrementalFormatApply` on the modified files to automatically fix any Checkstyle line-length and import ordering issues caused by longer/shorter identifiers. Verify the output using `git diff`.
4. **MANDATORY:** Always run `./gradlew build -x test` (or the equivalent compile task) after running OpenRewrite to ensure no compilation errors were introduced.
## Known Limitations & Troubleshooting
* **Static Imports Dropped on Class Rename:** When using `ChangeType` to rename a class, OpenRewrite may sometimes drop static imports for fields/constants belonging to the old class instead of updating them to the new class. If compilation fails due to "cannot find symbol" for a constant after a class rename, manually restore the static import (e.g., `import static com.new.ClassName.CONSTANT;`).
* **Continuous Improvement:** If any new issues or edge cases are found while running the refactoring (e.g., build failures, formatting issues, or missed transformations), proactively update this skill file (`SKILL.md`) and its accompanying scripts (`scripts/run_rewrite.sh`, `scripts/safe_rename.py`) to permanently fix the issue for future use.
* **Continuous Improvement:** If any new issues or edge cases are found while running the refactoring (e.g., build failures, formatting issues, or missed transformations), you **MUST** proactively ask the user if you should permanently update this skill file (`SKILL.md`) and its accompanying scripts (`scripts/run_rewrite.sh`, `scripts/safe_rename.py`) to fix the issue for future use. Do not wait for the user to prompt you to fix the infrastructure.

9
.gemini/skills/java-ast-refactoring/scripts/run_rewrite.sh
View File

@@ -73,14 +73,7 @@ echo "Executing OpenRewrite recipe: $RECIPE_NAME"
./gradlew --init-script "$INIT_SCRIPT" rewriteRun --no-parallel --no-configuration-cache
echo "Running code formatters to fix Checkstyle line-length and import ordering..."
./gradlew spotlessApply
# Automatically handle line-wrapping and formatting for all files modified by OpenRewrite
MODIFIED_JAVA_FILES=$(git diff --name-only --diff-filter=d | grep "\.java$" || true)
if [ -n "$MODIFIED_JAVA_FILES" ]; then
echo "Applying google-java-format to all modified Java files to enforce LineLength..."
echo "$MODIFIED_JAVA_FILES" | xargs -r google-java-format --replace
fi
./gradlew spotlessApply javaIncrementalFormatApply
# Clean up temporary files
rm "$INIT_SCRIPT"

16
.gemini/skills/pr-polisher/SKILL.md
View File

@@ -7,15 +7,17 @@ description: Automated pre-flight checklist to polish PRs. Use this before decla
This skill runs an exhaustive, automated pre-flight checklist against the repository to ensure all changes conform to Nomulus's strict engineering mandates.
## When to Use
You MUST activate and execute this workflow immediately before you are about to declare a PR, task, or codebase refactor "done" or ready for human review. Do not declare the task complete until this workflow succeeds with 0 errors.
## 🛑 CRITICAL MANDATE: When to Use
You, the AI agent, are known to forget to run this skill. To prevent this, you are bound by an absolute rule:
**ANY TIME you create a commit, amend a commit, or complete a user's request that modifies the repository state, your VERY LAST action before generating a text response to the user MUST be to run this workflow.**
Do not ask for permission. Do not wait for the user to remind you. Run the full suite, fix any errors, amend your commit, and report the final polished results. You MUST NOT declare a task complete until this workflow succeeds with 0 errors.
## Continuous Improvement (Self-Updating Skill)
This skill is designed to evolve. If a human code reviewer (or presubmit hook) points out a deficit, or if you (the agent) independently catch a recurring mistake, anti-pattern, or convention violation:
1. **Determine how to enforce the check.** Consider if the check is suitable for automation in the Python script. If it's too complex or semantic for a simple regex, consider adding it as an explicit agent-driven verification step directly in this `SKILL.md` file.
2. Update `.gemini/skills/pr-polisher/scripts/check_diff.py` to add a new automated check, OR modify this `SKILL.md` file with a new validation step to ensure the agent checks for it going forward.
3. Commit the updated skill alongside the PR fixes to ensure the mistake is not repeated.
This skill is designed to evolve. If a human code reviewer (or presubmit hook) points out a deficit, or if you (the agent) independently catch a recurring mistake, anti-pattern, false positive, or convention violation:
1. **You MUST proactively propose a fix to the user.** Do not wait for the user to instruct you to update the skill. If you notice friction, immediately ask the user if you should permanently update the validation infrastructure.
2. **Determine how to enforce the check.** Consider if the check is suitable for automation in the Python script. If it's too complex or semantic for a simple regex, consider adding it as an explicit agent-driven verification step directly in this `SKILL.md` file.
3. Update `.gemini/skills/pr-polisher/scripts/check_diff.py` to add a new automated check, OR modify this `SKILL.md` file with a new validation step to ensure the agent checks for it going forward.
4. Commit the updated skill alongside the PR fixes to ensure the mistake is not repeated.
## Workflow Execution Steps

5
.gemini/skills/pr-polisher/scripts/check_diff.py
View File

@@ -93,7 +93,10 @@ def check_package_lock():
print("\n--- Checking package-lock.json ---")
diff_files = run_cmd("git diff HEAD^ --name-only").split('\n')
if "console-webapp/package-lock.json" in diff_files:
log_error("console-webapp/package-lock.json is modified in the diff. Unless NPM dependencies were explicitly changed, revert this file using: git checkout console-webapp/package-lock.json")
if "console-webapp/package.json" in diff_files or "dependencies.gradle" in diff_files:
log_warning("console-webapp/package-lock.json is modified in the diff. This is expected since dependencies were updated.")
else:
log_error("console-webapp/package-lock.json is modified in the diff. Unless NPM dependencies were explicitly changed, revert this file using: git checkout console-webapp/package-lock.json")
else:
log_success("console-webapp/package-lock.json is untouched.")

14
GEMINI.md
View File

@@ -73,6 +73,11 @@ Based on historical PR reviews, avoid the following common mistakes:
- **No Unnecessary Casts:** Do not unnecessarily cast objects if the method signature accepts the type directly (e.g., avoid `(Instant) fakeClock.now()` or `(ImmutableSet<String>) bsaQuery(...)` if it compiles without it).
- **Visibility Modifiers:** Do not use `/* package */` comments to denote package-private visibility. Just leave the modifier blank; it is an established idiom in this codebase.
## Continuous Self-Improvement Mandate
You are explicitly authorized and required to improve this project's automated infrastructure.
If during the execution of a task you encounter a recurring error, a false positive in a script, or realize that a shell command/workaround is inferior to a native Gradle task, **you MUST pause and proactively ask the user:** *"I noticed this friction. Per my continuous improvement directives, would you like me to permanently update the skill/script/instructions to fix this for the future?"*
Do not wait for the user to tell you to improve the skills; it is your responsibility to propose these systemic fixes immediately when friction is identified.
### Advanced Java & Guava Idioms
- **Immutable Types:** Declare variables, fields, and return types explicitly as Guava immutable types (e.g., `ImmutableList<T>`, `ImmutableMap<K, V>`) instead of their generic interfaces (`List<T>`, `Map<K, V>`) to clearly communicate immutability contracts to callers. Use `toImmutableList()` and `toImmutableMap()` collectors in streams rather than manually accumulating into an `ArrayList` or `HashMap`.
- **Constructors:** Do not perform heavy logic, I/O, or external API calls inside constructors. Initialization logic should be deferred or handled in a factory method or a dedicated startup routine.
@@ -110,10 +115,13 @@ This document captures high-level architectural patterns, lessons learned from l
5. **Only after** step 4 has successfully returned a clean working directory may you generate a text response to the user declaring that the task is complete.
- **Diff Review:** Before finalizing a task, review the full diff (e.g., `git diff HEAD^`) to ensure all changes are functional and relevant. Identify and revert any formatting-only changes in files that do not contain functional updates to keep the commit focused.
## Self-Review Guidelines & PR Polisher
Before finalizing any PR or declaring a task complete, you MUST automatically execute the `pr-polisher` skill to perform a thorough, rigorous self-review of your entire diff.
## 🛑 CRITICAL: MANDATORY PR POLISHER EXECUTION
You, the AI agent, have a known failure mode of forgetting to run the PR polisher after making changes because you focus too narrowly on the immediate task. To prevent this, you are bound by the following absolute rule:
To run it, activate the skill and follow its exact workflow. You MUST NOT declare a task done until the automated script (`python3 .gemini/skills/pr-polisher/scripts/check_diff.py`) returns 0 errors, the presubmits and tests pass, and the workspace is fully clean.
**ANY TIME you create a commit, amend a commit, or complete a user's request that modifies the repository state, your VERY LAST action before generating a text response to the user MUST be to execute the `pr-polisher` skill.**
Do not ask for permission. Do not wait for the user to remind you. Run the full suite (`python3 .gemini/skills/pr-polisher/scripts/check_diff.py`, `runPresubmits`, `build`, and `test`), fix any errors, amend your commit, and report the final polished results in your response.
You MUST NOT declare a task done until this automated script returns 0 errors, the presubmits and tests pass, and the workspace is fully clean.
1. **Imports & FQNs:** Did I leave any fully-qualified class names or static variables inline? Did I add the necessary imports for them? *Crucial Exception:* If the file already imports a class with the identical name (e.g., it uses both `java.util.Date` and `java.sql.Date`), one MUST remain fully qualified to avoid a compilation conflict.
2. **Diff Scope:** Are there any formatting-only changes in files that I did not functionally modify? If so, revert them. Does the total line count of the diff align with the approved scope (e.g., ~1,000 lines for migrations)?