CURA-13166 Add basic instructions to customize Copilot PR review

[Erwan MATHIEU (wawanbreton)]

Hopefully this can make Copilot reviews more relevant. This is still very experimental and will have to be fine-tuned over time, if ever useful.
Feel free to suggest additions if you feel something critical is missing. I do think it should remain short though.

ℹ️ The files were originally based on the ones generated by Jelle, but those are more oriented for agent-generated PRs, which is not what we want, so most of it was irrelevant for our case.

CURA-13166

[Copilot]

🟡 Changes recommended

The PR-specific guidance is stored in an unsupported location and will not be loaded through the Markdown link.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds repository context and review guidance intended to improve Copilot PR feedback.

Changes:

• Documents CuraEngine architecture, development constraints, and tooling.
• Defines concise PR review guidelines.
• Links the PR assistant persona from Copilot’s repository instructions.

File summaries

File	Description
AGENTS.md	Documents architecture and development practices.
.github/copilot-instructions/pr-assistant.instructions.md	Defines PR review guidance.
.github/copilot-instructions.md	Attempts to activate the PR assistant guidance.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The final review comment on the loop-style guidance remains unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.github/copilot-instructions.md:14

• This blanket rule conflicts with existing required exception handling: CommandLine.cpp catches exceptions to prevent an unhandled Windows failure (src/communication/CommandLine.cpp:141-155), and Settings.cpp catches std::invalid_argument while parsing settings (src/settings/Settings.cpp:255-264). Applying it would create false-positive review comments or encourage removal of those error boundaries; please scope the rule to newly introduced exception paths with a concrete problem.

  * Exceptions should not be used

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The Copilot skill has three review findings to address before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.github/skills/code-review/SKILL.md:1

• This file is not a valid GitHub Copilot agent skill without YAML frontmatter, so the review instructions may be ignored. Add the required name and description metadata before the Markdown body.

# Role: Pull Request Assistant

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

[Erwan MATHIEU (wawanbreton)]

Copilot In the PR skill, I clearly mentioned not to generate the review details, however they are here. Is my instruction not good enough, or is there an other way I can disable this verbose and useless set of details ?

[Copilot]

Copilot In the PR skill, I clearly mentioned not to generate the review details, however they are here. Is my instruction not good enough, or is there an other way I can disable this verbose and usel...

Updated in 63310ac to make the output contract stricter: findings-only comments, no overview/file-summary/review-detail/recap sections, and no explanatory summary when there are no actionable findings. If verbose wrapper text still appears, that part is likely platform-generated outside the skill prompt.

[Copilot]

🟡 Changes recommended

The review skill will not be loaded without frontmatter, and its missing-test rule needs clearer scope.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

.github/skills/code-review/SKILL.md:10

• AGENTS.md says coverage is exhaustive only in some areas and that tests are not added for every feature. This rule will therefore produce inconsistent missing-test warnings for code without an established coverage expectation; scope it to code paths with comparable existing tests or make the warning conditional on that convention.

* Issue a warning when a piece of code is quite critical, very suitable for being unit tested, and no test has been added yet

AGENTS.md:17

• last-time modifications is not the standard expression and is unclear here; please use last-minute modifications.

* For each layer, translate the extrusion paths into actual GCode, while applying a few last-time modifications

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No unresolved issues were identified.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Narrow the blanket for-loop preference to counter/range iteration and retain while for condition- or state-driven loops.

Review details

Suppressed comments (1)

.github/skills/code-review/SKILL.md:20

• This still gives reviewers a blanket syntax preference: for can represent the condition-driven loops used for socket state, bucket merging, and worker availability, but that obscures their termination semantics. Please scope this to counter/range-based iteration and explicitly retain while for condition/state-driven loops or when for would reduce readability.

  * Prefer `for` loops over `while` loop whenever possible and if it does not significantly reduce the readability

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite