Skip to content

chore: remove file-group-reviewer droid, update version refs to v5#76

Merged
factory-nizar merged 2 commits intodevfrom
eval/remove-file-group-reviewer
Apr 16, 2026
Merged

chore: remove file-group-reviewer droid, update version refs to v5#76
factory-nizar merged 2 commits intodevfrom
eval/remove-file-group-reviewer

Conversation

@factory-nizar
Copy link
Copy Markdown
Contributor

Summary

Removes the file-group-reviewer custom droid (review methodology now lives in the CLI builtin review skill) and updates all version references from @v3 to @v5.

Changes

  • Remove file-group-reviewer droid and its test — review methodology is now inlined via the CLI builtin review skill
  • Update all @v3 references to @v5 in README.md and .github/workflows/droid.yml
  • Clean up stale references to file-group-reviewer in prompt templates, code comments, and the security-reviewer droid description

factory-nizar and others added 2 commits April 15, 2026 15:59
…ethodology

The review skill now uses worker subagents with the full shared
methodology inlined into each prompt. The file-group-reviewer.md
droid and its tests are no longer needed.
…nces

- Update all @V3 references to @v5 in README and droid.yml workflow
- Remove file-group-reviewer mentions from prompts, comments, and droid descriptions

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@factory-droid
Copy link
Copy Markdown
Contributor

factory-droid bot commented Apr 16, 2026

Droid finished @factory-nizar's task —— View job


Droid review complete.

LGTM — this PR cleanly removes the file-group-reviewer droid/test and updates docs/workflow references to @v5; no correctness or security issues found in the changes.

@factory-nizar factory-nizar merged commit e3d1f5e into dev Apr 16, 2026
3 checks passed
@factory-nizar factory-nizar deleted the eval/remove-file-group-reviewer branch April 16, 2026 18:28
factory-nizar added a commit that referenced this pull request Apr 16, 2026
* fix: forward review_model and reasoning_effort to validator step (#42)

The v3 refactor removed the hardcoded gpt-5.2/high fallback from
review-validator.ts but didn't add REVIEW_MODEL and REASONING_EFFORT
to the Prepare validator step's env block in action.yml. This causes
the validator pass to fall back to the Droid CLI's internal default
model instead of using the user's configured review_model.

* fix: pin upload-artifact to v4.6.2 SHA (#43)

* feat: support custom review guidelines via review-guidelines skill

Read .factory/skills/review-guidelines.md from the workspace and inject
its content into all review prompt templates (code review, candidates,
validator, and security review). This allows repository maintainers to
define repo-specific review guidelines without polluting AGENTS.md.

Closes FAC-16667

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* refactor: address review feedback

- Use .factory/skills/review-guidelines/SKILL.md path (follow skill conventions)
- Extract formatGuidelinesSection() helper to centralize prompt formatting
- Use consistent <custom_review_guidelines> tags across all templates

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat: add size cap and truncation for review guidelines

Truncate review guidelines exceeding 80k characters (matching AGENTS.md
handling) and append a note directing the model to read the full file.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix: match factory-mono truncation pattern more closely

- Reserve space for truncation marker within the size limit (total stays <= MAX)
- Add minimum meaningful space check (>200 chars)
- Nudge model to use tools to read full file when truncated

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* docs: add custom review guidelines section to README

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* refactor: extract MIN_TRUNCATED_CONTENT_CHARS constant

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Consolidate review summary into tracking comment instead of separate PR review body

The review previously posted two comments: the initial tracking comment (updated
with completion status) and a separate PR review summary via submit_review. This
consolidates them so the review summary is written to the tracking comment using
update_droid_comment, and submit_review is called without a body parameter.

Applies to both single-pass review and two-pass validator flows.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Remove extra formatting instructions from review summary prompts

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Remove single-pass review flow, always use two-pass validator

The single-pass review flow was initially created because we were unsure
about the performance of the two-pass validator flow. The two-pass flow
has since become the default and proven itself, and the single-pass flow
is no longer used. This removes the review_use_validator toggle and all
single-pass review code.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Batch all review comments into a single submit_review call

Instead of posting inline comments individually via create_inline_comment
(which creates separate review events for each), batch all comments into
the submit_review comments array. This produces a single cohesive review.

- Updated submit_review tool schema to support line/side/start_line format
- Updated review and validator prompts to batch comments
- Removed create_inline_comment from review allowed tools

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Update github-pr-server.ts

* refactor: move review guidelines from prompt injection to skill invocation

Remove direct injection of review-guidelines file content into orchestrator
prompts. Instead, the file-group-reviewer subagent now invokes the
review-guidelines skill via the Skill tool, ensuring the guidelines are
consumed where they're actually needed (by the reviewer, not the orchestrator).

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix: add Skill to enabled tools so subagents can invoke review-guidelines

The --enabled-tools allowlist passed to droid exec did not include Skill,
which meant file-group-reviewer subagents could not invoke the
review-guidelines skill even though their droid config listed it. Add
Skill to the candidateGenerationTools array in both generate-review-prompt
and review command.

Also strengthen file-group-reviewer instructions to treat custom guideline
violations as mandatory (not stylistic) and make skill invocation the
required first step.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat(review): add suggestion block guidance

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

fix(review): escape suggestion blocks in prompt

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

fix(review): enforce suggestion-only replacements

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

update

* fix(review): allow insert-only suggestions

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix(review): clarify insert-only suggestions

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix(review): keep anchors consistent across phases

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* refactor(review): slim suggestion rules

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* update 250 to 100

* Add validator_model input to allow separate model for validator pass

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Add strict deduplication rules to validator prompt

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat(review): make suggestion blocks configurable via include_suggestions parameter

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix(review): remove redundant anchor rejection rule from validator

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix(review): remove validator_model override

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat: load review methodology from factory-mono builtin skill

Load the review skill's shared methodology from factory-mono's
builtin-skills/review/SKILL.md instead of keeping it inline in the
CI prompt templates. The skill is loaded at runtime via local plugin
cache or GitHub fallback.

The shared methodology (bug patterns, reporting gate, confidence
calibration, deduplication, analysis discipline) is extracted via
BEGIN_SHARED_METHODOLOGY / END_SHARED_METHODOLOGY markers and injected
into both candidate and validator prompts. Suggestion block rules
remain controlled by the include_suggestions toggle at the CI template
level.

Depends on: Factory-AI/factory-mono#11498

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat: use CLI builtin review skill instead of fetching from GitHub

The Droid CLI bundles the review skill as a builtin, so the agent can
invoke it via the Skill tool at runtime. Remove the load-skill.ts
GitHub fetch and instead instruct the agent to invoke the 'review'
skill directly.

This eliminates the network dependency during the prepare step and
uses the CLI as the single source of truth for the review methodology.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* refactor: make templates thin wrappers, delegate structure to review skill

Both candidate and validator prompts now only provide:
- Session framing (senior engineer / validating candidates)
- PR context and file paths
- Skill invocation instruction (Pass 1 or Pass 2)
- CI-specific output schema and posting constraints

All review methodology, triage, parallel review, validation rules,
deduplication, and confidence filtering come from the skill.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat: add path_to_droid_executable input to review action

Allows using a custom CLI build for testing builtin skills from
unreleased branches.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* revert: remove path_to_droid_executable from review action

Not needed for this PR. Can be added separately if custom CLI
testing is needed in the future.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* Merge pull request #66 from Factory-AI/feat/review-depth-shallow-deep

feat: add shallow/deep review depth presets (default: shallow with kimi-k2-0711)

* security review plugins

* security review plugins

* STRIDE enforcement

* fix: create prompt file when both automatic review flags are set

When both automatic_review and automatic_security_review were true,
prepareTagExecution returned early without calling prepareReviewMode,
so no prompt file was written. The Droid Exec step then failed with
'Prompt file does not exist'.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat: move security review to skill + run concurrently with code review (#71)

* feat: move security review to skill + run concurrently with code review

- Create security-reviewer custom droid that invokes the security-review
  skill and returns findings as a JSON array (same format as
  file-group-reviewer)
- Simplify security-review-prompt.ts to a candidates prompt that
  references the security-review skill instead of inlining all STRIDE
  methodology
- When both automatic_review and automatic_security_review are enabled,
  spawn security-reviewer as a Task subagent alongside file-group-reviewers
  during pass 1, merging all candidates into one JSON for the validator
- Standalone @droid security now uses the two-pass pipeline (candidates +
  validator) instead of posting inline comments directly
- Update security/action.yml with validator steps
- Enable Task/Skill/FetchUrl tools for security review candidate generation

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* test: point droid-review workflow at branch for CI testing

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* feat: use [security] tag instead of STRIDE letters, add security badge to tracking comment

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix: show correct tracking comment when running both code review and security review

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

---------

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix: bump internal CI workflow from @V3 to @dev (#74)

The internal droid-review.yml workflow passes automatic_security_review: true,
but @V3 does not support that input. The @V3 prepare step enters the combined
review code path which returns early without generating a prompt file, causing
the base-action to fail with 'Prompt file does not exist'.

Pointing at @dev picks up the concurrent security review support. This only
affects CI for this repo -- customers continue using @V3 in their own workflows.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* chore: remove file-group-reviewer droid, update version refs to v5 (#76)

* eval: remove file-group-reviewer droid in favor of SKILL.md inlined methodology

The review skill now uses worker subagents with the full shared
methodology inlined into each prompt. The file-group-reviewer.md
droid and its tests are no longer needed.

* chore: update version refs to v5, clean up file-group-reviewer references

- Update all @V3 references to @v5 in README and droid.yml workflow
- Remove file-group-reviewer mentions from prompts, comments, and droid descriptions

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

---------

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

---------

Co-authored-by: Zahid Khawaja <zahidkhawaja@icloud.com>
Co-authored-by: Varin Nair <varin@factory.ai>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: Shashank Sharma <shashank@factory.ai>
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.

2 participants