fix(uipath-review): reduce review verbosity - #2471
Conversation
|
Claude finished @andreiancuta-uipath's task in 2m 36s —— View job Coder-eval task lint (advisory)1 task YAML changed; verdict counts: 0 Critical, 0 High, 0 Medium, 0 Low, 1 OK. Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge. Evidence of passing run❌ High — PR body does not claim the changed task (
Per-task lint
|
|
Claude finished @andreiancuta-uipath's task in 3m 4s —— View job Review: fix(uipath-review): reduce review verbosity
SummaryThis PR consolidates the review report format by eliminating the standalone "Rule Findings" section and merging all findings into the existing Critical / Warning / Info tables, restates the grade at the report tail for visibility, removes stale/phantom rule IDs from the common-issues catalog and checklist, replaces low-code-agent checklist items that overlapped with judgment-catalog rules with higher-altitude deployment-context checks, and relaxes the stale-generated-files test to focus on the final report rather than asserting generated files were never read. Change-by-Change Review1.
|
| Area | Status | Notes |
|---|---|---|
| Frontmatter | OK | No frontmatter changes in this PR |
| E2E Tests | OK | Test files updated consistently with the new report format; grade-footer assertion is a good addition |
| Skill Body | OK | SKILL.md report template and critical rules updated coherently |
| References & Assets | Issue | review-workflow-guide.md template is missing the "Rules Skipped" section present in SKILL.md |
| Repo Hygiene | OK | No secrets, no cross-skill references, scoped changes |
Issues for Manual Review
- The large checklist rewrite (
agent-review-checklist.md) removes many specific, actionable checks and replaces them with higher-altitude items. This is intentional per the PR description (removing overlap with CLI + judgment catalog), but the new items are notably less prescriptive (e.g., "Compare model capabilities and deployment constraints with the intended workload" vs the old "Checksettings.modelis set"). Worth validating that the remaining coverage is sufficient for an LLM agent to follow without the removed specificity. - The nine new judgment rules in
agents-lowcode-rules.mdare well-formed but have not been verified against a real agent review run. Confirm they fire correctly and don't over- or under-trigger.
Conclusion
Well-structured PR that achieves its stated goal of reducing review verbosity by consolidating findings into unified tables and removing duplicate/stale content. The report format changes are internally consistent across SKILL.md, guardrails references, rule-catalog-workflow, and rule-format. The test updates correctly enforce the new format including the grade footer.
One Medium finding: the review-workflow-guide.md report template is missing the "Rules Skipped" section that SKILL.md includes. Recommend adding it for consistency before merging.
Otherwise the changes look correct and ready.
|
chore: revert some overeager deletions
Validation Details restated what each command checks and that it passed -- Info counts already carry that in the table. Detail lines now cover Errors and Warnings only.
…e IDs The skill claimed `uip agent review` runs eval count/diversity checks. It does not: the shipped agent-reviewer-sdk registry has 41 rules and none score eval diversity. EVAL_LOW_DIVERSITY / EVAL_MODERATE_DIVERSITY were deliberately excluded from CLI v1 (sentence-transformer embeddings are too heavy for the browser target) and the port plan assigned them to the wrapping skill, which never picked them up. Reviewing an agent with a 1-datapoint eval set therefore produced a fabricated Rules Skipped row speculating about which rules the CLI ships. - Capability lists now defer to the CLI's output as the authority on which deterministic checks exist, and state that an empty Data.Issues[] means no rule fired -- never grounds for a Rules Skipped entry. - Drop 15 rule IDs cited across the skill that exist in neither the CLI registry nor a judgment catalog (Critical Rule 12 forbids citing them), including the phantom in the report template's own example row. - Add LC_EVAL_INPUT_DIVERSITY to the low-code judgment catalog, claiming the ownership the port plan intended. It assesses scenario classes semantically rather than asserting a datapoint count.
7587446 to
0eb6eae
Compare
Ran reviews locally and compared the outputs.
PTOQueryAgent-review-report-codex-full-original.md
PTOQueryAgent-review-report-codex-full-new.md
Changes: