Skip to content

fix(uipath-review): reduce review verbosity - #2471

Open
andreiancuta-uipath wants to merge 16 commits into
mainfrom
fix/reduce-review-verbosity
Open

fix(uipath-review): reduce review verbosity#2471
andreiancuta-uipath wants to merge 16 commits into
mainfrom
fix/reduce-review-verbosity

Conversation

@andreiancuta-uipath

@andreiancuta-uipath andreiancuta-uipath commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Ran reviews locally and compared the outputs.

Changes:

  • Eliminate "Rule Findings" section; instead, reuse the existing Critical / Warning / Info sections. Reorganize issue list into tables for clear delimitation.
  • Restate grade at the bottom of the report, for better visibility.
  • Remove a few stale/non-existent rules.
  • Remove the agent checklist and common issues files; replaced with new catalog rules where applicable. The checklist items were usually broad, leading to the reviewer agent finding random and sometimes unfounded issues. Prefer more precise rules instead.
  • Others:
    • Relax lowcode-stale-files test: only verify final report; don't assert generated files were not read.
    • Repair a codedagent fixture

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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 (skill-review-agents-lowcode-stale-generated-files) has been run and passed. The PR body mentions running reviews locally and comparing outputs, but does not assert the coder-eval task itself passed. Please edit the PR description to add a line like:

Ran skill-review-agents-lowcode-stale-generated-files locally and it passed.

Per-task lint

tests/tasks/uipath-review/agents/lowcode_review_with_stale_generated_files/lowcode_review_with_stale_generated_files.yaml — verdict: OK

No issues. The task is well-designed:

  • No self-report anti-pattern — the run_command criterion delegates to check_stale_generated_files.py which validates derived state and report content via side effects, not agent prose.
  • Prompt is goal-oriented (line 22: "review it and save the structured report") without leaking procedure.
  • Meaningful coverage — criteria span skill trigger, read-only contract (Write/Edit exclusion), three CLI commands (refresh, validate, review), and a substantive Python checker (weight 6.0) that validates derived JSON state, report structure, canary absence, defect-section correctness, and grade consistency.
  • Not gameable — the Python checker cross-validates agent.json against entry-points.json and inspects report sections for false-positive defect filings.
  • Not a near-duplicate — the stale-generated-files scenario (pre-seeded stale entry-points.json + generated-tree canaries) is unique among siblings, which focus on judgment-catalog rule violations (tool overlap, vague descriptions, prompt conflicts, guardrails).
  • CLI verbs agent refresh, agent validate, agent review are current catalog verbs.

Changes in this PR: Removed the command_not_executed criterion that checked the agent did not Read .agent-builder or .local/build paths (line 41–45 in the base version). The check_stale_generated_files.py script was updated to verify only the final report content (defect sections, grade footer) rather than also asserting generated files were never read. This is a reasonable relaxation — reading generated trees is not inherently wrong if the report doesn't misreport their state as defects.

Within-PR duplicates

  • No duplicate clusters detected (only 1 task YAML changed).

Conclusion

  • ⚠ Task passes the rubric, but no evidence of a passing coder-eval run was claimed in the PR body. Advisory only — not blocking merge.

--- ·

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @andreiancuta-uipath's task in 3m 4s —— View job


Review: fix(uipath-review): reduce review verbosity

  • Read review criteria and project conventions
  • Analyze full diff (13 files, +224/−351)
  • Read changed files in full context
  • Check skill-specific rules (links, cross-refs, self-containment)
  • Post review findings

Summary

This 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 Review

1. skills/uipath-review/SKILL.md (+41/−36)

Severity: OK

Report template updated coherently: "Rule Findings" section removed, findings merged into the three severity tables (Critical / Warning / Info) as table rows, "Rules Skipped" promoted to its own subsection with a table format, **Final grade: <A–F>** added as the report's last line. The guidance note at line 575 is clear and well-placed. Critical Rules 7, 10, 11 updated consistently to reflect the new format. All changes are internally consistent with each other and with the broader instruction set (Step 2.5b merge instruction now says "into the Critical / Warning / Info findings tables").

2. skills/uipath-review/references/agents/agent-common-issues.md (+7/−7)

Severity: OK

Removed references to non-existent rule IDs (LOWCODE_TOOL_GUARDRAIL_FIELD_MISSING, LOWCODE_GUARDRAIL_TOOL_REF_NONEXISTENT, NO_EVALS, MISSING_EVAL_DIR, TOO_FEW_EVALS, FEW_EVALS, EVAL_LLM_JUDGE_ONLY, EVAL_NO_LLM_JUDGE, EVAL_LOW_DIVERSITY, EVAL_MODERATE_DIVERSITY, EVAL_UNIFORM_EXPECTED_BEHAVIOR, NO_TRACING, EVAL_TRAJECTORY_NEEDS_TRACE_SPANS, LOWCODE_CONTEXT_INDEX_NAME_PLACEHOLDER). Replaced with the correct current rule IDs that actually exist in the catalogs or the review CLI. The cross-reference notes now point to rules that the skill actually defines or the CLI actually emits.

3. skills/uipath-review/references/agents/agent-grading-rubric.md (+1/−1)

Severity: OK

Description of what uip agent review / uip codedagent review checks updated to match the narrower, accurate list used elsewhere in this PR. Consistent.

4. skills/uipath-review/references/agents/agent-review-checklist.md (+73/−252)

Severity: Low

Major rewrite: removes ~250 lines of checklist items that duplicated CLI deterministic checks or judgment-catalog rules (schema validation, system prompt checks, tool configuration details, error handling, framework-specific structural checks, eval existence/count/evaluator-selection checks). Replaced with higher-altitude checks focused on deployment fitness, operational context, and decisions the CLI/catalog cannot make (model fit, deployment environment verification, sync cadence, escalation SLA, mocking quality, platform constraints).

The intro note at line 5 is good: "Do not reproduce checks those commands actually performed."

One observation: the old checklist had explicit *(rule: RULE_ID)* annotations on rows that overlapped with catalog rules. The new checklist drops these entirely. This is fine since the overlapping rows themselves are removed, but worth noting that any new checks added in the future should follow the same annotation pattern if they overlap.

Section 8 "AI Trust Layer Audit" was retained with minor wording polish — appropriate since those are environment-level checks the CLI doesn't cover.

5. skills/uipath-review/references/agents/agents-lowcode-rules.md (+11/−2)

Severity: OK

Five new judgment rules added: LC_EVAL_INPUT_DIVERSITY, LC_EVAL_ARCHETYPE_FIT, LC_EVAL_JUDGE_RUBRIC_FIT, LC_TOOL_PURPOSE_AUTHORITY_FIT, LC_PROMPT_REFERENCES_NONEXISTENT_RESOURCE, LC_PROMPT_STRUCTURE, LC_PROMPT_EXAMPLE_MAINTAINABILITY, LC_PROMPT_DEPLOYMENT_SPECIFIC_LOGIC, LC_PROMPT_ESCALATION_CRITERIA. All follow the established row schema (rule_id, severity, category, trigger, detection_method, suggested_fix). The intro paragraph's description of CLI checks updated to match.

One rule LC_FAILURE_PROMPT_INJECTION was updated to broaden the trigger language ("Untrusted-content instruction boundary is ineffective" vs the old "Tool-output injection risk") — this is a semantic improvement that reduces false positives from literal-delimiter checks.

6. skills/uipath-review/references/agents/guardrails/coded-guardrails-review.md (+2/−2)

Severity: OK

Report format instruction updated from the old line format to the new table-row format. Consistent with SKILL.md changes.

7. skills/uipath-review/references/agents/guardrails/guardrails-review.md (+2/−2)

Severity: OK

Same report format update as coded-guardrails-review.md. Consistent.

8. skills/uipath-review/references/review-workflow-guide.md (+19/−10)

Severity: Medium — template inconsistency with SKILL.md

Report template updated to table format for findings and the **Final grade:** footer added. The "Validation Details" now correctly says "Errors and Warnings only" and drops the V-I- line. The V-I- Info detail line was removed from the template (line 331 old), consistent with the SKILL.md instruction that Info feeds only the table count.

Issue: The report template in this file (lines 321–368) is missing the ### Rules Skipped subsection that the SKILL.md template (line 547) includes. SKILL.md's template has ### Rules Skipped between "Validation Details" and "Critical Findings", but review-workflow-guide.md's template jumps directly from "Validation Details" to "Critical Findings". Since many references point to the "Rules Skipped" subsection by name (found 20+ references across the skill), both templates should include it.

Fix this →

9. skills/uipath-review/references/rule-catalog-workflow.md (+5/−5)

Severity: OK

Updated to reference "Critical / Warning / Info findings tables" instead of "Rule Findings subsection." The merge instruction and CLI description updated consistently. The new wording at line 85 ("A finding appears in exactly one table, once") is a useful clarity improvement.

10. skills/uipath-review/references/rule-format.md (+1/−1)

Severity: OK

Minor: changed "same 'Rule Findings' subsection" to "same severity table." Consistent.

11. tests/.../inject_output_enum_missing.py (+32/−3)

Severity: OK

The fixture now synchronizes entry-points.json schemas with the injected main.py Input/Output models. This is a real fix — the coded agent scaffold's write_baseline_function_agent generates a generic entry-points.json, but after overwriting main.py with specific schemas, the entry-points.json was stale. The new code reads the entry-points JSON, patches the first entry point's input and output to match the injected classifier, and writes it back. Schema values match the MAIN_PY Pydantic models exactly.

12. tests/.../check_stale_generated_files.py (+26/−19)

Severity: OK

Key changes:

  • REFRESH_MANAGED_TOKENS expanded to include .agent-builder and .local/build (previously only checked entry-points.json, legacyrequest, legacyresponse). This matches SKILL.md Critical Rule 14's CLI-managed paths.
  • DEFECT_SECTIONS renamed to FINDING_SECTION_PREFIXES and uses startswith() instead of in. The new section names ("critical findings", "warnings", "improvement opportunities") match the PR's updated report section headers.
  • The grade-checking logic is now stricter and better: requires both a Summary **Agent Grade:** line with a valid letter AND a **Final grade: <letter>** at the report's very end, and verifies they match. This is correct enforcement of the new Final grade: footer.
  • The \Z anchor in the final_grade regex (line 98) is correct for matching the absolute end of the (rstripped) report.

13. tests/.../lowcode_review_with_stale_generated_files.yaml (+4/−11)

Severity: OK

  • Removed the command_not_executed criterion that checked agents didn't Read .agent-builder or .local/build paths. The PR description explains this relaxation.
  • Task description and run_command description updated to reflect the new grade-footer assertion.
  • The weight: 6.0 on the run_command criterion (the highest-weighted check) is appropriate since it now validates more: derived state + report boundary + grade footer match.

What's Missing

  • review-workflow-guide.md template missing "Rules Skipped" section — The SKILL.md template includes ### Rules Skipped with the table format between Validation Details and Critical Findings, but the review-workflow-guide.md template does not. Since this is a secondary template (the SKILL.md one is authoritative), this is Medium severity but should be fixed for consistency. (See finding in item 8 above.)

Area Ratings

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 "Check settings.model is 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.md are 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.
@andreiancuta-uipath
andreiancuta-uipath force-pushed the fix/reduce-review-verbosity branch from 7587446 to 0eb6eae Compare August 7, 2026 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant