feat(api): PROOF9 REST API router — /api/v2/proof endpoints - #459
Conversation
Add proof_v2.py router exposing PROOF9 quality system via REST API:
- POST /api/v2/proof/requirements — capture_requirement() (classify glitch → obligations → stubs)
- GET /api/v2/proof/requirements — list_requirements() with optional ?status= filter
- GET /api/v2/proof/requirements/{id} — get_requirement()
- POST /api/v2/proof/run — run_proof() with full/gate_filter params
- POST /api/v2/proof/requirements/{id}/waive — waive_requirement()
- GET /api/v2/proof/status — aggregated counts + full requirement list
- GET /api/v2/proof/requirements/{id}/evidence — list_evidence()
Router registered in server.py; OpenAPI tag "proof-v2" added to /docs.
35 new integration tests in tests/ui/test_proof_v2.py (all passing).
Thin-adapter pattern: all logic delegates to codeframe.core.proof.*.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughA new v2 FastAPI router was added at /api/v2/proof exposing endpoints to capture, list, fetch, run, waive, aggregate status, and list evidence for PROOF9; the router delegates to core proof functions, is registered in the server OpenAPI tags, and is covered by integration tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router as ProofV2 Router
participant Workspace as Workspace (dep)
participant Core as core.proof (ledger/runner)
Client->>Router: POST /api/v2/proof/requirements (capture payload)
Router->>Workspace: get_v2_workspace()
Router->>Core: capture_requirement(workspace, payload)
Core-->>Router: requirement object
Router-->>Client: 201 Created (RequirementResponse)
Client->>Router: POST /api/v2/proof/run (RunProofRequest)
Router->>Workspace: get_v2_workspace()
Router->>Core: run_proof(workspace, run_id, filters)
Core-->>Router: run results (gate outcomes)
Router-->>Client: 200 OK (RunProofResponse with run_id and results)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Review: PROOF9 REST API RouterThis is a clean, well-structured addition. The thin-adapter pattern is correctly applied, the rate limiting choices are appropriate ( A few items worth addressing: 1. In 2. The generated 3. Double When a 4. When 5.
6. Missing auth dependency Other v2 routers include API key auth via a dependency. These endpoints have none. If this is intentional (e.g., internal use only, or auth added in a follow-up), add a comment explaining why it's omitted so the omission is clearly deliberate rather than accidental. 7. Minor: non-standard
Overall this is ready to merge with items 1, 5, and 6 resolved. Items 2–4 are judgment calls that can be deferred to follow-up if preferred. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@codeframe/ui/routers/proof_v2.py`:
- Around line 74-78: The schema only declares `gate` but the API contract
expects `gate_filter`; update the RunProofRequest model to include an
Optional[Gate] field named `gate_filter` (use Field(default=None,
description="Run only this gate (unit, sec, contract, etc.)") and consider
adding aliasing/backwards-compatibility if needed), and ensure code that
consumes RunProofRequest (references to `gate`) reads from `gate_filter` when
present (or normalizes into a single internal variable) so POST bodies using
`gate_filter` validate and are handled correctly.
- Line 224: The endpoints currently accept workspace: Workspace =
Depends(get_v2_workspace) where get_v2_workspace allows an optional
workspace_path; update each endpoint that uses this dependency (any function
with the parameter workspace: Workspace = Depends(get_v2_workspace)) to require
an explicit workspace_path query param (e.g., add workspace_path: str =
Query(..., alias="workspace_path") to the endpoint signature) and ensure the
Workspace dependency is resolved using that explicit workspace_path (replace the
bare Depends(get_v2_workspace) usage with a dependency invocation that passes
the required workspace_path into get_v2_workspace or create a small wrapper
dependency that accepts workspace_path and calls
get_v2_workspace(workspace_path)); apply this change to every endpoint currently
using get_v2_workspace so workspace_path is mandatory and unambiguous.
- Around line 318-337: run_proof writes artifacts/evidence and returns the
authoritative run identifier, but the handler currently overwrites it by
generating a new UUID for run_id before returning RunProofResponse; instead,
extract and use the run_id produced by run_proof (from its return value or
metadata it returns) and pass that into RunProofResponse.run_id, removing the
local uuid generation; keep the serialized results construction and only replace
the run_id assignment so clients can correlate the response with the persisted
evidence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5810930-b16d-4e74-957b-542628950739
📒 Files selected for processing (3)
codeframe/ui/routers/proof_v2.pycodeframe/ui/server.pytests/ui/test_proof_v2.py
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| router = APIRouter(prefix="/api/v2/proof", tags=["proof-v2"]) |
There was a problem hiding this comment.
Add scoped auth to this router before shipping.
These handlers only apply rate limiting; none of them enforce the required auth scopes. At minimum, the GET routes should require read and the mutating POST routes should require write/admin as appropriate. As written, this router can be mounted and exercised without any router-level authorization boundary, and the current test app does exactly that. As per coding guidelines, API key authentication in routers must validate scopes via auth/api_key_service.py with supported scopes: read, write, admin.
Also applies to: 219-220, 252-253, 284-285, 305-306, 347-348, 377-378, 396-397
| class RunProofRequest(BaseModel): | ||
| """Request body for running proof obligations.""" | ||
|
|
||
| full: bool = Field(default=False, description="Run ALL obligations regardless of scope") | ||
| gate: Optional[Gate] = Field(default=None, description="Run only this gate (unit, sec, contract, etc.)") |
There was a problem hiding this comment.
Expose the documented gate_filter field here.
The PR contract says POST /api/v2/proof/run accepts full and gate_filter, but this schema only accepts gate. Clients following the documented request shape will get a 422, and the current tests won't catch it because they post the same non-contract field name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@codeframe/ui/routers/proof_v2.py` around lines 74 - 78, The schema only
declares `gate` but the API contract expects `gate_filter`; update the
RunProofRequest model to include an Optional[Gate] field named `gate_filter`
(use Field(default=None, description="Run only this gate (unit, sec, contract,
etc.)") and consider adding aliasing/backwards-compatibility if needed), and
ensure code that consumes RunProofRequest (references to `gate`) reads from
`gate_filter` when present (or normalizes into a single internal variable) so
POST bodies using `gate_filter` validate and are handled correctly.
| async def capture_requirement_endpoint( | ||
| request: Request, | ||
| body: CaptureRequirementRequest, | ||
| workspace: Workspace = Depends(get_v2_workspace), |
There was a problem hiding this comment.
Make workspace_path explicit on every endpoint.
get_v2_workspace declares workspace_path as optional and falls back to the default workspace/CWD, so these routes currently succeed without the query parameter. That violates the v2 router contract and makes the target workspace ambiguous in multi-workspace deployments; the dependency override in tests/ui/test_proof_v2.py also hides the regression. As per coding guidelines, All API endpoints must require workspace_path query parameter.
Also applies to: 257-257, 289-289, 310-310, 353-353, 381-381, 401-401
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@codeframe/ui/routers/proof_v2.py` at line 224, The endpoints currently accept
workspace: Workspace = Depends(get_v2_workspace) where get_v2_workspace allows
an optional workspace_path; update each endpoint that uses this dependency (any
function with the parameter workspace: Workspace = Depends(get_v2_workspace)) to
require an explicit workspace_path query param (e.g., add workspace_path: str =
Query(..., alias="workspace_path") to the endpoint signature) and ensure the
Workspace dependency is resolved using that explicit workspace_path (replace the
bare Depends(get_v2_workspace) usage with a dependency invocation that passes
the required workspace_path into get_v2_workspace or create a small wrapper
dependency that accepts workspace_path and calls
get_v2_workspace(workspace_path)); apply this change to every endpoint currently
using get_v2_workspace so workspace_path is mandatory and unambiguous.
| try: | ||
| results = run_proof( | ||
| workspace, | ||
| full=body.full, | ||
| gate_filter=body.gate, | ||
| ) | ||
| # Serialize: dict[req_id → list[tuple[Gate, bool]]] → JSON-safe | ||
| serialized = { | ||
| req_id: [{"gate": gate.value, "satisfied": satisfied} for gate, satisfied in gate_results] | ||
| for req_id, gate_results in results.items() | ||
| } | ||
| # Derive a run_id from the results or generate one | ||
| import uuid | ||
| run_id = str(uuid.uuid4())[:8] | ||
|
|
||
| return RunProofResponse( | ||
| success=True, | ||
| run_id=run_id, | ||
| results=serialized, | ||
| message=f"Proof run complete: {len(results)} requirement(s) evaluated.", |
There was a problem hiding this comment.
Return the same run_id that was persisted.
codeframe.core.proof.runner.run_proof() is the code that writes artifacts and evidence, but Line 331 creates a brand new UUID after that call. The response run_id therefore won't match the evidence rows generated by the run, so clients can't correlate /run with /requirements/{id}/evidence.
💡 Suggested fix
try:
+ import uuid
+ run_id = str(uuid.uuid4())[:8]
results = run_proof(
workspace,
full=body.full,
gate_filter=body.gate,
+ run_id=run_id,
)
# Serialize: dict[req_id → list[tuple[Gate, bool]]] → JSON-safe
serialized = {
req_id: [{"gate": gate.value, "satisfied": satisfied} for gate, satisfied in gate_results]
for req_id, gate_results in results.items()
}
- # Derive a run_id from the results or generate one
- import uuid
- run_id = str(uuid.uuid4())[:8]
-
return RunProofResponse(
success=True,
run_id=run_id,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@codeframe/ui/routers/proof_v2.py` around lines 318 - 337, run_proof writes
artifacts/evidence and returns the authoritative run identifier, but the handler
currently overwrites it by generating a new UUID for run_id before returning
RunProofResponse; instead, extract and use the run_id produced by run_proof
(from its return value or metadata it returns) and pass that into
RunProofResponse.run_id, removing the local uuid generation; keep the serialized
results construction and only replace the run_id assignment so clients can
correlate the response with the persisted evidence.
CodeRabbit Review ResponseFixed (1 item):
Deferred (1 item):
Dismissed (2 items):
|
Code Review — PR #459: PROOF9 REST API RouterOverall this is a solid, well-structured addition. The thin-adapter pattern is followed correctly, core is kept headless, and the test suite is comprehensive. Below are issues grouped by severity. Critical
The other compute-heavy endpoints in this repo delegate to background tasks or use import asyncio
results = await asyncio.run_in_executor(None, lambda: run_proof(workspace, full=body.full, gate_filter=body.gate, run_id=run_id))Without this, a long proof run will make the server unresponsive to all other requests. ImportantDouble reqs = list_requirements(workspace, status=status_filter)
all_reqs = list_requirements(workspace) if status_filter else reqsWhen a status filter is active, this issues two sequential database queries — one filtered, one for all rows (to compute
counts: dict[str, int] = {s.value: 0 for s in ReqStatus}
for req in reqs:
counts[req.status.value] = counts.get(req.status.value, 0) + 1The dict comprehension pre-fills all statuses with 0, but then
Missing If a requirement is already waived and
An 8-character hex prefix of a UUID provides only ~4 billion unique values before truncation artifacts. This is the same pattern used in Maintainability / Nice-to-Have
client = TestClient(app)
client.workspace = test_workspace
return client
This is correct behaviour (return 404 for unknown req_id), but it adds an extra DB round-trip. No The core Architecture note: no CLAUDE.md Rule 0 and the Golden Path require the CLI to work without a server. This PR adds the REST layer but the What is done well
Summary: The blocking I/O issue in |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
codeframe/ui/routers/proof_v2.py (3)
275-282: Avoid double database call when filtering by status.When
status_filteris set,list_requirementsis called twice: once filtered (line 275) and once unfiltered (line 276) for aggregate counts. Consider fetching all requirements once and filtering in memory.♻️ Proposed optimization
- reqs = list_requirements(workspace, status=status_filter) - all_reqs = list_requirements(workspace) if status_filter else reqs + all_reqs = list_requirements(workspace) + reqs = [r for r in all_reqs if r.status == status_filter] if status_filter else all_reqs return RequirementListResponse( requirements=[_req_to_response(r) for r in reqs], total=len(reqs), by_status=_count_by_status(all_reqs), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@codeframe/ui/routers/proof_v2.py` around lines 275 - 282, The current code calls list_requirements twice (list_requirements(workspace, status=status_filter) and list_requirements(workspace)) causing a duplicate DB call when status_filter is set; instead call list_requirements(workspace) once to get all_reqs, then derive reqs in-memory by applying the status_filter (e.g., filter all_reqs by requirement.status) and pass RequirementListResponse(requirements=[_req_to_response(r) for r in reqs], total=len(reqs), by_status=_count_by_status(all_reqs)); update references to reqs/all_reqs accordingly and keep use of _req_to_response and _count_by_status.
245-250: Broad exception handling may mask validation errors.Catching all exceptions and returning 500 with
EXECUTION_FAILEDcould hide specific errors from core (e.g., validation failures, classification errors) that might warrant different status codes (400, 422). Consider catching specific exception types or re-raisingHTTPExceptioninstances.♻️ Proposed approach
except Exception as e: + if isinstance(e, HTTPException): + raise logger.error("Failed to capture requirement: %s", e, exc_info=True) raise HTTPException( status_code=500, detail=api_error("Failed to capture requirement", ErrorCodes.EXECUTION_FAILED, str(e)), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@codeframe/ui/routers/proof_v2.py` around lines 245 - 250, The catch-all except block in the proof capture flow (the except Exception as e handling that logs and raises a 500) can mask validation or domain errors; update the handler in proof_v2.py to catch and handle specific exception types (e.g., ValidationError, ValueError, ClassificationError or whatever domain exceptions your core raises) and return appropriate HTTPException status codes (400/422) or re-raise existing HTTPException instances unchanged; only use a generic except Exception as e as a final fallback to log and return the 500 with ErrorCodes.EXECUTION_FAILED. Ensure you reference the existing logger.error call and the HTTPException raise site so you modify that block to differentiate and re-raise HTTPException when encountered.
207-212: Minor: Simplify counter increment.Since
countsis pre-initialized with allReqStatusvalues at line 209,counts.get(..., 0)is unnecessary.♻️ Proposed simplification
def _count_by_status(reqs) -> dict[str, int]: """Aggregate requirement counts by status value.""" counts: dict[str, int] = {s.value: 0 for s in ReqStatus} for req in reqs: - counts[req.status.value] = counts.get(req.status.value, 0) + 1 + counts[req.status.value] += 1 return counts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@codeframe/ui/routers/proof_v2.py` around lines 207 - 212, The _count_by_status function pre-initializes counts for every ReqStatus so using counts.get(..., 0) is redundant; change the increment inside the loop in _count_by_status to directly increment the pre-existing key (e.g., counts[req.status.value] += 1) and keep the dict[str, int] typing and initialization using ReqStatus to ensure all statuses exist before counting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@codeframe/ui/routers/proof_v2.py`:
- Around line 374-375: waive_requirement(workspace, req_id, waiver) can return
None if the requirement was deleted between the earlier existence check and the
waiver attempt, so guard against that before calling _req_to_response; after
calling updated = waive_requirement(...), check if updated is None and return an
appropriate error response (e.g., 404/HTTPException or a JSON error) referencing
req_id and context, otherwise call and return _req_to_response(updated).
---
Nitpick comments:
In `@codeframe/ui/routers/proof_v2.py`:
- Around line 275-282: The current code calls list_requirements twice
(list_requirements(workspace, status=status_filter) and
list_requirements(workspace)) causing a duplicate DB call when status_filter is
set; instead call list_requirements(workspace) once to get all_reqs, then derive
reqs in-memory by applying the status_filter (e.g., filter all_reqs by
requirement.status) and pass
RequirementListResponse(requirements=[_req_to_response(r) for r in reqs],
total=len(reqs), by_status=_count_by_status(all_reqs)); update references to
reqs/all_reqs accordingly and keep use of _req_to_response and _count_by_status.
- Around line 245-250: The catch-all except block in the proof capture flow (the
except Exception as e handling that logs and raises a 500) can mask validation
or domain errors; update the handler in proof_v2.py to catch and handle specific
exception types (e.g., ValidationError, ValueError, ClassificationError or
whatever domain exceptions your core raises) and return appropriate
HTTPException status codes (400/422) or re-raise existing HTTPException
instances unchanged; only use a generic except Exception as e as a final
fallback to log and return the 500 with ErrorCodes.EXECUTION_FAILED. Ensure you
reference the existing logger.error call and the HTTPException raise site so you
modify that block to differentiate and re-raise HTTPException when encountered.
- Around line 207-212: The _count_by_status function pre-initializes counts for
every ReqStatus so using counts.get(..., 0) is redundant; change the increment
inside the loop in _count_by_status to directly increment the pre-existing key
(e.g., counts[req.status.value] += 1) and keep the dict[str, int] typing and
initialization using ReqStatus to ensure all statuses exist before counting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9622a7d-2063-402d-955b-41f10f00c711
📒 Files selected for processing (1)
codeframe/ui/routers/proof_v2.py
| updated = waive_requirement(workspace, req_id, waiver) | ||
| return _req_to_response(updated) |
There was a problem hiding this comment.
Handle potential None return from waive_requirement.
waive_requirement() returns Optional[Requirement]. If the requirement is deleted between the existence check (line 357) and the waive call (line 374), updated will be None, causing an error in _req_to_response().
🛡️ Proposed defensive check
updated = waive_requirement(workspace, req_id, waiver)
+ if not updated:
+ raise HTTPException(
+ status_code=404,
+ detail=api_error(
+ f"Requirement not found: {req_id}",
+ ErrorCodes.NOT_FOUND,
+ "Requirement may have been deleted",
+ ),
+ )
return _req_to_response(updated)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@codeframe/ui/routers/proof_v2.py` around lines 374 - 375,
waive_requirement(workspace, req_id, waiver) can return None if the requirement
was deleted between the earlier existence check and the waiver attempt, so guard
against that before calling _req_to_response; after calling updated =
waive_requirement(...), check if updated is None and return an appropriate error
response (e.g., 404/HTTPException or a JSON error) referencing req_id and
context, otherwise call and return _req_to_response(updated).
Follow-up to previous reviewsThe diff is unchanged from my last review. Two items are still open: 1. Blocking I/O in results = run_proof(workspace, full=body.full, gate_filter=body.gate, run_id=run_id)This calls blocking subprocess I/O (pytest, ruff) directly inside an import asyncio
results = await asyncio.to_thread(run_proof, workspace, full=body.full, gate_filter=body.gate, run_id=run_id)
2. The 404 guard exists but the No new issues from the diff — the |
1 participant
Summary
Closes #456.
Adds
proof_v2.py— the missing v2 REST API router for the PROOF9 quality system. Follows the thin-adapter pattern used by all other 18 v2 routers.core/proof/{capture,ledger,runner}.pyfunctionstasks_v2.py,blockers_v2.py)@rate_limit_standard()for reads/writes,@rate_limit_ai()forPOST /run(gate execution is compute-heavy)proof-v2added — proof endpoints now appear in/docsserver.pyEndpoints
/api/v2/proof/requirementscapture_requirement()/api/v2/proof/requirementslist_requirements()/api/v2/proof/requirements/{id}get_requirement()/api/v2/proof/runrun_proof()/api/v2/proof/requirements/{id}/waivewaive_requirement()/api/v2/proof/statuslist_requirements()aggregated/api/v2/proof/requirements/{id}/evidencelist_evidence()Test plan
tests/ui/test_proof_v2.py— all passingtests/core/test_proof9.py) — all still passingruff checkclean on both new filescore/proof/*Summary by CodeRabbit