Drafted documentation for the autograder overlay system#4
Drafted documentation for the autograder overlay system#4dnunez02 wants to merge 8 commits intopawtograder:mainfrom
Conversation
…rojects with Gradle
…. Added yaml syntax highlighting for easier reading
…rlay autograder and further rewrote the overview to refer to only those three main properties
…oved superfluous wording
📝 WalkthroughWalkthroughAdds a comprehensive autograding guide at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 11
🤖 Fix all issues with AI agents
In `@docs/staff/assignments/autograding.md`:
- Line 14: Fix the numbered list formatting by replacing the stray prefix "s4."
with "4." in the affected markdown line that mentions `submissionFiles` in the
"Files" section of the student's results on Pawtograder; locate the line
containing 's4. The `submissionFiles` are listed in the "Files" section of the
student's results on Pawtograder.' and change it to start with '4.' so the
ordered list sequence renders correctly.
- Line 106: Typo: change "Thee" to "These" in the sentence that reads "Thee
steps are commented out to prevent issues with building..." — locate that phrase
in the autograding documentation (the paragraph that starts "Pawtograder
automatically populates steps 1 and 3...") and replace "Thee" with "These"; also
search the same doc for any other occurrences of "Thee" and correct them
similarly.
- Line 27: Update the markdown line describing the `cmd` property so there's a
space after the colon; locate the line containing `` `cmd`:the CLI command for
running your testing infrastructure. `` and change it to include a space after
the colon (i.e., `` `cmd`: the CLI command for running your testing
infrastructure. ``) to fix formatting.
- Line 13: Replace the duplicated word in the sentence describing the grading
summary: change "organized according the the `gradedParts` collection" to
"organized according to the `gradedParts` collection" in
docs/staff/assignments/autograding.md so the phrase reads correctly and
references the gradedParts collection.
- Line 158: Change the sentence describing the workflow file to use singular
form: replace "These are the GitHub workflow that runs with each student
submission" with "This is the GitHub workflow that runs with each student
submission." — update the reference to `grade.yml` and keep the mention of the
autograder and `main` branch unchanged.
- Line 61: Update the documentation string for `gradedUnits` to fix grammar:
change "an array of the specific units that make up a graded parts" to "an array
of the specific units that make up a graded part" so the phrase describes a
single graded part correctly.
- Line 87: Typo in the property name: replace the incorrect `file`_ token with
the correct `files` property in the documentation text (the phrase "`file`_ are
the source code...") so it reads "`files` are the source code and any auxiliary
files you want to either automatically grade or manually grade."; update the
string in the docs where `file`_ appears to `files`.
- Around line 324-408: The markdown code fence containing the Gradle build
snippet (the block that starts with the plugins { ... } section and includes
jacocoTestReport, test, application, checkstyle, and pitest settings) is missing
a language identifier; update the opening triple-backtick to include a language
(e.g., change ``` to ```gradle) so the block is fenced as ```gradle to satisfy
markdown linting and enable proper syntax highlighting.
- Line 104: Replace the misspelled filename string "pawotgrader.yml" with the
correct "pawtograder.yml" in the docs text (the sentence containing "Collect
Submission and Run Grader" that references the autograder configuration file);
ensure any other occurrences of "pawotgrader.yml" in the same document are
updated to "pawtograder.yml" for consistency.
- Line 251: Remove the stray backtick before the YAML key `testCount: 1` and
restore proper YAML indentation so `testCount` is a normal key (e.g., align it
with the surrounding keys in the same mapping); locate the `testCount` entry in
the document and delete the leading backtick and any incorrect spacing so the
YAML parses correctly.
- Around line 85-88: Update the documentation to use the camelCase field name
consistently: change any occurrences of `test_files` to `testFiles` so it
matches `submissionFiles` and the YAML examples; also correct the verb form
"autograder" to "autograde" where used (e.g., in the description for `files` and
`testFiles`) to keep wording and field naming consistent with the rest of the
document.
🧹 Nitpick comments (5)
docs/staff/assignments/autograding.md (5)
28-28: Verify the artifacts property behavior.The TODO comment indicates uncertainty about whether artifacts are "generated by the build" or if this description is accurate. This needs to be verified against the actual implementation before release.
Do you want me to help search the codebase for the artifacts property implementation to clarify its behavior?
45-45: Verify the cache_key property purpose.The TODO comment indicates the purpose of
cache_keyis unclear. This property description should be verified and clarified before the documentation is released.Do you want me to help search the codebase or schema definition to clarify what the
cache_keyproperty does?
74-74: Verify default behavior for partial credit.The TODO comment indicates uncertainty about whether partial credit is enabled by default. This should be verified against the actual implementation to ensure documentation accuracy.
Do you want me to search the codebase for the default value of
allow_partial_credit?
78-78: Verify and clarify mutation testing locations format.The TODO comment indicates uncertainty about the locations array format. The description "A location is an array of strings reporting the locations of the mutants" is circular and needs clarification about what these location strings represent (e.g., file paths, class names, method signatures).
Do you want me to search for mutation testing implementation details to clarify the expected format?
299-299: Clarify trailing period syntax for class-level test selection.The test name "SimpleBoxSetVisibleTest." ends with a period, which appears to be intentional syntax for selecting all tests in a class (as mentioned on line 271). However, this syntax is not explicitly documented in the "Graded parts" section. Consider adding a note in the documentation explaining that a trailing period after a fully qualified class name selects all tests in that class.
The same clarification applies to line 308 with "SimpleBoxSetHiddenTest."
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/staff/assignments/autograding.md`:
- Line 94: Update the incorrect capitalization of "Github" to the proper
"GitHub" wherever it appears in the document; specifically replace the heading
text "## Github Workflow and `grade.yml`" and the other occurrence of the word
"Github" (e.g., at the second noted occurrence) so the file consistently uses
"GitHub" throughout.
- Line 29: Fix the typo in the documentation sentence describing the `linter`
property: change the word "liner" to "linter" so the line reads that the
`linter` property specifies how code style should be checked and the results of
running that linter; update the single occurrence in the text referring to the
`linter` property.
🧹 Nitpick comments (4)
docs/staff/assignments/autograding.md (4)
28-28: TODO comments indicate uncertain behavior.Lines 28 and 45 contain TODO comments marking uncertain property behaviors. Consider verifying the actual behavior of the
artifactsandcache_keyproperties to complete the documentation.Do you want me to help search for documentation or implementation details to clarify these properties?
Also applies to: 45-45
74-74: TODO comments indicate uncertain behavior.Lines 74 and 78 contain TODO comments about property behavior. Verify whether
allow_partial_creditis enabled by default and clarify the structure of thelocationsarray for mutation testing units.Would you like me to help search for implementation details or documentation to clarify these behaviors?
Also applies to: 78-78
109-109: TODO: Complete build system integration description.The TODO comment indicates that build system integration needs to be described. This section appears incomplete.
Would you like me to help draft content describing how the pawtograder.yml integrates with Gradle and Python build systems?
158-158: Simplify sentence structure.The phrasing "The contents within are the GitHub workflow" is awkward. Consider simplifying to improve clarity.
♻️ Proposed simplification
-This is the `grade.yml` for that same assignment. The contents within are the GitHub workflow that runs with each student submission. It is the bare minimum version that installs Java 21 Temurin on an image and runs the above autograder whenever we push to the `main` branch. +This is the `grade.yml` for that same assignment. This GitHub workflow runs with each student submission. It is the bare minimum version that installs Java 21 Temurin on an image and runs the above autograder whenever we push to the `main` branch.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/staff/assignments/autograding.md`:
- Line 321: Change the incorrect subject–verb agreement in the sentence so
"section" (singular) matches the verb: replace "The `plugins` section need to
know the name of the mutation testing library, pitest, and its version." with
"The `plugins` section needs to know the name of the mutation testing library,
pitest, and its version." and ensure backticks around `plugins` and `pitest`
remain as shown.
- Line 324: The fenced code block currently labeled with the wrong language tag
("yaml") should be changed to "gradle" so syntax highlighting matches the
Groovy/Gradle content; locate the triple-backtick fence in the
docs/staff/assignments/autograding.md file (the block around the Gradle snippet)
and replace the opening "```yaml" with "```gradle" (leave the closing "```"
unchanged).
- Line 100: Add a period at the end of the sentence "When a student pushes their
code to GitHub on the branches specified in `grade.yml`, the grading job
activates and proceeds through each of the steps. By default, there are three
steps performed" so it ends with a period before the numbered list; update that
sentence text to include the trailing period to prevent it from running into the
list.
… unclear wording and punctuation issues.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/staff/assignments/autograding.md`:
- Around line 299-309: Remove the accidental trailing periods from the test
identifiers in the example under "Hidden Test Results": change
"SimpleBoxSetVisibleTest." and "SimpleBoxSetHiddenTest." to
"SimpleBoxSetVisibleTest" and "SimpleBoxSetHiddenTest" respectively (the entries
under gradedUnits/tests) so the examples show the fully qualified class names
without the trailing dot.
- Line 13: Replace the incorrect phrase in the sentence "A grading summary is
generated from the results of the tests run. This summary is organized according
the `gradedParts` collection and reported to the students in their submission."
by changing "organized according the `gradedParts` collection" to "organized
according to the `gradedParts` collection" so the sentence reads grammatically
correct.
🧹 Nitpick comments (1)
docs/staff/assignments/autograding.md (1)
28-45: Resolve TODOs or clearly label this as draft-only content.Multiple TODOs indicate uncertain behavior (e.g.,
artifacts,cache_key, mutationlocations, partial-credit default). If this doc is intended for staff consumption, these should be verified or flagged as “draft/unverified” in a single consistent banner to avoid misleading guidance.If you want, I can help draft a “Known unknowns” callout or propose final text once behavior is confirmed.
Also applies to: 74-79
|
|
||
| 1. The system checks for all the files under the `submissionFiles` section. If any of those files are missing, the autograder aborts and reports those files are missing in the Grading Summary. | ||
| 2. The autograder runs according to the `build` configuration. If the run fails, the failure is reported and no autograder results are given. | ||
| 3. A grading summary is generated from the results of the tests run. This summary is organized according the `gradedParts` collection and reported to the students in their submission. |
There was a problem hiding this comment.
Fix missing word (“according to”).
The sentence reads ungrammatically and should use “according to.”
📝 Suggested fix
-3. A grading summary is generated from the results of the tests run. This summary is organized according the `gradedParts` collection and reported to the students in their submission.
+3. A grading summary is generated from the results of the tests run. This summary is organized according to the `gradedParts` collection and reported to the students in their submission.🤖 Prompt for AI Agents
In `@docs/staff/assignments/autograding.md` at line 13, Replace the incorrect
phrase in the sentence "A grading summary is generated from the results of the
tests run. This summary is organized according the `gradedParts` collection and
reported to the students in their submission." by changing "organized according
the `gradedParts` collection" to "organized according to the `gradedParts`
collection" so the sentence reads grammatically correct.
| - SimpleBoxSetVisibleTest. | ||
| - name: Hidden Test Results | ||
| hide_until_released: true | ||
| gradedUnits: | ||
| - name: Simple BoxSet Hidden | ||
| points: 25 | ||
| testCount: 5 | ||
| allow_partial_credit: true | ||
| tests: | ||
| - SimpleBoxSetHiddenTest. | ||
| submissionFiles: |
There was a problem hiding this comment.
Trailing dot in test identifiers looks accidental.
SimpleBoxSetVisibleTest. and SimpleBoxSetHiddenTest. likely shouldn’t end with a dot, especially since earlier text says “fully qualified class name.” Remove the trailing dot to avoid confusion in examples.
📝 Suggested fix
- - SimpleBoxSetVisibleTest.
+ - SimpleBoxSetVisibleTest
...
- - SimpleBoxSetHiddenTest.
+ - SimpleBoxSetHiddenTest📝 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.
| - SimpleBoxSetVisibleTest. | |
| - name: Hidden Test Results | |
| hide_until_released: true | |
| gradedUnits: | |
| - name: Simple BoxSet Hidden | |
| points: 25 | |
| testCount: 5 | |
| allow_partial_credit: true | |
| tests: | |
| - SimpleBoxSetHiddenTest. | |
| submissionFiles: | |
| - SimpleBoxSetVisibleTest | |
| - name: Hidden Test Results | |
| hide_until_released: true | |
| gradedUnits: | |
| - name: Simple BoxSet Hidden | |
| points: 25 | |
| testCount: 5 | |
| allow_partial_credit: true | |
| tests: | |
| - SimpleBoxSetHiddenTest | |
| submissionFiles: |
🤖 Prompt for AI Agents
In `@docs/staff/assignments/autograding.md` around lines 299 - 309, Remove the
accidental trailing periods from the test identifiers in the example under
"Hidden Test Results": change "SimpleBoxSetVisibleTest." and
"SimpleBoxSetHiddenTest." to "SimpleBoxSetVisibleTest" and
"SimpleBoxSetHiddenTest" respectively (the entries under gradedUnits/tests) so
the examples show the fully qualified class names without the trailing dot.
Describes the overlay system in three parts
Describes the relationship between grade.yml and pawtograder.yml
Also includes a couple of real YAML files for setting up Python and Java autograders.
TODO: There are some properties I tried to describe but I don't really know what they do. They require a once-over and are marked with TODO in the documentation.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.