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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ coverage/
results/
findings-output/

# Live integration tests (contain org-specific config)
tests/integration/test_e2e_review_then_verify.py
tests/integration/test_fix_verification.py

# Python
__pycache__/
*.pyc
Expand Down
3 changes: 2 additions & 1 deletion entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ python3 /app/src/run_agent.py \
--model "$OPENAI_MODEL" \
--prompt-file /app/commands/review-pr-core.md \
--commit-id "${COMMIT_ID:-}" \
${DRY_RUN_FLAG:-}
${DRY_RUN_FLAG:-} \
"$@"

echo "==> codehawk complete."
40 changes: 40 additions & 0 deletions src/batch_review_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ def run(self, dry_run: bool = False, commit_id: str = "") -> Dict[str, Any]:
if previous_findings:
logger.info("Re-push detected: %d previous findings", len(previous_findings))

# --- Step 2d: Re-push → verify fixes only (cheap), skip full review ---
if previous_findings:
return self._run_verify_only(
previous_findings, dry_run=dry_run, commit_id=commit_id,
)

# --- Step 3: Build graph once ---
graph_store = self._build_graph(len(code_files))

Expand Down Expand Up @@ -191,6 +197,40 @@ def _fetch_pr_details(self):
logger.warning("PR pre-fetch failed: %s", exc)
return None

_VERIFY_MODEL = "gpt-5-codex"
_VERIFY_MAX_TURNS = 5

def _run_verify_only(
self,
previous_findings: list,
dry_run: bool = False,
commit_id: str = "",
) -> Dict[str, Any]:
"""Run fix verification only (no full review). Cheap agent-based path for re-pushes.

Uses a ReviewJob in VERIFY_FIXES mode with gpt-4o-mini and a low turn
budget. The agent has tools (read_local_file, search_code) so it can
investigate cross-file fixes — unlike the deterministic fix_verifier.
"""
logger.info(
"Running fix verification agent (%s, max_turns=%d) for %d prior findings",
self._VERIFY_MODEL, self._VERIFY_MAX_TURNS, len(previous_findings),
)

config = ReviewJobConfig(
pr_id=self.pr_id,
repo=self.repo,
workspace=self.workspace,
model=self._VERIFY_MODEL,
max_turns=self._VERIFY_MAX_TURNS,
prompt_path=self.prompt_path,
vcs=self.vcs,
previous_findings=previous_findings,
review_mode=ReviewMode.VERIFY_FIXES,
)
job = ReviewJob(config, settings=self.settings)
return job.run(dry_run=dry_run, commit_id=commit_id)

def _fetch_previous_findings(self) -> list:
"""Fetch existing review threads with cr_id markers."""
try:
Expand Down
2 changes: 2 additions & 0 deletions src/models/review_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ class FixVerification:
cr_id: str # matches a Finding.id from prior review
status: str # "fixed" | "still_present" | "not_relevant"
reason: str # human-readable explanation
severity: Optional[str] = None # from original finding: "critical" | "warning" | "suggestion"
category: Optional[str] = None # from original finding: "security" | "performance" | etc.


@dataclass
Expand Down
43 changes: 15 additions & 28 deletions src/post_findings.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ def _parse_findings_file(data: dict):
cr_id=fv.get("cr_id", "unknown"),
status=fv.get("status", "not_relevant"),
reason=fv.get("reason", ""),
severity=fv.get("severity"),
category=fv.get("category"),
))
except Exception as exc:
logger.warning("Skipping unparseable fix_verification: %s", exc)
Expand Down Expand Up @@ -840,28 +842,7 @@ def _build_summary_markdown(
lines.append(f"- ➖ **{fv.cr_id}** — Not relevant: {fv.reason}")
lines.append("")

# CI Gate
gate_passed = gate_result.get("passed", True)
gate_icon = "✅" if gate_passed else "🚨"
lines += [
"## 🚦 CI Gate",
f"{gate_icon} Gate: **{'PASSED' if gate_passed else 'FAILED'}**",
"",
]
if gate_result.get("reasons"):
for reason in gate_result["reasons"]:
lines.append(f"- {reason}")
lines.append("")

# Next steps
lines += [
"## 🚀 Next Steps",
"1. Review the inline comments on specific files",
"2. Address critical and warning items",
"3. Consider implementing suggestions for code quality",
"4. Reply to any comments if you need clarification",
"",
]
# CI Gate (internal use only — not shown in PR summary)

# Overall summary — agent-generated narrative
if getattr(findings_file, "summary", None):
Expand Down Expand Up @@ -1195,9 +1176,16 @@ def run(
new_findings = [f for f in capped if f.id not in posted_cr_ids]
deduped_count = len(capped) - len(new_findings)

# 8. Score (use mode-adjusted findings)
all_adjusted = scorer.apply_mode_multipliers(capped, findings_file.review_modes)
score = scorer.calculate_pr_score(all_adjusted)
# 8. Score (apply mode multipliers so severity is consistent for scoring AND posting)
all_adjusted = capped
if is_verify_only and findings_file.fix_verifications:
score = scorer.calculate_verify_score(findings_file.fix_verifications, findings_file.review_modes)
else:
all_adjusted = scorer.apply_mode_multipliers(capped, findings_file.review_modes)
score = scorer.calculate_pr_score(all_adjusted)
# Rebuild new_findings from adjusted list so posted comments reflect adjusted severity
adjusted_by_id = {f.id: f for f in all_adjusted}
new_findings = [adjusted_by_id[f.id] for f in new_findings if f.id in adjusted_by_id]

# 9. Post inline comments (skip entirely for verify-only)
posted_count = 0
Expand Down Expand Up @@ -1260,8 +1248,7 @@ def run(
coverage_ratio = 1.0
coverage_gate_mode = getattr(settings, "coverage_gate_mode", "hard") if settings else "hard"

# 11c. Apply coverage penalty to score (only when coverage tracking is active)
score = scorer.apply_coverage_penalty(score, coverage_ratio)
# 11c. Coverage is tracked for display but does not affect the penalty score

# 12. Gate evaluation from .codereview.yml (verify-only always passes — no new findings)
gate_config = _load_codereview_yml(workspace)
Expand All @@ -1288,7 +1275,7 @@ def run(
# 13. Post/update summary
summary_md = _build_summary_markdown(
findings_file=findings_file,
filtered_findings=capped,
filtered_findings=all_adjusted if not is_verify_only else capped,
score=score,
gate_result=gate_result,
fix_verifications=findings_file.fix_verifications,
Expand Down
81 changes: 66 additions & 15 deletions src/pr_scorer.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,25 +259,76 @@ def _extract_severity_counts(self, statistics: Dict[str, int]) -> Dict[str, int]
'good': statistics.get('good', 0)
}

def apply_coverage_penalty(self, score: PRScore, coverage_ratio: float) -> PRScore:
def calculate_verify_score(self, fix_verifications, review_modes: List[str] = None) -> PRScore:
"""Calculate score for verify-only runs from prior findings' severity/category.

Penalty = sum of penalties for all prior findings that are still_present.
Fixed findings contribute zero penalty.
Applies the same mode multipliers as full reviews for consistency.
"""
Add a coverage penalty to an existing PRScore.
if not self.enable_scoring:
return self._create_disabled_score()

For coverage below 100%, adds: (1 - coverage_ratio) * 50 penalty points.
This ensures incomplete reviews score poorly even when the gate mode is 'log'.
total_penalty = 0.0
original_penalty = 0.0
category_penalties: Dict[str, float] = {}
stats: Dict[str, int] = {'critical': 0, 'warning': 0, 'suggestion': 0, 'good': 0}

Args:
score: Existing PRScore to augment.
coverage_ratio: Fraction of code files reviewed (0.0–1.0).
modes = {m.lower() for m in (review_modes or [])}

Returns:
New PRScore with coverage penalty applied.
"""
if coverage_ratio >= 1.0:
return score
penalty = round((1.0 - coverage_ratio) * 50.0, 1)
from dataclasses import replace
return replace(score, total_penalty=round(score.total_penalty + penalty, 1))
for fv in fix_verifications:
sev = fv.severity or "warning"
cat = fv.category or "best_practices"

# Apply the same mode multipliers used during full review
if 'migration' in modes:
sev = 'critical'
elif 'security' in modes and cat == 'security':
if sev == 'warning':
sev = 'critical'
elif 'performance' in modes and cat == 'performance':
if sev == 'warning':
sev = 'critical'
elif 'architecture' in modes and cat in ('best_practices', 'architecture'):
if sev == 'suggestion':
sev = 'warning'

issue_penalty = self._calculate_issue_penalty(sev, cat)
original_penalty += issue_penalty

if fv.status == "still_present":
total_penalty += issue_penalty
category_penalties[cat] = category_penalties.get(cat, 0.0) + issue_penalty
stats[sev] = stats.get(sev, 0) + 1

category_penalties = {k: round(v, 1) for k, v in category_penalties.items() if v > 0}
total_penalty = round(total_penalty, 1)

overall_stars = self._penalty_to_stars(total_penalty)
quality_level = self._get_quality_level(total_penalty)
category_stars = {
cat: self._penalty_to_stars(p) for cat, p in category_penalties.items()
}

fixed_count = sum(1 for fv in fix_verifications if fv.status == "fixed")
total_count = len(fix_verifications)
breakdown = [
f"Original penalty: {original_penalty:.1f} points ({total_count} prior findings)",
f"Fixed: {fixed_count} findings (penalty removed)",
f"Still present: {total_count - fixed_count} findings",
f"Current penalty: {total_penalty:.1f} points",
f"Quality level: {quality_level}",
]

return PRScore(
total_penalty=total_penalty,
overall_stars=overall_stars,
category_penalties=category_penalties,
category_stars=category_stars,
issues_by_severity=self._extract_severity_counts(stats),
scoring_breakdown=breakdown,
quality_level=quality_level,
)

def _create_disabled_score(self) -> PRScore:
return PRScore(
Expand Down
88 changes: 87 additions & 1 deletion src/review_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,11 @@ def create_findings(self) -> Path:
except Exception as exc:
logger.debug("Previous findings fetch skipped: %s", exc)

# VERIFY_FIXES: lightweight path — no diffs, no graph, just the
# previous findings table + tools so the agent can investigate.
if self.config.review_mode == ReviewMode.VERIFY_FIXES:
return self._create_verify_findings()

changed_files = []
pr_details = None
skipped_count = 0
Expand Down Expand Up @@ -254,6 +259,13 @@ def create_findings(self) -> Path:

two_pass_enabled = getattr(self.settings, "two_pass_enabled", True)

# VERIFY_FIXES mode must use single-pass — the two-pass scan/verify
# pipeline looks for NEW candidates, but verify-only needs the full
# prompt with previous findings + fix_verifications instructions.
if self.config.review_mode == ReviewMode.VERIFY_FIXES:
two_pass_enabled = False
logger.info("VERIFY_FIXES mode — forcing single-pass")

if two_pass_enabled:
try:
self._agent_result = self._run_two_pass(
Expand Down Expand Up @@ -288,6 +300,51 @@ def create_findings(self) -> Path:

return self._findings_path

def _create_verify_findings(self) -> Path:
"""Lightweight VERIFY_FIXES path — no diffs, no graph, just tools + prior findings."""
prompt = self._build_fix_verify_prompt()
logger.info("VERIFY_FIXES prompt: %d chars", len(prompt))

runner = OpenAIAgentRunner(
settings=self.settings,
workspace=self.config.workspace,
model=self.config.model,
pr_id=self.config.pr_id,
repo=self.config.repo,
)
self._agent_result = runner.run(prompt, max_turns=self.config.max_turns)

if not self._agent_result.findings_data:
import warnings
warnings.warn(
"Verify agent did not produce findings JSON; empty fix_verifications.",
stacklevel=2,
)

self._stamp_usage(self._agent_result)
self._write_findings(self._agent_result.findings_data)
return self._findings_path

def _build_fix_verify_prompt(self) -> str:
"""Build a minimal prompt for VERIFY_FIXES — just the base instructions,
previous findings table, and verify-only constraints. No diffs injected."""
if self.config.prompt_text:
text = self.config.prompt_text
else:
text = self.config.prompt_path.read_text(encoding="utf-8")

ws_posix = str(self.config.workspace).replace("\\", "/")
text = text.replace("/workspace/", ws_posix + "/")
text = text.replace("$PR_ID", str(self.config.pr_id))
text = text.replace("$REPO", self.config.repo)
text = text.replace("$VCS", self.config.vcs)

if self.config.previous_findings:
text += self._build_previous_findings_section(self.config.previous_findings)

text += self._build_verify_only_instructions()
return text

# ------------------------------------------------------------------
# Phase 2 — score, gate, post comments
# ------------------------------------------------------------------
Expand Down Expand Up @@ -523,7 +580,21 @@ def _build_verify_only_instructions(self) -> str:
"- `findings[]` MUST be empty — do NOT add new findings\n"
"- Populate `fix_verifications[]` for EVERY cr_id in the table above\n"
"- Skip Steps 3-5 of the standard review process\n"
"- Each `fix_verifications` entry requires: `cr_id`, `status` (fixed/still_present/not_relevant), `reason`\n"
"- Each `fix_verifications` entry requires: `cr_id`, `status`, `reason`\n\n"
"### Status definitions (use EXACTLY these values)\n"
"- `fixed` — the issue described in the finding has been resolved "
"(code was changed, test was added, bug was fixed — the problem no longer exists)\n"
"- `still_present` — the issue still exists in the current code\n"
"- `not_relevant` — the file was deleted or the finding no longer applies "
"because the relevant code was entirely removed (NOT for issues that were fixed)\n\n"
"**IMPORTANT:** If a finding was addressed/fixed by the developer, use `fixed` — "
"do NOT use `not_relevant`. Reserve `not_relevant` only for deleted files or "
"removed code paths.\n\n"
"### How to verify\n"
"- Use `read_local_file` to read the current file contents from the workspace\n"
"- Use `search_code` to search for patterns across the codebase (e.g. test files)\n"
"- Do NOT use `get_file_content` or `get_file_diff` — the workspace is already cloned locally\n"
"- Do NOT call `get_pr` — all prior findings are in the table above\n"
)

def _build_check_new_instructions(self) -> str:
Expand Down Expand Up @@ -1328,6 +1399,20 @@ def _stamp_usage(self, result: AgentResult):
result.findings_data.setdefault("tool_calls", result.tool_calls_count)
result.findings_data.setdefault("agent", "openai-api")

def _enrich_fix_verifications(self, data: dict):
"""Stamp severity/category from previous_findings onto each fix_verification entry."""
prev = self.config.previous_findings or []
if not prev:
return
by_cr_id = {f.cr_id: f for f in prev if f.cr_id}
for fv in data.get("fix_verifications", []):
if not isinstance(fv, dict):
continue
prior = by_cr_id.get(fv.get("cr_id"))
if prior:
fv.setdefault("severity", prior.severity)
fv.setdefault("category", prior.category)

def _write_findings(self, data: dict):
data["pr_id"] = self.config.pr_id
data["repo"] = self.config.repo
Expand All @@ -1339,6 +1424,7 @@ def _write_findings(self, data: dict):
data["findings"] = []
if "verify_fixes" not in review_modes:
review_modes.append("verify_fixes")
self._enrich_fix_verifications(data)
elif self.config.review_mode == ReviewMode.CHECK_NEW:
data["fix_verifications"] = []
if "check_new" not in review_modes:
Expand Down
2 changes: 1 addition & 1 deletion src/tools/workspace_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
def _resolve_workspace_path(workspace: Path, file_path: str) -> Path:
"""Resolve a file path safely within the workspace.

ADO file paths start with '/' (e.g. '/BluSKYFunctionApps/...').
ADO file paths start with '/' (e.g. '/MyRepo/...').
On Windows, Path(workspace) / '/absolute' drops the workspace prefix.
Strip the leading '/' so it joins correctly.
"""
Expand Down
Loading
Loading