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
Original file line number Diff line number Diff line change
Expand Up @@ -121,26 +121,26 @@ pub fn default_review_team_definition() -> ReviewTeamDefinition {
role(
"worker",
REVIEW_WORKER_AGENT_TYPE,
"Review Worker",
"Dynamic Review Worker",
"A read-only worker whose concrete lens, question, and scope are selected for the current change instead of being fixed in the agent identity.",
"Focused Review",
"On-demand Review Check",
"A read-only check whose focus and scope are chosen for the current change when more evidence would be useful.",
&[
"Apply only the lens and question supplied by the owning Review agent.",
"Stay within the prepared target and return evidence-backed findings and exact coverage.",
"Do not widen permissions, modify files, or repeat the primary review.",
"Check only the question assigned by the main review.",
"Stay within the selected scope and support conclusions with concrete evidence.",
"Do not modify files or repeat work already completed by the main review.",
],
"#3b82f6",
),
role(
"judge",
REVIEW_JUDGE_AGENT_TYPE,
"Review Arbiter",
"Review Quality Inspector",
"An independent arbiter used only for high-severity, conflicting, or materially low-confidence conclusions.",
"Independent Review Check",
"Review Quality Check",
"A read-only independent check used only when a serious finding, conflicting evidence, or an uncertain conclusion needs validation.",
&[
"Validate or reject disputed findings against concrete evidence.",
"Spot-check only the claims that need independent verification.",
"Ensure every surviving issue has a safe actionable response.",
"Confirm or reject disputed findings using concrete evidence.",
"Check only the claims that need independent validation.",
"Make sure each retained issue has a safe, practical next step.",
],
"#8b5cf6",
),
Expand All @@ -152,7 +152,7 @@ pub fn default_review_team_definition() -> ReviewTeamDefinition {
strategy_profile(
"quick",
"Quick",
"Quick keeps the primary review concise and allows only a narrowly justified worker lens.",
"Quick keeps the main review concise and allows narrowly focused extra checks only when justified.",
"0.4-0.6x",
"0.5-0.7x",
"fast",
Expand All @@ -166,7 +166,7 @@ pub fn default_review_team_definition() -> ReviewTeamDefinition {
strategy_profile(
"normal",
"Normal",
"Normal balances evidence depth with one optional dynamically selected specialist lens.",
"Normal balances evidence depth with optional independent checks selected for the current change.",
"1x",
"1x",
"fast",
Expand All @@ -180,7 +180,7 @@ pub fn default_review_team_definition() -> ReviewTeamDefinition {
strategy_profile(
"deep",
"Deep",
"Deep gives the primary reviewer and one justified dynamic lens the longest bounded budget.",
"Deep gives the main review and any justified independent checks the longest bounded budget.",
"1.8-2.5x",
"1.5-2.5x",
"primary",
Expand Down Expand Up @@ -208,7 +208,7 @@ pub fn default_review_team_definition() -> ReviewTeamDefinition {
ReviewTeamDefinition {
id: "default-review-team".to_string(),
name: "Code Review".to_string(),
description: "One primary review with an optional dynamically scoped worker and conditional quality inspection.".to_string(),
description: "One main review that can request focused independent checks when more evidence is needed.".to_string(),
warning: "Strict review may take longer and usually consumes more tokens than a standard review.".to_string(),
default_model: "fast".to_string(),
default_strategy_level: "normal".to_string(),
Expand Down Expand Up @@ -250,6 +250,40 @@ mod tests {
.all(|profile| profile.role_directives.len() == 2));
}

#[test]
fn default_team_uses_readable_user_facing_copy() {
let definition = default_review_team_definition();
let worker = &definition.core_roles[0];
let judge = &definition.core_roles[1];

assert_eq!(worker.fun_name, "Focused Review");
assert_eq!(worker.role_name, "On-demand Review Check");
assert_eq!(judge.fun_name, "Independent Review Check");
assert_eq!(judge.role_name, "Review Quality Check");
assert_eq!(
definition.description,
"One main review that can request focused independent checks when more evidence is needed."
);

let user_facing_copy = definition
.strategy_profiles
.values()
.map(|profile| profile.summary.as_str())
.chain([worker.description.as_str(), judge.description.as_str()])
.collect::<Vec<_>>()
.join("\n")
.to_ascii_lowercase();
for implementation_term in ["worker", "lens", "specialist", "inspector"] {
assert!(
!user_facing_copy.contains(implementation_term),
"user-facing copy should not contain {implementation_term}"
);
}
assert!(!user_facing_copy.contains("one optional"));
assert!(!user_facing_copy.contains("one justified"));
assert!(!user_facing_copy.contains("one narrowly focused"));
}

#[test]
fn serialized_default_team_keeps_the_frontend_fallback_contract() {
let value = serde_json::to_value(default_review_team_definition())
Expand All @@ -258,7 +292,7 @@ mod tests {
assert_eq!(value["name"], "Code Review");
assert_eq!(
value["description"],
"One primary review with an optional dynamically scoped worker and conditional quality inspection."
"One main review that can request focused independent checks when more evidence is needed."
);
assert_eq!(value["coreRoles"][0]["subagentId"], "ReviewWorker");
assert_eq!(value["coreRoles"][0]["accentColor"], "#3b82f6");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,17 @@ describeWithJsdom('DeepReviewConsentDialog', () => {
expect(container.textContent).toContain('BitFun selected the most relevant checks for this target.');
expect(container.textContent).not.toContain('Estimated reviewer prompt input');
expect(container.textContent).not.toContain('Reviewer prompt input only');
expect(container.textContent).toContain('Independent checks: 3 planned review agent run');
expect(container.textContent).toContain('Up to 4 review agent runs may occur without another confirmation.');
expect(container.textContent).toContain(
'Maximum checks: 4. Actual work depends on the review target and the evidence found.',
);
expect(container.textContent).not.toContain('Expected checks:');
expect(container.textContent).not.toContain('up to 4 initial calls');
expect(container.textContent).toContain('Run strategy: Standard');
expect(container.textContent).not.toContain('Do not show this again');
expect(container.textContent).not.toContain('Risk areas: Backend core');
expect(container.textContent).toContain('Planned review agent runs; model requests and token use are not estimated here.');
expect(container.textContent).toContain(
'The review budget allows optional independent checks. Actual requests and token use depend on the evidence found.',
);
expect(container.textContent).not.toContain('1 extra specialist');
expect(container.textContent).not.toContain('Review depth: Risk-expanded');
expect(container.textContent).not.toContain('Frontend reviewer');
Expand All @@ -316,6 +320,31 @@ describeWithJsdom('DeepReviewConsentDialog', () => {
expect(container.textContent).not.toContain('Custom security reviewer');
});

it('keeps a single-check limit grammatically readable', async () => {
const result = vi.fn();
const basePreview = buildPreviewWithoutSkippedReviewers();
const preview: ReviewTeamRunManifest = {
...basePreview,
tokenBudget: {
...basePreview.tokenBudget,
estimatedReviewerCalls: 1,
maxReviewerCalls: 1,
},
};

await act(async () => {
root.render(<Harness preview={preview} onResult={result} />);
});
await act(async () => {
container.querySelector('button')?.dispatchEvent(new window.Event('click', { bubbles: true }));
});

expect(container.textContent).toContain(
'Maximum checks: 1. Actual work depends on the review target and the evidence found.',
);
expect(container.textContent).not.toContain('1 checks');
});

it('uses a generic target summary when the review is not file-based', async () => {
const result = vi.fn();
const preview: ReviewTeamRunManifest = {
Expand Down Expand Up @@ -344,7 +373,9 @@ describeWithJsdom('DeepReviewConsentDialog', () => {
expect(container.textContent).toContain('Provided context');
expect(container.textContent).not.toContain('0 files');
expect(container.textContent).not.toContain('Risk areas:');
expect(container.textContent).toContain('Planned review agent runs; model requests and token use are not estimated here.');
expect(container.textContent).toContain(
'The review budget allows optional independent checks. Actual requests and token use depend on the evidence found.',
);
});

it('still opens when skip preference is set but reviewers are skipped', async () => {
Expand Down Expand Up @@ -426,15 +457,19 @@ describeWithJsdom('DeepReviewConsentDialog', () => {
expect(container.querySelectorAll('.deep-review-consent__strategy-heading')).toHaveLength(0);
expect(container.textContent).not.toContain('Quick is narrower');
expect(container.textContent).not.toContain('Risk areas: Backend core');
expect(container.textContent).toContain('Planned review agent runs; model requests and token use are not estimated here.');
expect(container.textContent).toContain(
'The review budget allows optional independent checks. Actual requests and token use depend on the evidence found.',
);
expect(container.textContent).not.toContain('1 extra specialist');
expect(container.textContent).not.toContain('Expected cost:');
expect(container.querySelectorAll('.deep-review-consent__strategy-selected-summary')).toHaveLength(0);
expect(container.querySelectorAll('.deep-review-consent__strategy-current')).toHaveLength(1);
expect(container.querySelectorAll('.deep-review-consent__strategy-option')).toHaveLength(0);
expect(container.querySelectorAll('.deep-review-consent__strategy-option--active')).toHaveLength(0);
expect(container.textContent).not.toContain('Team default');
expect(container.textContent).toContain('Standard adds independent coverage while keeping cost practical.');
expect(container.textContent).toContain(
'Standard review examines the selected target in more depth and may add independent checks when useful.',
);
expect(container.querySelectorAll('.deep-review-consent__strategy-option-summary')).toHaveLength(0);

const quickStrategyButton = Array.from(container.querySelectorAll('button'))
Expand Down
26 changes: 9 additions & 17 deletions src/web-ui/src/flow_chat/components/DeepReviewConsentDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@ export interface DeepReviewConsentControls {
deepReviewConsentDialog: React.ReactNode;
}

function getInitialReviewCallFacts(preview: ReviewTeamRunManifest): {
planned: number;
maximum: number;
} {
const planned = Math.max(1, preview.tokenBudget.estimatedReviewerCalls || 1);
return {
planned,
maximum: Math.max(planned, preview.tokenBudget.maxReviewerCalls || planned),
};
function getReviewCallLimit(preview: ReviewTeamRunManifest): number {
return Math.max(
1,
preview.tokenBudget.estimatedReviewerCalls || 1,
preview.tokenBudget.maxReviewerCalls || 1,
);
}

function getReviewTargetFileCount(preview: ReviewTeamRunManifest): number {
Expand Down Expand Up @@ -123,7 +120,7 @@ export function useDeepReviewConsent(): DeepReviewConsentControls {
const skippedCount = skippedReviewers.length;
const selectedStrategyLabel = getStrategyLabel(preview.strategyLevel, t);
const targetSummary = getReviewTargetSummary(preview, t);
const callFacts = getInitialReviewCallFacts(preview);
const reviewCallLimit = getReviewCallLimit(preview);
return (
<div className="deep-review-consent__summary">
<div className="deep-review-consent__summary-header">
Expand Down Expand Up @@ -159,15 +156,10 @@ export function useDeepReviewConsent(): DeepReviewConsentControls {

<div className="deep-review-consent__token-estimate">
<strong>
{t('deepReviewConsent.initialCalls', {
planned: callFacts.planned,
{t('deepReviewConsent.callLimit', {
count: reviewCallLimit,
})}
</strong>
<span>
{t('deepReviewConsent.parallelCalls', {
count: callFacts.maximum,
})}
</span>
</div>

{preview.workspacePath && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ describe('formatReviewCoverageSource', () => {
it('maps known read-only review roles to user-facing labels', () => {
expect(formatReviewCoverageSource('ReviewSecurity')).toBe('Security coverage');
expect(formatReviewCoverageSource('ReviewJudge')).toBe('Quality check');
expect(formatReviewCoverageSource('Review Quality Check')).toBe('Quality check');
});

it('does not hide Review-prefixed remediation sources', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const REVIEW_SOURCE_ALIASES: Record<string, ReviewCoverageSourceLabelKey> = {
reviewjudge: 'qualityGate',
reviewarbiter: 'qualityGate',
reviewqualityinspector: 'qualityGate',
reviewqualitycheck: 'qualityGate',
qualityinspector: 'qualityGate',
};

Expand Down
11 changes: 9 additions & 2 deletions src/web-ui/src/flow_chat/tool-cards/TaskToolDisplay.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ vi.mock('react-i18next', () => {
if (key === 'toolCards.taskTool.defaultAgentKind') {
return 'Sub-agent';
}
if (key === 'toolCards.taskTool.reviewCoverageLabel') {
return 'Review check';
}
if (key === 'toolCards.taskTool.reviewCoverageDescription') {
return 'Checking review coverage';
}
if (key === 'toolCards.taskTool.cancelSession') {
return `Cancel session: ${options?.sessionId}`;
}
Expand Down Expand Up @@ -396,7 +402,7 @@ describeWithJsdom('TaskToolDisplay', () => {
id: 'launch-review-call-1',
input: {
packet_id: 'managed-review:batch-1-of-4',
description: '[packet managed-review:batch-1-of-4] Review web UI changes',
description: '[packet managed-review:batch-1-of-4] Review batch 1',
prompt: 'Internal worker prompt',
subagent_type: 'ReviewGeneral',
},
Expand All @@ -409,7 +415,8 @@ describeWithJsdom('TaskToolDisplay', () => {
);
});

expect(container.textContent).toContain('Review web UI changes');
expect(container.textContent).toContain('Checking review coverage');
expect(container.textContent).not.toContain('Review batch 1');
expect(container.textContent).not.toContain('LaunchReviewAgent');
expect(container.textContent).not.toContain('ReviewGeneral');
expect(container.textContent).not.toContain('managed-review:batch-1-of-4');
Expand Down
7 changes: 5 additions & 2 deletions src/web-ui/src/flow_chat/tool-cards/TaskToolDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,11 @@ export const TaskToolDisplay: React.FC<ToolCardProps> = ({
readStringValue(toolCall.input.modelId);

if (isReviewCoverageTask) {
const reviewDescription = readStringValue(description)
.replace(/^\[packet\s+[^\]]+\]\s*/i, '');
const packetId = readStringValue(toolCall.input.packet_id)
|| readStringValue(toolCall.input.packetId);
const reviewDescription = /^managed-review:/i.test(packetId)
? ''
: readStringValue(description).replace(/^\[packet\s+[^\]]+\]\s*/i, '');
return {
description: reviewDescription || t('toolCards.taskTool.reviewCoverageDescription'),
prompt: 'Not provided',
Expand Down
15 changes: 7 additions & 8 deletions src/web-ui/src/locales/en-US/flow-chat.json
Original file line number Diff line number Diff line change
Expand Up @@ -1041,20 +1041,19 @@
"deepReviewConsent": {
"windowTitle": "Review plan",
"eyebrow": "Review plan",
"title": "Add broader review coverage?",
"body": "BitFun selected additional independent checks for this change. Review the target scope, planned checks, runtime tendency, and read-only boundary before continuing.",
"title": "Start this review?",
"body": "BitFun reviews this change directly and may add focused independent checks only when the evidence needs them. Confirm the target, review budget, expected time, and read-only boundary before continuing.",
"readonlyLabel": "Read-only",
"readonly": "Reviewers do not modify files.",
"sessionConcurrencyTitle": "Active session is busy",
"sessionConcurrencyBody": "The target session already has {{count}} review tasks running. Continuing will share the available review capacity.",
"costLabel": "Coverage",
"cost": "Planned review agent runs; model requests and token use are not estimated here.",
"costLabel": "Review budget",
"cost": "The review budget allows optional independent checks. Actual requests and token use depend on the evidence found.",
"timeLabel": "Time",
"time": "Runs in background and may take longer.",
"cancel": "Cancel",
"confirm": "Start review",
"initialCalls": "Independent checks: {{planned}} planned review agent run",
"parallelCalls": "Up to {{count}} review agent runs may occur without another confirmation.",
"callLimit": "Maximum checks: {{count}}. Actual work depends on the review target and the evidence found.",
"runStrategy": "Run strategy: {{strategy}}",
"strategyLabels": {
"quick": "Focused",
Expand All @@ -1063,8 +1062,8 @@
},
"strategySummaries": {
"quick": "Focused review checks the most relevant risks for the selected target.",
"normal": "Standard adds independent coverage while keeping cost practical.",
"deep": "Extensive review adds the broadest applicable coverage for high-risk changes."
"normal": "Standard review examines the selected target in more depth and may add independent checks when useful.",
"deep": "Extensive review examines all applicable high-risk areas and may add independent validation when the evidence needs it."
},
"summaryTitle": "Review plan",
"targetFiles": "{{count}} files",
Expand Down
Loading