Impurity annotation #83
Conversation
…g into impurity-annotation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
subject-programs/get-upstream-source.py (3)
29-36: Validate required keys in each YAML project entry
project["source"]andproject["dir"]are accessed without checking presence, so malformedbuild-info.yamlwill yield an unhelpfulKeyError. Consider validating required keys up front and raising a descriptiveValueError(or similar) that includes the offending project entry.
56-63: Improve non-HTTP command execution robustness (split, output capture, error message)Using
source.split()breaks for quoted/escaped arguments, andsubprocess.runis invoked without capturing stdout/stderr, so the printed values will usually beNone. The raisedExceptionalso passes multiple arguments, producing a tuple message.Within this block, consider:
- else: - command = source.split() - print("command = ", command) - completed_process = subprocess.run(command, cwd=src_upstream_dir) - if completed_process.returncode != 0: - print("stdout", completed_process.stdout) - print("stderr", completed_process.stderr) - raise Exception("command failed: ", command) + else: + command = shlex.split(source) + print("command =", command) + completed_process = subprocess.run( + command, cwd=src_upstream_dir, capture_output=True, text=True + ) + if completed_process.returncode != 0: + print("stdout:", completed_process.stdout) + print("stderr:", completed_process.stderr) + raise Exception(f"Command failed: {' '.join(command)}")And add at the top of the file:
import shlexoutside this hunk.
64-75: Apply the same command parsing and output-capture fixes to post-extract commandsThe post-extract commands use the same naive
split()and non-capturingsubprocess.run, and again raise anExceptionwith multiple arguments, yielding a tuple message.Within this block, consider:
- key = "post-extract-command" - if key in project: - commands = project[key].split(" && ") - for command_unsplit in commands: - command = command_unsplit.split() - print("dir =", src_upstream_dir) - print("command =", command) - completed_process = subprocess.run(command, cwd=src_upstream_dir) - if completed_process.returncode != 0: - print("stdout", completed_process.stdout) - print("stderr", completed_process.stderr) - raise Exception("command failed: ", command) + key = "post-extract-command" + if key in project: + commands = project[key].split(" && ") + for command_unsplit in commands: + command = shlex.split(command_unsplit) + print("dir =", src_upstream_dir) + print("command =", command) + completed_process = subprocess.run( + command, cwd=src_upstream_dir, capture_output=True, text=True + ) + if completed_process.returncode != 0: + print("stdout:", completed_process.stdout) + print("stderr:", completed_process.stderr) + raise Exception(f"Command failed: {' '.join(command)}")This keeps behavior similar while making error handling and argument parsing much more robust.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
subject-programs/get-upstream-source.py(1 hunks)
🔇 Additional comments (1)
subject-programs/get-upstream-source.py (1)
1-22: Module setup and YAML loading look goodDocstring, path handling via
script_directory, andSafeLoader-based YAML loading are straightforward and appropriate for this script.
📝 WalkthroughWalkthroughThe pull request reorganizes subject-program management and build automation: adds build-info.yaml, buildall.py, and buildall.sh; introduces scripts to fetch GRT and upstream sources; centralizes JARs under a jars directory and updates scripts to use them; adds checker-framework support and a checker-qual artifact in build/make targets; updates CI to install Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-14T03:30:11.765ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/make.yml (1)
13-18:.venvactivation will fail because no virtualenv is created in this workflow.The
style checkstep unconditionally runssource .venv/bin/activate, but there is no prior step in this job that creates.venv. On a fresh GitHub runner this will cause the step to fail beforemake style-checkruns.Either add a step that creates/populates
.venv, or guard the activation so CI still works when.venvis absent. For example:- - name: style check - run: | - source .venv/bin/activate - make style-check + - name: style check + run: | + if [ -f .venv/bin/activate ]; then + . .venv/bin/activate + fi + make style-checkAlso applies to: 24-26
♻️ Duplicate comments (6)
subject-programs/annotated-jars/README.md (1)
3-7: Document how to build Checker Framework locally and clarify classpath usage.The narrative is good, but a couple of gaps will trip users up:
Checker Framework prerequisite still undocumented (re-raising earlier feedback).
Step 2 assumes a local Checker Framework atscripts/build/checker-framework, but the README never tells users how to create it. Consider adding a short prerequisites block just before “### Setup”, for example:## Rebuilding an annotated JAR **Prerequisites:** Build the local Checker Framework distribution once: ```sh cd scripts make build/checker-framework/checker/dist/checker-qual.jarThen the existing `CHECKERFRAMEWORK=/path/to/grt-testing/scripts/build/checker-framework` export will make sense.Classpath instructions are incomplete.
You explain how to generatewpi-classpath.txt, but the inference command uses$RUNTIME_CLASSPATHwithout showing how to set it. One concrete option:RUNTIME_CLASSPATH=$(tr '\n' ':' < wpi-classpath.txt)and then use
$RUNTIME_CLASSPATHin theinfer-and-annotate.shinvocation.Minor punctuation tweak.
On the introductory paragraph, add a period after the package name:-
org.checkerframework.framework.qualCompared tosubject-programs/jars/, the
+org.checkerframework.framework.qual. Compared tosubject-programs/jars/, theAlso applies to: 17-36, 39-58 </blockquote></details> <details> <summary>subject-programs/README.build (1)</summary><blockquote> `15-17`: **Nice Maven portability fix; a few lingering doc nits remain (jaxax/mail, Shiro version, SSH URLs).** - The change to ```sh mvn -B clean package -Dmaven.test.failure.ignore -Dmaven.javadoc.skip 2>&1 | tee mvn-output.txtis a good improvement over
|&for/bin/shportability.
In the “Test directory” table, the mail artifact name still has the previously flagged typo:
jaxax.mail-1.5.1 (uses mvn)
- javax.mail-1.5.1 (uses mvn)
In the “software (version)” table, the Shiro line still shows a mismatched version:
Apache Shiro-core (1.2.6) https://archive.apache.org/dist/shiro/1.2.3/shiro-root-1.2.3-source-release.zip
- Apache Shiro-core (1.2.3) https://archive.apache.org/dist/shiro/1.2.3/shiro-root-1.2.3-source-release.zip
- Optional: the Jcommander and Pmd DCD rows still use
git@github.com:SSH URLs; switching to HTTPS would make the instructions friendlier to users without SSH keys.Also applies to: 44-56, 61-90
subject-programs/buildall.py (1)
1-3:buildall.pyis still effectively a no-op placeholder.Adding the
"TODO."docstring fixes the lint error, but this script still has no real implementation or entry point. To avoid confusion:
- Either delete
subject-programs/buildall.pyuntil you are ready to wire in a Python-based build driver, or- Flesh it out with at least a minimal stub (e.g., a
main()that prints a “not yet implemented” message and a short module docstring describing the intended purpose).subject-programs/build-info.yaml (1)
223-231: Fix invalidpost-extract-commandfornekomud-r16.The
post-extract-commandhere:post-extract-command: rm -rf $(findfile .svn ClassViewer-5.0.5b)is both syntactically invalid for a shell (
$(findfile ...)is Make syntax) and appears to reference the wrong project directory (ClassViewer-5.0.5binstead ofnekomud-r16). As a result, it won’t actually clean.svndirectories as intended.A safer, project-scoped replacement:
-post-extract-command: rm -rf $(findfile .svn ClassViewer-5.0.5b) +post-extract-command: find nekomud-r16 -name .svn -type d -exec rm -rf {} +This matches the project dir and uses pure shell utilities, so it will work under
subprocess.run(...).subject-programs/get-grt-source.sh (1)
15-26: YAML-driven directory discovery remains brittle; consider a parser or validation.The
grep/sedloop overbuild-info.yamlassumes very specific formatting and will silently break if the YAML structure changes (indentation, comments, key order).If feasible, prefer a real YAML parser (e.g.,
yq) or, minimally, add a guard that fails if nodir:entries are found so problems don’t go unnoticed.Also applies to: 28-31
subject-programs/get-upstream-source.py (1)
30-41: Fix syntax error andPath.namemisuse in HTTP download handling.Two issues will currently prevent this script from working:
Invalid use of
Path.name(line 39):basename = Path.name(source)
Path.nameis a property, not a callable; this will raise at runtime. It should construct aPathinstance:
basename = Path.name(source)
basename = Path(source).name
Duplicate
else:(lines 55–56):elif source.endswith(".tar.gz"): ... else: else: raise Exception(f"Unknown archive type: {source}")The extra
else:makes the file invalid Python. Remove the redundant branch:
elif source.endswith(".tar.gz"):
elif source.endswith(".tar.gz"): with tarfile.open(archive_path, "r:gz") as tar: tar.extractall(path=dest_dir)
else:else:raise Exception(f"Unknown archive type: {source}")
else:raise Exception(f"Unknown archive type: {source}")Additionally, for the
post-extract-commandblock (lines 67–77), consider matching your earlier pattern and capturing output for better diagnostics:- completed_process = subprocess.run(command, cwd=src_upstream_dir) + completed_process = subprocess.run( + command, cwd=src_upstream_dir, capture_output=True, text=True + ) if completed_process.returncode != 0: - print("stdout", completed_process.stdout) - print("stderr", completed_process.stderr) - raise Exception("command failed: ", command) + print("stdout", completed_process.stdout) + print("stderr", completed_process.stderr) + raise Exception(f"Command failed: {' '.join(command)}")The first two fixes are blocking; the subprocess improvement will make failures much easier to debug.
Also applies to: 46-58, 67-79
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (61)
subject-programs/annotated-jars/ClassViewer-5.0.5b.jaris excluded by!**/*.jarsubject-programs/annotated-jars/JSAP-2.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/a4j-1.0b.jaris excluded by!**/*.jarsubject-programs/annotated-jars/asm-5.0.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/bcel-5.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-cli-1.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-codec-1.9.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-collections4-4.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-compress-1.8.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-lang3-3.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-math3-3.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-primitives-1.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/dcParseArgs-10.2008.jaris excluded by!**/*.jarsubject-programs/annotated-jars/easymock-3.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/fixsuite-r48.jaris excluded by!**/*.jarsubject-programs/annotated-jars/guava-16.0.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/hamcrest-core-1.3.jaris excluded by!**/*.jarsubject-programs/annotated-jars/javassist-3.19.jaris excluded by!**/*.jarsubject-programs/annotated-jars/javax.mail-1.5.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jaxen-1.1.6.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jcommander-1.35.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jdom-1.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/joda-time-2.3.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jvc-1.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/nekomud-r16.jaris excluded by!**/*.jarsubject-programs/annotated-jars/pmd-core-5.2.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/sat4j-core-2.3.5.jaris excluded by!**/*.jarsubject-programs/annotated-jars/shiro-core-1.2.3.jaris excluded by!**/*.jarsubject-programs/annotated-jars/slf4j-api-1.7.12.jaris excluded by!**/*.jarsubject-programs/annotated-jars/tiny-sql-2.26.jaris excluded by!**/*.jarsubject-programs/jars/ClassViewer-5.0.5b.jaris excluded by!**/*.jarsubject-programs/jars/JSAP-2.1.jaris excluded by!**/*.jarsubject-programs/jars/a4j-1.0b.jaris excluded by!**/*.jarsubject-programs/jars/asm-5.0.1.jaris excluded by!**/*.jarsubject-programs/jars/bcel-5.2.jaris excluded by!**/*.jarsubject-programs/jars/commons-cli-1.2.jaris excluded by!**/*.jarsubject-programs/jars/commons-codec-1.9.jaris excluded by!**/*.jarsubject-programs/jars/commons-collections4-4.0.jaris excluded by!**/*.jarsubject-programs/jars/commons-compress-1.8.jaris excluded by!**/*.jarsubject-programs/jars/commons-lang3-3.0.jaris excluded by!**/*.jarsubject-programs/jars/commons-math3-3.2.jaris excluded by!**/*.jarsubject-programs/jars/commons-primitives-1.0.jaris excluded by!**/*.jarsubject-programs/jars/dcParseArgs-10.2008.jaris excluded by!**/*.jarsubject-programs/jars/easymock-3.2.jaris excluded by!**/*.jarsubject-programs/jars/fixsuite-r48.jaris excluded by!**/*.jarsubject-programs/jars/guava-16.0.1.jaris excluded by!**/*.jarsubject-programs/jars/hamcrest-core-1.3.jaris excluded by!**/*.jarsubject-programs/jars/javassist-3.19.jaris excluded by!**/*.jarsubject-programs/jars/javax.mail-1.5.1.jaris excluded by!**/*.jarsubject-programs/jars/jaxen-1.1.6.jaris excluded by!**/*.jarsubject-programs/jars/jcommander-1.35.jaris excluded by!**/*.jarsubject-programs/jars/jdom-1.0.jaris excluded by!**/*.jarsubject-programs/jars/joda-time-2.3.jaris excluded by!**/*.jarsubject-programs/jars/jvc-1.1.jaris excluded by!**/*.jarsubject-programs/jars/nekomud-r16.jaris excluded by!**/*.jarsubject-programs/jars/pmd-core-5.2.2.jaris excluded by!**/*.jarsubject-programs/jars/sat4j-core-2.3.5.jaris excluded by!**/*.jarsubject-programs/jars/shiro-core-1.2.3.jaris excluded by!**/*.jarsubject-programs/jars/slf4j-api-1.7.12.jaris excluded by!**/*.jarsubject-programs/jars/tiny-sql-2.26.jaris excluded by!**/*.jaruv.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.github/workflows/make.yml(2 hunks).gitignore(2 hunks)Makefile(1 hunks)pyproject.toml(1 hunks)scripts/Makefile(2 hunks)scripts/get-all-subject-src.sh(0 hunks)scripts/mutation-evosuite.sh(2 hunks)scripts/mutation-randoop.sh(5 hunks)subject-programs/README(1 hunks)subject-programs/README.build(2 hunks)subject-programs/annotated-jars/README.md(1 hunks)subject-programs/build-info.yaml(1 hunks)subject-programs/buildall.py(1 hunks)subject-programs/buildall.sh(1 hunks)subject-programs/get-grt-source.sh(1 hunks)subject-programs/get-upstream-source.py(1 hunks)
💤 Files with no reviewable changes (1)
- scripts/get-all-subject-src.sh
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-13T23:36:38.701Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defect-randoop.sh:236-243
Timestamp: 2025-10-13T23:36:38.701Z
Learning: In Defects4J, the command `defects4j export -p cp.compile` automatically compiles the project before returning the classpath, so an explicit `defects4j compile` step is not needed when using `export -p cp.compile`.
Applied to files:
subject-programs/README.buildscripts/Makefile
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
scripts/Makefilescripts/mutation-randoop.sh.gitignorescripts/mutation-evosuite.sh
🧬 Code graph analysis (2)
scripts/mutation-randoop.sh (1)
scripts/defs.sh (1)
require_file(27-32)
subject-programs/buildall.sh (1)
scripts/defs.sh (1)
usejdk8(53-61)
🔇 Additional comments (6)
pyproject.toml (1)
1-11: pyproject configuration looks consistent with the new tooling.The build-system and project sections are well-formed and align with the added Python tooling; no changes needed here.
scripts/Makefile (1)
3-8: Checker Framework and hamcrest wiring inscripts/Makefilelooks sound.
- Adding
build/checker-framework/checker/dist/checker-qual.jartoallaligns with the new annotated-jar workflow and the docs that point atscripts/build/checker-framework.- The new rule correctly downloads, unzips, and normalizes the Checker Framework directory under
build/checker-framework.- Updating the hamcrest copy to
../subject-programs/jars/hamcrest-core-1.3.jarmatches the central jars layout introduced elsewhere in this PR.No changes needed; if you later add more Checker Framework artifacts, you might consider hoisting
CFVER=3.50.0to a top-level Make variable for reuse.Also applies to: 66-69, 75-81
scripts/mutation-evosuite.sh (1)
175-176: Centralized jar paths look consistent.Using
../subject-programs/jars/$SUBJECT_PROGRAM.jarand the sharedjaxen-1.1.6.jaraligns this script with the new centralized jars layout and preserves existing failure behavior underset -e.Also applies to: 321-322
.gitignore (1)
19-25: Ignore patterns align with new source/layout tooling.The new subject-programs
src-*entries,files.diff,.plume-scripts, and the jacoco comment fix all look correct and consistent with the restructured source management.Also applies to: 34-37
scripts/mutation-randoop.sh (1)
59-65: Impurity checker-qual integration and GRT_FUZZING jar selection look correct.
CHECKER_QUAL_JARis resolved, validated, and added to the Randoop classpath consistently with other jars.- The
GRT_FUZZINGcheck now correctly looks at requested features and only selects the annotated jar when enabled, with a clear error if the annotated jar is missing.- The
jdom-1.0dependency now uses the centralizedsubject-programs/jars/jaxen-1.1.6.jar, matching the new layout.These changes are coherent with the rest of the tooling and shouldn’t alter behavior outside the impurity/annotation scenario.
Also applies to: 68-73, 223-234, 367-368, 470-471
subject-programs/buildall.sh (1)
5-9: Fix directory typos and undefinedusejdk8to avoid runtime failures.A few issues will cause this script to fail or skip builds:
- Line 71:
jaxax.mail-1.5.1is almost certainly a typo; everything else usesjavax.mail-1.5.1.- Line 98:
sat4j.core-2.3.5likely should besat4j-core-2.3.5to match other tooling/config.- Line 107:
usejdk8is called but never defined or sourced in this script; underset -ethis will abort the run.- Line 16:
get-all-subject-src.shmay be stale if the repo has fully migrated toget-upstream-source.py/get-grt-source.sh; please confirm the intended entrypoint.Suggested fix for the clearly broken pieces:
-(cd jaxax.mail-1.5.1 \ +(cd javax.mail-1.5.1 \ && mvn -B clean package -Dmaven.test.failure.ignore -Dmaven.javadoc.skip 2>&1 | tee mvn-output.txt) @@ -(cd sat4j.core-2.3.5 \ +(cd sat4j-core-2.3.5 \ && mvn -B clean package -Dmaven.test.failure.ignore -Dmaven.javadoc.skip 2>&1 | tee mvn-output.txt) @@ -usejdk8 +# If you want to reuse the shared Java 8 helper, source defs.sh first: +# . "${SCRIPT_DIR}"/../scripts/defs.sh +# usejdk8Adjust the
get-all-subject-src.shcall as needed depending on whether that helper is still part of the new workflow.
Makefile
Outdated
| .plume-scripts: | ||
| git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
.plume-scripts target is fine; consider a shallow clone.
The new target cleanly fetches plume-scripts into a hidden .plume-scripts directory and will be effectively idempotent once the directory exists.
Optionally, you could make the clone lighter-weight:
-.plume-scripts:
- git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts
+.plume-scripts:
+ git clone -q --depth 1 https://github.com/plume-lib/plume-scripts.git .plume-scripts📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .plume-scripts: | |
| git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts | |
| .plume-scripts: | |
| git clone -q --depth 1 https://github.com/plume-lib/plume-scripts.git .plume-scripts |
🤖 Prompt for AI Agents
In Makefile around lines 11 to 13, the .plume-scripts target currently does a
full git clone; change the clone to be shallow to reduce fetch size by adding
git clone --depth 1 --single-branch -q
https://github.com/plume-lib/plume-scripts.git .plume-scripts (optionally also
pin a branch with --branch <name>), so update the Makefile line accordingly.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
subject-programs/get-upstream-source.py (4)
29-31: Add validation for required YAML fields.The code assumes
project["source"]andproject["dir"]exist. Malformed YAML will raise an unhelpfulKeyError.Add validation at the start of the loop:
for project in yaml_data: + required_keys = ["source", "dir"] + missing = [k for k in required_keys if k not in project] + if missing: + raise ValueError(f"Project missing required keys: {missing}. Project: {project}") + source = project["source"] proj_dir = project["dir"]
37-55: Fix AttributeError:sourceis a string, not a Path.Line 38 attempts to call
.nameon a string, which will raiseAttributeErrorat runtime for any HTTP source.Apply this diff to fix:
if source.startswith("http"): - basename = source.name + basename = pathlib.Path(source).name archive_path = src_upstream_dir / basename
67-78: Capture subprocess output and fix exception formatting.Line 74 doesn't capture output, so lines 76-77 will print
None. Line 78 raises an exception with multiple arguments, creating a tuple instead of a clear message.Apply this diff:
for command_unsplit in commands: command = shlex.split(command_unsplit) print("dir =", src_upstream_dir) print("command =", command) - completed_process = subprocess.run(command, cwd=src_upstream_dir) + completed_process = subprocess.run( + command, cwd=src_upstream_dir, capture_output=True, text=True + ) if completed_process.returncode != 0: print("stdout", completed_process.stdout) print("stderr", completed_process.stderr) - raise Exception("command failed: ", command) + msg = f"Command failed: {' '.join(command)}" + raise Exception(msg)
79-80: Ensure destination directory exists and remove redundant wrapper.The code should ensure
project_direxists before renaming. Also,archive_pathis already a Path object, so the wrapper is redundant.Apply this diff:
if source.startswith("http"): - pathlib.Path(archive_path).rename(src_upstream_dir / proj_dir / basename) + project_dir.mkdir(parents=True, exist_ok=True) + archive_path.rename(project_dir / basename)Makefile (1)
11-12: Consider using shallow clone for .plume-scripts.Following the suggestion from the previous review, add
--depth 1to reduce the git clone size. This makes the target more efficient without loss of functionality..plume-scripts: - git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts + git clone -q --depth 1 https://github.com/plume-lib/plume-scripts.git .plume-scriptspyproject.toml (1)
1-4: Remove blank line before [project] section.Per the previous review, Ruff's W391 flag indicates a trailing blank line. Line 4 contains an unnecessary blank line between the [build-system] and [project] sections that should be removed.
[build-system] requires = ["uv_build >= 0.9.11, <0.10.0"] build-backend = "uv_build" - [project]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/make.yml(1 hunks)Makefile(2 hunks)pyproject.toml(1 hunks)scripts/experiment-scripts/generate-grt-figures.py(8 hunks)subject-programs/get-upstream-source.py(1 hunks)
🔇 Additional comments (13)
subject-programs/get-upstream-source.py (1)
56-66: LGTM!The command execution logic correctly uses
shlex.split()for shell-safe parsing, captures subprocess output, and provides clear error reporting.scripts/experiment-scripts/generate-grt-figures.py (6)
31-31: Added matplotlib.figure import.The explicit import of
matplotlib.figuresupports the type annotationlist[mpl.figure.Figure]on line 217. This is consistent with the function's return type and improves type clarity.
79-81: Reformatted groupby().agg() for readability.The multi-line dictionary formatting improves code readability without altering the aggregation logic or results.
191-191: Using tuple for rect parameter is a minor style improvement.Tuple (0, 0, 1, 0.95) is functionally equivalent to the list form and is a minor stylistic preference that makes the fixed coordinates more explicitly immutable.
231-231: Chained pandas aggregation is valid and concise.The
.mean().reset_index()chain on the grouped selection is a standard pandas pattern and preserves the original logic.
271-286: Improved data cleaning and bug detection logic.The added
str.strip()and.lower()calls defensively handle whitespace and case variations in the "TestClassification" column. The bug detection logic correctly identifies if any test case for a bug failed (line 274-278), then counts bugs per configuration (lines 282-287). This is a sound approach for aggregating test results.
310-312: Improved title formatting and clarity.The multi-line formatting of the figure title improves readability, and the title accurately reflects the table's content.
.github/workflows/make.yml (3)
14-16: Condensed apt commands with quiet flag.The combined
apt -q updateandapt -yq installimproves CI output clarity by suppressing unnecessary package manager output. The-yflag auto-confirms installation, and-qsuppresses verbose output. This is a reasonable improvement.
19-22: Using official uv installer script is appropriate.The curl-based installation of uv from
astral.sh/uv/install.shis the official recommended method. The installer automatically adds~/.local/binto$PATH, makinguvanduvxavailable for subsequent workflow steps. This aligns with the Makefile's reliance onuvx ruffcommands.Verify that the uv installer's PATH modifications persist across steps in GitHub Actions. The installer script should export
PATH=$HOME/.local/bin:$PATH, which should be available in subsequent steps.
24-25: Reformatted for workflow consistency.The multi-line shell block format is consistent with other steps in the workflow (e.g., "Install shellcheck and checkbashisms"). This change has no functional impact.
Makefile (2)
31-39: uvx ruff approach is sound for tool management.Using
uvx ruffis appropriate for CI environments; it manages tool isolation and automatic installation. The version output (lines 31, 37) provides debugging information during CI runs. However, these could optionally be removed to reduce verbose output, but they're not problematic.
41-43: Consistent with project's uv-based tooling and dev dependencies.The switch from
mypytouv run ty checkaligns with the project's adoption of uv for tool management. Thetypackage is declared inpyproject.tomldev dependencies, anduv runexecutes it within the uv environment. This ensures reproducible type checking with the pinned version.pyproject.toml (1)
8-8: Clarify Python 3.14 minimum requirement.
requires-python = ">= 3.14"specifies Python 3.14 as the minimum supported version. While Python 3.14 is now released (October 2025), this is an unusually restrictive requirement that excludes users on Python 3.11, 3.12, and 3.13. Confirm whether this is intentional (e.g., due to required features in 3.14) or should be lowered to a more permissive version like>= 3.11.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/make.yml(1 hunks)Makefile(2 hunks)pyproject.toml(1 hunks)scripts/experiment-scripts/generate-grt-figures.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.7)
scripts/experiment-scripts/generate-grt-figures.py
31-31: Redefinition of unused matplotlib.figure from line 29
Remove definition: matplotlib.figure
(F811)
🔇 Additional comments (5)
Makefile (1)
11-24: Good integration of build dependency changes.The
.plume-scriptstarget correctly establishes a prerequisite repository, and the newstyle-check: markdownlint-checkdependency (line 24) properly chains the markdown linting step into the style-check flow. The shell script globs in lines 54–55 correctly exclude the new.plume-scriptsdirectory to avoid linting its contents..github/workflows/make.yml (1)
14-24: Clean workflow restructuring for uv-based build system.The multi-line formatting improves clarity, and the addition of the Python
uvinstaller (lines 20–21) properly supports the newuv_buildbackend introduced inpyproject.toml. The quiet flags onaptcommands reduce unnecessary CI log output. Ordering and sequencing of steps is logical.pyproject.toml (3)
1-6: Build-system configuration is properly scoped.The new
[build-system]section correctly specifiesuv_buildas the backend with an appropriate version constraint (>= 0.9.11, < 0.10.0), preventing potential breaking changes from future minor/major versions.
14-14: PyYAML dependency addition aligns with new build scripts.The new
pyyaml >= 6.0.3dependency correctly supports the YAML-based configuration introduced by the build scripts (subject-programs/build-info.yamlper the PR summary).
1-16: No action needed. Python 3.14 was released on October 7, 2025, and is the current stable version as of December 2025. Therequires-python = ">=3.14"requirement in pyproject.toml is valid and intentional.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
1-4: Verify uv_build version constraint and consider the existing whitespace issue.The version constraint for
uv_buildis quite restrictive (>= 0.9.11, <0.10.0). Please verify that this version range is appropriate and that version 0.9.11 exists and is stable.Additionally, the past review comment flagged trailing whitespace at line 4 (the blank line after
build-backend). Consider addressing that issue if it hasn't been resolved.Please search for the latest uv_build documentation to verify the version:
What is the latest stable version of uv_build Python package?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.python-version(1 hunks)pyproject.toml(1 hunks)
🔇 Additional comments (4)
pyproject.toml (3)
10-10: Python version requirement corrected appropriately.The change from
>=3.14to>=3.10is appropriate, as Python 3.14 is not yet available. This aligns with the.python-versionfile change. However, please ensure that all dependencies are compatible with Python 3.10 (see comment on.python-version).
14-14: LGTM! PyYAML dependency aligns with PR objectives.The addition of
pyyaml>=6.0.3is appropriate given that the PR introduces YAML-based build manifest for project configurations. The version compatibility with Python 3.10 will be verified as part of the overall dependency check.
6-6: Verify that the project name change doesn't break existing references.The project name has been simplified from "grt-testing-branch-main" to "grt-testing". Ensure this change doesn't affect imports, package references, build configurations, or CI/CD scripts that depend on the old name. Check pyproject.toml's effect on any: setup.py or setup.cfg if present, GitHub Actions workflows, documentation, or version constraints in downstream dependencies.
.python-version (1)
1-1: The review comment is based on incorrect dependency versions that do not exist.The cited dependency versions (
matplotlib>=3.10.7,pandas>=2.3.3, etc.) do not appear to exist on PyPI. matplotlib's latest version is in the 3.8-3.9 range, and pandas 2.x is in the 2.0-2.2 range. Verify the actual dependencies inpyproject.tomlbefore assessing Python 3.10 compatibility. If real dependencies are specified, their actual requires_python constraints should be checked against Python 3.10 to ensure compatibility.Likely an incorrect or invalid review comment.
| dev = [ | ||
| "ty>=0.0.1a31", | ||
| ] | ||
| # dynamic = ["version"] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing or documenting the commented line.
The commented # dynamic = ["version"] line suggests future work with dynamic versioning. If this is planned, consider adding a TODO comment explaining the intent. Otherwise, consider removing it to reduce clutter.
🤖 Prompt for AI Agents
In pyproject.toml around line 22, there is a commented line "# dynamic =
[\"version\"]"; either remove this commented line if it's not needed to reduce
clutter, or replace it with a short TODO comment documenting intent (e.g., note
that dynamic versioning via setuptools_scm or similar is planned and link to
issue/PR) so future readers know whether it's intentional work-in-progress.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
11-13: Shallow clone recommended for bandwidth optimization.Based on the previous review feedback, consider using a shallow clone (--depth 1) to reduce the fetch size:
.plume-scripts: - git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts + git clone -q --depth 1 https://github.com/plume-lib/plume-scripts.git .plume-scriptsThis aligns with the optimization suggestion from the prior review.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/make.yml(1 hunks).gitignore(2 hunks)Makefile(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
.gitignore
🔇 Additional comments (4)
.gitignore (3)
19-25: ✓ Subject-programs directory and file patterns are well-structured.The new ignore patterns correctly use trailing slashes for directories (src-grt/, src-upstream/, etc.) and no slash for files (files.diff), following the existing style conventions in the file. These align well with the PR's subject-programs reorganization.
34-34: ✓ Comment typo fixed.The documentation comment was corrected from "runnin" to "running", improving clarity.
38-38: ✓.pymarkdownentry is appropriate.The new
.pymarkdownentry correctly ignores markdown linter output, keeping generated artifacts out of version control.Makefile (1)
11-20: Redundant clone logic: explicit target and fallback conditional are both cloning.Lines 11–13 define an explicit
.plume-scriptstarget that clones the repository, while lines 17–19 include a fallback conditional clone that executes at include-time if the directory doesn't exist. Both mechanisms serve the same purpose and will clone the repository if it's missing.Consider consolidating to a single approach:
- Option 1 (preferred): Keep only the explicit target (lines 11–13) and remove the fallback conditional (lines 17–19). Rely on the target's idempotency and the conditional include guard.
- Option 2: Keep only the fallback conditional (lines 17–19) if immediate availability at include-time is critical, and remove the explicit target (lines 11–13) to avoid confusion.
.plume-scripts: git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts # Code style; defines `style-check` and `style-fix`. CODE_STYLE_EXCLUSIONS_USER := --exclude-dir subject-programs -ifeq (,$(wildcard .plume-scripts)) -dummy != git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts -endif include .plume-scripts/code-style.mak
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
11-13: Apply shallow clone to reduce bandwidth.The past review suggested using
--depth 1to make the clone lightweight, but it was not applied. This is still unaddressed..plume-scripts: - git clone -q https://github.com/plume-lib/plume-scripts.git .plume-scripts + git clone -q --depth 1 https://github.com/plume-lib/plume-scripts.git .plume-scripts
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Makefile(1 hunks)
🔇 Additional comments (1)
Makefile (1)
16-16: VerifyCODE_STYLE_EXCLUSIONS_USERvariable usage.The new variable
CODE_STYLE_EXCLUSIONS_USER := --exclude-dir subject-programsis defined but its usage should be confirmed in the included.plume-scripts/code-style.makfile to ensure it's correctly integrated into the style-check and style-fix targets.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/make.yml (1)
22-23: Remove the duplicateuvinstall step (already installed viaastral-sh/setup-uv).This is redundant and may introduce PATH/version inconsistencies vs the action-managed install.
pyproject.toml (1)
23-23: Either remove or document the intent of# dynamic = ["version"].
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/make.yml(1 hunks).gitignore(2 hunks)pyproject.toml(3 hunks)scripts/mutation-randoop.sh(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
scripts/mutation-randoop.sh.gitignore
🔇 Additional comments (6)
.github/workflows/make.yml (1)
25-26: Style-check command change is fine..gitignore (2)
19-25: New ignore patterns match the subject-programs source layout.
34-35: Comment typo fix is fine.scripts/mutation-randoop.sh (2)
465-471: Addingchecker-qual.jarto the Randoop classpath is reasonable for impurity annotation.
367-368: Jaxen jar path update looks OK; ensure the new jar is actually present insubject-programs/jars.If that jar is produced/downloaded elsewhere now, this branch will fail at runtime.
#!/bin/bash # quick repo check (adjust if paths differ) ls -la subject-programs/jars | sed -n '1,200p'pyproject.toml (1)
1-3: No action needed. Theuv_build >= 0.9.11, <0.10.0backend configuration is correct and uses a valid stable version released in late 2025. The backend name and version range will not break packaging or CI.Likely an incorrect or invalid review comment.
3 participants
@varuniy @776styjsu
Should this pull request be merged?