Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions docs/architecture/deep-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,10 @@ The backend does not resolve the review target or build the launch manifest. The

`src/crates/assembly/core/src/agentic/agents/definitions/review/review_specialists.rs` defines read-only reviewer agents:

- `ReviewBusinessLogic`
- `ReviewGeneral` (internal managed-batch worker)
- `ReviewPerformance`
- `ReviewSecurity`
- `ReviewArchitecture`
- `ReviewFrontend`
- `ReviewWorker`
- `ReviewJudge`

These agents form an optional specialist pool, not mandatory coverage lanes. A new strict run may launch at most one specialist for a concrete uncertainty. The existing generic Git exposure remains for legacy compatibility, but it is not authorized as prepared changed-code evidence. Prepared `GetFileDiff` is the source of truth for changed code; when the local binding is `matching_clean`, existing Read/Grep/Glob/LS tools may supplement it with repository context. `ReviewJudge` is a conditional quality check used only for a high-severity finding, conflicting evidence, or a materially low-confidence conclusion; it does not perform a full independent review pass.
`ReviewWorker` is an optional capability, not a fixed domain lane. The owning `DeepReview` agent selects a concrete lens from the actual change or the user's requested focus, then supplies the exact question, scope, and evidence expectation in the launch prompt. A new strict run may launch at most one such worker for a concrete uncertainty. The retired `ReviewBusinessLogic`, `ReviewPerformance`, `ReviewSecurity`, `ReviewArchitecture`, `ReviewFrontend`, and `ReviewGeneral` ids remain non-discoverable compatibility aliases for stored configuration, historical manifests, and their direct task invocations; they resolve to `ReviewWorker` under the same DeepReview visibility, manifest, read-only, and budget gates, but are not registered or emitted for new runs. The existing generic Git exposure remains for legacy compatibility, but it is not authorized as prepared changed-code evidence. Prepared `GetFileDiff` is the source of truth for changed code; when the local binding is `matching_clean`, existing Read/Grep/Glob/LS tools may supplement it with repository context. `ReviewJudge` is a conditional quality check used only for a high-severity finding, conflicting evidence, or a materially low-confidence conclusion; it does not perform a full independent review pass.

`ReviewFixer` is the separate writable remediation identity. DeepReview runtime policy rejects it during review execution. The frontend action surface invokes it only after user approval, and a new read-only Review run checks the fix when requested.

Expand Down Expand Up @@ -83,6 +78,8 @@ An explicit, complete Git range with a matching clean workspace or a provider PR

Prepared Review target evidence uses bounded `GetFileDiff` pages as changed-code evidence. Local ranges read exact Git revisions; PR targets read provider diffs on demand and revalidate base/head before each file. The parent Review has a 240,000-character aggregate allowance and admits at most 128 provider diff acquisitions before provider I/O; one acquisition normally performs one file-page request and one detail request. Repeating the same page for the same reviewer returns a compact already-served result instead of the diff again. Exhaustion and stale target bindings return structured limited evidence. Existing generic Git exposure remains for legacy compatibility but does not authorize ref guessing or scope widening; Read/Grep/Glob/LS are supplemental only for a matching clean Git-range binding, never for a provider-only PR target.

Local Review `Read` calls also keep a session-scoped, metadata-only receipt of returned line ranges keyed by logical path, nanosecond mtime, byte length, and a streamed SHA-256 content digest. A fully covered repeat on the unchanged revision returns a compact already-served result; changed files, remote workspaces, tail reads, partial overlaps, and non-Review agents continue through the normal read path. Digest work is restricted to receipt-enabled Review agents. Replacing or compacting model context clears these receipts so the runtime never suppresses content that is no longer present in context.

Deleted, renamed, binary, oversized, conflicted, or unavailable files remain visible as coverage facts. The PR panel is the only built-in PR Review entry and associates progress/results by provider repository, PR id, and immutable revisions. Cached overview data is display-only until the selected PR is revalidated; revision or runtime-evidence changes make prior results stale, and failed or unavailable results remain distinct from limited coverage. The implementation does not add automatic checkout, reviewer command execution, speculative cache plans, automatic Review, inline comments, approval, merge, or automatic publishing.

## Strict Review Delegation Policy
Expand Down Expand Up @@ -115,7 +112,7 @@ For new strict launches:

For managed large L1 launches:

- `workPackets` contains only deterministic `ReviewGeneral` file batches;
- `workPackets` contains only deterministic `ReviewWorker` file batches;
- packet calls are foreground-waited and may never be converted to background `Task` calls;
- `managedReviewPlan` records total, planned, and deferred file counts plus batch, concurrency, and timeout bounds;
- the final report must mark deferred, provider-omitted, timed-out, or unavailable scope as limited coverage;
Expand All @@ -133,13 +130,13 @@ Review launches start directly without routine confirmation. Exceptional states

## Managed Work Packets and Historical Compatibility

New strict reviews do not generate work packets or module-aware reviewer shards. New managed large L1 reviews generate only bounded `ReviewGeneral` packets. Stored manifests may also contain historical reviewer/judge packets, launch batches, packet ids, assigned scopes, and retry metadata. Runtime parsing, report enrichment, recovery UI, and target-evidence validation distinguish the new managed plan from historical manifests.
New strict reviews do not generate work packets or module-aware reviewer shards. New managed large L1 reviews generate only bounded `ReviewWorker` packets. Stored manifests may also contain historical fixed reviewer ids, reviewer/judge packets, launch batches, packet ids, assigned scopes, and retry metadata. Runtime parsing, report enrichment, recovery UI, and target-evidence validation distinguish the new managed plan from historical manifests.

Packet support is not a general fan-out policy. New packets are admitted only when `managedReviewPlan` is present; strict specialist policy remains unchanged. Packet-specific queue and retry behavior applies only when the prepared manifest actually contains those packets.

## Backend Policy and Admission

`DeepReviewExecutionPolicy` parses runtime policy and the per-turn specialist-call ceiling. `DeepReviewRunManifestGate` admits specialist-pool members, the optional `ReviewJudge`, and `ReviewGeneral` only when it is named by a prepared managed packet. It rejects `ReviewFixer`, nested `DeepReview`, skipped members, and unconfigured agents.
`DeepReviewExecutionPolicy` parses runtime policy and the per-turn specialist-call ceiling. `DeepReviewRunManifestGate` admits the dynamic `ReviewWorker`, explicitly configured custom specialists, and the optional `ReviewJudge`; worker packets require a prepared bounded managed plan. It rejects `ReviewFixer`, nested `DeepReview`, skipped members, and unconfigured agents.

`DeepReviewBudgetTracker` separately permits at most one initial specialist and one Judge call for a new strict turn. This keeps the safety boundary deterministic without hard-coding which domain deserves delegation.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ BitFun 后续不应把 dynamic workflow 理解成一个需要用户学习的新

用户可以用自然语言表达“更快”“更稳”或“只看安全”等关注点。当前严格审查只识别 `/review strict`、历史 `/DeepReview` alias 和内部显式 strict follow-up;自然语言 strict 映射若未来接入,必须仍由用户明确表达严格意图,不能由风险启发式规则代替。

上述渐进升级适用于批量执行、失败队列和验证策略。普通 Review 不因风险启发式增加专家 reviewer;仅当目标超过单 reviewer 边界或 provider 证据不完整时,才自动启用有界 `ReviewGeneral` 工作包。团队策略当前只能提示严格审查,不能自动启动。
上述渐进升级适用于批量执行、失败队列和验证策略。普通 Review 不因风险启发式增加专家 reviewer;仅当目标超过单 reviewer 边界或 provider 证据不完整时,才自动启用有界 `ReviewWorker` 工作包。严格主审按实际变更或用户指定关注点动态生成 worker 的具体 lens、问题和范围,不再通过固定 reviewer 身份表达审核维度。团队策略当前只能提示严格审查,不能自动启动。

### 4.2 DeepReview 收敛为显式 Strict Review

Expand All @@ -82,7 +82,7 @@ BitFun 后续不应把 dynamic workflow 理解成一个需要用户学习的新
| L1 | 普通 `Review`;小目标单 reviewer,大目标内部受管分批 | 一个聚合后的问题清单、证据状态、实际覆盖和残余风险 |
| L3 | `/review strict`、历史 `/DeepReview` alias 或内部显式 strict follow-up | 一个严格主审直接检查;必要时最多一个专家和一个条件质量检查;无需例行启动确认 |

L2 只保留历史 manifest 的读取与运行时校验兼容,不产生新的 Review 启动。安全、性能、架构、前端体验、跨模块或验证缺口等信号交给主审决定调查重点,不自动增加专家 reviewer。新 Strict Review 不预生成 work packets、不做同角色文件分片、不默认运行 Judge。普通 L1 仅在目标超过单 reviewer 边界或 provider 证据不完整时生成受时长、批次数和并发约束的 `ReviewGeneral` 工作包;所有 worker 都由所属 Review 回合前台等待并聚合,未纳入本轮预算的范围必须标为 deferred coverage。
L2 只保留历史 manifest 的读取与运行时校验兼容,不产生新的 Review 启动。安全、性能、架构、前端体验、跨模块或验证缺口等信号交给主审决定调查重点,不自动增加专家 reviewer。新 Strict Review 不预生成 work packets、不做同角色文件分片、不默认运行 Judge。普通 L1 仅在目标超过单 reviewer 边界或 provider 证据不完整时生成受时长、批次数和并发约束的 `ReviewWorker` 工作包;所有 worker 都由所属 Review 回合前台等待并聚合,未纳入本轮预算的范围必须标为 deferred coverage。固定的业务逻辑、性能、安全、架构和前端 reviewer id 仅作为历史配置与会话兼容别名保留,不再进入新 manifest 或 Agent 列表

迁移/兼容规则:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ You do NOT make code changes. You produce a **Security Posture Report** with con
When this skill is invoked by BitFun Team Mode, this skill supplies the security-review lens. Use existing Task sub-agents for independent security evidence gathering, then make final severity and remediation calls in the main Team session.

- Do not assume a CSO sub-agent exists. Choose only from the Task tool's available agents.
- Prefer a matching custom security sub-agent if available; otherwise use `ReviewSecurity` for diff-focused review when available, `Explore` for broader code/config mapping, and `FileFinder` for security-sensitive files.
- Prefer a matching custom security sub-agent if available; otherwise use one `CodeReview` task with an exact security lens for diff-focused review, `Explore` for broader code/config mapping, and `FileFinder` for security-sensitive files.
- Keep Task work read-only. Ask for concrete evidence: file paths, trust boundaries, inputs, auth/data flows, exploit preconditions, and confidence.
- In parallel batches, return a compact Security brief: `critical/high findings`, `trust-boundary risks`, `false-positive notes`, `required fixes`, `verification`.
- The main Team orchestrator decides what blocks Build/Ship and asks the user for risk acceptance when needed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ You are running the specialized pre-landing workflow. Analyze the current branch
When this skill is invoked by BitFun Team Mode, this skill supplies the pre-landing review lens. Use existing Task sub-agents for independent diff review tracks, then consolidate findings in the main Team session.

- Do not assume a Staff Engineer sub-agent exists. Choose only from the Task tool's available agents.
- Prefer built-in review sub-agents when available: `ReviewBusinessLogic` for correctness, `ReviewPerformance` for hot paths, `ReviewSecurity` for security-sensitive diff, and `ReviewJudge` for evidence/quality inspection after reviewers return.
- Prefer matching custom review sub-agents over generic ones. Use `Explore` only for broad read-only investigation when specialist reviewers are unavailable.
- Use at most one built-in `CodeReview` sub-agent for an independent pass, and put the exact correctness, performance, security, or architecture question in its prompt. Broader dynamic lens selection belongs to the unified `/review` path.
- Prefer a matching custom review sub-agent when the user configured one. Use `Explore` only for broad read-only investigation when no review sub-agent fits.
- Keep Task work read-only. Ask for tight findings with file paths, line references if possible, severity, confidence, and why tests might miss it.
- The main Team orchestrator owns final severity ordering, AUTO-FIX vs ASK classification, and any code changes.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,4 @@ mod review_fixer;
mod review_specialists;

pub use review_fixer::ReviewFixerAgent;
pub use review_specialists::{
ArchitectureReviewerAgent, BusinessLogicReviewerAgent, FrontendReviewerAgent,
GeneralReviewerAgent, PerformanceReviewerAgent, ReviewJudgeAgent, SecurityReviewerAgent,
};
pub use review_specialists::{ReviewJudgeAgent, ReviewWorkerAgent};
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
use crate::agentic::agents::AgentToolPolicyOverrides;
use crate::agentic::deep_review_policy::{
REVIEWER_ARCHITECTURE_AGENT_TYPE, REVIEWER_BUSINESS_LOGIC_AGENT_TYPE,
REVIEWER_FRONTEND_AGENT_TYPE, REVIEWER_PERFORMANCE_AGENT_TYPE, REVIEWER_SECURITY_AGENT_TYPE,
REVIEW_JUDGE_AGENT_TYPE,
};
use crate::agentic::deep_review_policy::{REVIEW_JUDGE_AGENT_TYPE, REVIEW_WORKER_AGENT_TYPE};
use crate::agentic::tools::framework::ToolExposure;
use crate::define_readonly_subagent_with_overrides;

Expand All @@ -14,61 +10,11 @@ fn reviewer_tool_exposure_overrides() -> AgentToolPolicyOverrides {
}

define_readonly_subagent_with_overrides!(
GeneralReviewerAgent,
"ReviewGeneral",
"General Review Worker",
r#"Read-only general review worker for one bounded managed-Review shard. It checks correctness, security, performance, architecture, frontend contracts, and tests within only the assigned files, then returns evidence and exact coverage to the owning Review agent."#,
"review_general_agent",
&["Read", "Grep", "Glob", "LS", "GetFileDiff"],
reviewer_tool_exposure_overrides()
);

define_readonly_subagent_with_overrides!(
BusinessLogicReviewerAgent,
REVIEWER_BUSINESS_LOGIC_AGENT_TYPE,
"Business Logic Reviewer",
r#"Independent read-only reviewer focused on workflow correctness, business rules, state transitions, data integrity, and edge-case handling in the review target. Use this when you need a fresh perspective on whether the change still does the right thing for real users."#,
"review_business_logic_agent",
&["Read", "Grep", "Glob", "LS", "GetFileDiff"],
reviewer_tool_exposure_overrides()
);

define_readonly_subagent_with_overrides!(
PerformanceReviewerAgent,
REVIEWER_PERFORMANCE_AGENT_TYPE,
"Performance Reviewer",
r#"Independent read-only reviewer focused on latency, hot-path efficiency, unnecessary allocations, N+1 patterns, blocking calls, over-fetching, and scale-sensitive regressions introduced by the review target."#,
"review_performance_agent",
&["Read", "Grep", "Glob", "LS", "GetFileDiff"],
reviewer_tool_exposure_overrides()
);

define_readonly_subagent_with_overrides!(
SecurityReviewerAgent,
REVIEWER_SECURITY_AGENT_TYPE,
"Security Reviewer",
r#"Independent read-only reviewer focused on security risks such as injection, auth gaps, data exposure, unsafe command/file handling, privilege escalation, and trust-boundary mistakes in the review target."#,
"review_security_agent",
&["Read", "Grep", "Glob", "LS", "GetFileDiff"],
reviewer_tool_exposure_overrides()
);

define_readonly_subagent_with_overrides!(
ArchitectureReviewerAgent,
REVIEWER_ARCHITECTURE_AGENT_TYPE,
"Architecture Reviewer",
r#"Independent read-only reviewer focused on structural and architectural issues such as module boundary violations, API contract design, abstraction integrity, dependency direction, and cross-cutting concern impact in the review target."#,
"review_architecture_agent",
&["Read", "Grep", "Glob", "LS", "GetFileDiff"],
reviewer_tool_exposure_overrides()
);

define_readonly_subagent_with_overrides!(
FrontendReviewerAgent,
REVIEWER_FRONTEND_AGENT_TYPE,
"Frontend Reviewer",
r#"Independent read-only reviewer focused on frontend-specific issues such as i18n key synchronization, frontend performance patterns (e.g., memoization, virtualization, effect/reactivity dependencies), accessibility, state management, frontend-backend API contract alignment, and platform boundary compliance in the review target."#,
"review_frontend_agent",
ReviewWorkerAgent,
REVIEW_WORKER_AGENT_TYPE,
"Dynamic Review Worker",
r#"Read-only Review worker for one bounded assignment. The owning Review agent supplies the concrete lens, question, scope, and evidence limits at launch time; this worker never selects its own broader role or target."#,
"review_worker_agent",
&["Read", "Grep", "Glob", "LS", "GetFileDiff"],
reviewer_tool_exposure_overrides()
);
Expand All @@ -85,21 +31,13 @@ define_readonly_subagent_with_overrides!(

#[cfg(test)]
mod tests {
use super::{
ArchitectureReviewerAgent, BusinessLogicReviewerAgent, FrontendReviewerAgent,
GeneralReviewerAgent, PerformanceReviewerAgent, ReviewJudgeAgent, SecurityReviewerAgent,
};
use super::{ReviewJudgeAgent, ReviewWorkerAgent};
use crate::agentic::agents::{Agent, UserContextPolicy};

#[test]
fn specialist_reviewers_use_workspace_context_and_instructions() {
let agents: Vec<Box<dyn Agent>> = vec![
Box::new(BusinessLogicReviewerAgent::new()),
Box::new(GeneralReviewerAgent::new()),
Box::new(PerformanceReviewerAgent::new()),
Box::new(SecurityReviewerAgent::new()),
Box::new(ArchitectureReviewerAgent::new()),
Box::new(FrontendReviewerAgent::new()),
Box::new(ReviewWorkerAgent::new()),
Box::new(ReviewJudgeAgent::new()),
];

Expand Down
6 changes: 1 addition & 5 deletions src/crates/assembly/core/src/agentic/agents/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ pub use definitions::modes::{
AgenticMode, ClawMode, CoworkMode, DebugMode, DeepResearchMode, MultitaskMode, PlanMode,
TeamMode,
};
pub use definitions::review::{
ArchitectureReviewerAgent, BusinessLogicReviewerAgent, FrontendReviewerAgent,
GeneralReviewerAgent, PerformanceReviewerAgent, ReviewFixerAgent, ReviewJudgeAgent,
SecurityReviewerAgent,
};
pub use definitions::review::{ReviewFixerAgent, ReviewJudgeAgent, ReviewWorkerAgent};
pub use definitions::shared::ReadonlySubagent;
pub use definitions::subagents::{
ComputerUseMode, ExploreAgent, FileFinderAgent, GeneralPurposeAgent, ResearchSpecialistAgent,
Expand Down
Loading