Skip to content

feat(openspec): code review bug-finding and sidecar venv fix#176

Closed
djm81 wants to merge 1 commit intodevfrom
feature/code-review-bug-finding-and-sidecar-venv-fix
Closed

feat(openspec): code review bug-finding and sidecar venv fix#176
djm81 wants to merge 1 commit intodevfrom
feature/code-review-bug-finding-and-sidecar-venv-fix

Conversation

@djm81
Copy link
Copy Markdown
Contributor

@djm81 djm81 commented Apr 8, 2026

Summary

  • Adds OpenSpec change proposal code-review-bug-finding-and-sidecar-venv-fix covering four improvements discovered during external repo validation (crewAI, gpt-researcher)
  • Registers change in CHANGE_ORDER.md under new "Code review and sidecar validation improvements" section, linked to Feature #175 and Epic #162
  • No code changes — spec, design, tasks, and delta specs only

Change scope

  1. Semgrep bugs.yaml — new security/bug rule config alongside existing clean_code.yaml
  2. CrossHair --bug-hunt mode — extended timeouts (10s per path, 120s total) via new flag
  3. MISSING_ICONTRACT auto-suppression — detect icontract imports via AST; skip findings when absent
  4. Sidecar venv self-scan fix — exclude .specfact/ from rglob("*.py") in all framework extractors

Test plan

  • Confirm change folder structure matches OpenSpec schema (openspec status --change code-review-bug-finding-and-sidecar-venv-fix)
  • No code changes to verify in this PR — implementation tracked in #174

🤖 Generated with Claude Code

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38aafd9d-d5b0-4276-82f6-970e52fdf3ba

📥 Commits

Reviewing files that changed from the base of the PR and between 5677559 and c0e42ac.

📒 Files selected for processing (9)
  • openspec/CHANGE_ORDER.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/.openspec.yaml
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/design.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/proposal.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-bug-finding/spec.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/contract-runner/spec.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/review-run-command/spec.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/sidecar-route-extraction/spec.md
  • openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/tasks.md

📝 Walkthrough

OpenSpec Change: code-review-bug-finding-and-sidecar-venv-fix

Overview

This PR introduces an OpenSpec change proposal documenting four improvements to the specfact-code-review bundle and specfact-codebase module, identified during external repo validation on crewAI and gpt-researcher. No code changes are included in this PR—only specification, design, tasks, and delta specs. Implementation is tracked in issue #174.

Bundle and Module Surface

specfact-code-review: New CLI Surface and Config

New Command Flag:

  • specfact code review run --bug-hunt — Routes to extended CrossHair timeouts (10s per-path, 120s total vs. standard 2s/30s). Backward-compatible; defaults to False. Composable with existing --scope, --json, --out flags.

New Semgrep Configuration:

  • .semgrep/bugs.yaml — A separate security/correctness rule set (initial scope: 5–10 high-confidence rules for dangerous system calls, hardcoded secrets, unsafe eval/exec, useless equality checks). Runs as a second semgrep pass after clean_code.yaml. Gracefully skips when absent; no error propagation.

Modified Internal API:

  • ReviewRunRequest — New bug_hunt: bool = False field
  • run_contract_check() — New bug_hunt parameter; extended per-path timeout logic
  • _run_crosshair() — Timeout config controlled by bug_hunt flag
  • contract_runner.py — New _has_icontract_usage(files) AST scan; auto-suppresses MISSING_ICONTRACT findings when zero icontract imports exist in reviewed files
  • semgrep_runner.py — New find_semgrep_bugs_config() helper; new run_semgrep_bugs() runner function

specfact-codebase: Framework Extractor Bug Fix

Sidecar Route Extraction:

  • All framework extractors (FastAPI, Flask, Django, DRF) exclude .specfact/ (plus .git/, __pycache__/, node_modules/) from Python file discovery via a shared BaseFrameworkExtractor._iter_python_files() generator.
  • Fixes inflated route counts (gpt-researcher: 25,947 reported vs. 19 real) caused by scanning the sidecar's own installed venv.

Manifest and Integrity

Version Bumps:

  • specfact-codebase: Patch version bump (bug fix to extractor scan path exclusion)
  • specfact-code-review: No immediate bump required — the --bug-hunt flag and MISSING_ICONTRACT auto-suppression are backward-compatible additions. If semgrep bugs.yaml is shipped as a bundled resource, a minor bump is appropriate; if rules are discovered from the repo only, no bundle version change needed.

Registry/Signature Impact:

  • None. Both changes are additive (new flag, new config file) or bug fixes (extractor exclusion). Existing behavior unchanged when --bug-hunt is not passed and when bugs.yaml is absent.

Cross-Repository Alignment

specfact-cli Dependency:

  • No specfact-cli changes required. The --bug-hunt flag and contract-runner logic are self-contained within specfact-code-review.
  • ReviewRunRequest and ReviewFinding contracts already support the needed fields; no new imports or adapter changes.

Tool Dependencies:

  • CrossHair is already a declared dev-dependency in specfact-code-review (no new tooling required).
  • Semgrep already integrated; bugs.yaml is a new config file, not a tool upgrade.

Developer-Facing Paths:

  • No changes to assumptions about virtualenv locations or dev-dependency installation paths beyond the fix to exclude .specfact/ from sidecar scans.

Documentation

Specification Coverage:

  • specs/code-review-bug-finding/spec.md — Semgrep bugs.yaml pass behavior and --bug-hunt timeout requirements (52 lines)
  • specs/contract-runner/spec.md — Modified MISSING_ICONTRACT suppression and CrossHair timeout scenarios (61 lines)
  • specs/review-run-command/spec.md--bug-hunt flag CLI surface and defaults (20 lines)
  • specs/sidecar-route-extraction/spec.md — Framework extractor .specfact/ exclusion requirements with per-framework scenarios (39 lines)

design.md — Rationale for decisions: separate bugs.yaml config (vs. merged), flag-based bug-hunt mode (vs. subcommand), auto-detection of icontract (vs. suppression flag), shared helper for sidecar exclusion (vs. orchestrator-level filtering). Includes risk assessment (CrossHair false positives mitigated by existing SideEffectDetected filtering, bugs.yaml rule maintenance process, MISSING_ICONTRACT suppression scope limits).

tasks.md — Implementation checklist across five work streams: sidecar extractor fixes, MISSING_ICONTRACT auto-suppression, bug-hunt timeout wiring, semgrep bugs.yaml integration, and TDD evidence gates.

Backward Compatibility:

  • All changes are backward-compatible; no CHANGELOG migration section required. Existing specfact code review run calls unchanged. --bug-hunt is opt-in.

OpenSpec Integration

Change ID: code-review-bug-finding-and-sidecar-venv-fix
Registered in: CHANGE_ORDER.md under "Code review and sidecar validation improvements" section
Related:

  • Feature #175 (parent feature for code-review enhancements)
  • Epic #162 (related epic scope)
  • Issue #174 (implementation work; separate from this spec PR)

Scenario Coverage:
All specs include concrete GIVEN-WHEN-THEN scenarios covering present/absent configurations, flag combinations, edge cases (private functions, missing icontract, missing bugs.yaml), and integration with existing flags.

Walkthrough

Added OpenSpec documentation defining specifications, design, and implementation tasks for code-review and sidecar validation improvements, including a --bug-hunt flag, semgrep bug-finding pass, icontract detection, and .specfact/ directory exclusion from route extraction.

Changes

Cohort / File(s) Summary
Change Registration
openspec/CHANGE_ORDER.md, openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/.openspec.yaml
Registered new change entry in CHANGE_ORDER.md with dependencies on #175 and #162; added standard spec metadata file.
Design & Proposal
openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/design.md, openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/proposal.md
Introduced design rationale and functional proposal for bug-hunt mode, semgrep bugs.yaml pass, icontract suppression, and sidecar .specfact/ exclusion with backwards-compatibility and migration notes.
Specifications
openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/code-review-bug-finding/spec.md, openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/contract-runner/spec.md, openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/review-run-command/spec.md, openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/specs/sidecar-route-extraction/spec.md
Defined behavioral specifications for second semgrep pass (bugs.yaml), --bug-hunt CLI flag with extended CrossHair timeouts, icontract-aware contract checking with MISSING_ICONTRACT suppression, and framework extractor .specfact/ path exclusion across FastAPI/Flask/Django/DRF.
Implementation Planning
openspec/changes/code-review-bug-finding-and-sidecar-venv-fix/tasks.md
Enumerated five task areas covering BaseFrameworkExtractor helper, icontract detection, bug-hunt option threading, semgrep bugs configuration, and quality-gate execution with version bump expectations in module-package.yaml.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Possibly related PRs

Suggested labels

openspec, documentation, enhancement


📌 Review Note on Adapter Boundaries

This OpenSpec change registers modifications to bundled modules (specfact-code-review and specfact-codebase). The specs and tasks reference module-package.yaml version bumping and cross-module API contracts (e.g., ReviewRunRequest.bug_hunt addition, BaseFrameworkExtractor public helper). Ensure that any implementation PRs maintain registry consistency, validate module-package.yaml updates match versioning policy, and preserve compatibility with specfact-cli core's module-loading and API contracts.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/code-review-bug-finding-and-sidecar-venv-fix

Comment @coderabbitai help to get the list of available commands and usage tips.

@djm81 djm81 closed this Apr 8, 2026
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