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
98 changes: 98 additions & 0 deletions alembic/versions/b94c1a2be26e_add_p2c_repro_context_pack_tables.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
"""add_p2c_repro_context_pack_tables

Revision ID: b94c1a2be26e
Revises: 4c71b28a2f67
Create Date: 2026-02-26 16:34:35.636031

"""
from __future__ import annotations

import sqlalchemy as sa
from alembic import op


revision = 'b94c1a2be26e'
down_revision = '4c71b28a2f67'
branch_labels = None
depends_on = None


def upgrade() -> None:
op.create_table(
'repro_context_pack',
sa.Column('id', sa.String(length=64), nullable=False),
sa.Column('user_id', sa.String(length=64), nullable=False, server_default='default'),
sa.Column('project_id', sa.String(length=64), nullable=True),
sa.Column('paper_id', sa.String(length=256), nullable=False),
sa.Column('paper_title', sa.Text(), nullable=True),
sa.Column('version', sa.String(length=16), nullable=False, server_default='v1'),
sa.Column('depth', sa.String(length=16), nullable=False, server_default='standard'),
sa.Column('status', sa.String(length=32), nullable=False, server_default='pending'),
sa.Column('objective', sa.Text(), nullable=True),
sa.Column('pack_json', sa.Text(), nullable=False, server_default='{}'),
sa.Column('confidence_overall', sa.Float(), nullable=False, server_default='0.0'),
sa.Column('warning_count', sa.Integer(), nullable=False, server_default='0'),
sa.Column('created_at', sa.DateTime(timezone=True), nullable=False),
sa.Column('updated_at', sa.DateTime(timezone=True), nullable=False),
sa.Column('deleted_at', sa.DateTime(timezone=True), nullable=True),
sa.PrimaryKeyConstraint('id'),
)
op.create_index('ix_repro_context_pack_user_id', 'repro_context_pack', ['user_id'])
op.create_index('ix_repro_context_pack_paper_id', 'repro_context_pack', ['paper_id'])
op.create_index('ix_repro_context_pack_project_id', 'repro_context_pack', ['project_id'])
op.create_index('ix_repro_context_pack_status', 'repro_context_pack', ['status'])
op.create_index('ix_repro_context_pack_confidence_overall', 'repro_context_pack', ['confidence_overall'])
op.create_index('ix_repro_context_pack_created_at', 'repro_context_pack', ['created_at'])

op.create_table(
'repro_context_stage_result',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('context_pack_id', sa.String(length=64), nullable=False),
sa.Column('stage_name', sa.String(length=64), nullable=False),
sa.Column('status', sa.String(length=16), nullable=False, server_default='completed'),
sa.Column('result_json', sa.Text(), nullable=True),
sa.Column('confidence', sa.Float(), nullable=False, server_default='0.0'),
sa.Column('duration_ms', sa.Integer(), nullable=False, server_default='0'),
sa.Column('error_message', sa.Text(), nullable=True),
sa.Column('created_at', sa.DateTime(timezone=True), nullable=False),
sa.ForeignKeyConstraint(['context_pack_id'], ['repro_context_pack.id'], ondelete='CASCADE'),
sa.PrimaryKeyConstraint('id'),
)
op.create_index('ix_repro_context_stage_result_context_pack_id', 'repro_context_stage_result', ['context_pack_id'])
op.create_index('ix_repro_context_stage_result_stage_name', 'repro_context_stage_result', ['stage_name'])
op.create_index('ix_repro_context_stage_result_status', 'repro_context_stage_result', ['status'])

op.create_table(
'repro_context_evidence',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('context_pack_id', sa.String(length=64), nullable=False),
sa.Column('evidence_type', sa.String(length=32), nullable=False),
sa.Column('ref', sa.Text(), nullable=False, server_default=''),
sa.Column('supports_json', sa.Text(), nullable=False, server_default='[]'),
sa.Column('confidence', sa.Float(), nullable=False, server_default='0.0'),
sa.ForeignKeyConstraint(['context_pack_id'], ['repro_context_pack.id'], ondelete='CASCADE'),
sa.PrimaryKeyConstraint('id'),
)
op.create_index('ix_repro_context_evidence_context_pack_id', 'repro_context_evidence', ['context_pack_id'])
op.create_index('ix_repro_context_evidence_evidence_type', 'repro_context_evidence', ['evidence_type'])

op.create_table(
'repro_context_feedback',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('context_pack_id', sa.String(length=64), nullable=False),
sa.Column('user_id', sa.String(length=64), nullable=False),
sa.Column('rating', sa.Integer(), nullable=True),
sa.Column('comment', sa.Text(), nullable=True),
sa.Column('created_at', sa.DateTime(timezone=True), nullable=False),
sa.ForeignKeyConstraint(['context_pack_id'], ['repro_context_pack.id'], ondelete='CASCADE'),
sa.PrimaryKeyConstraint('id'),
)
op.create_index('ix_repro_context_feedback_context_pack_id', 'repro_context_feedback', ['context_pack_id'])
op.create_index('ix_repro_context_feedback_user_id', 'repro_context_feedback', ['user_id'])


def downgrade() -> None:
op.drop_table('repro_context_feedback')
op.drop_table('repro_context_evidence')
op.drop_table('repro_context_stage_result')
op.drop_table('repro_context_pack')
2 changes: 2 additions & 0 deletions src/paperbot/api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
harvest,
model_endpoints,
studio_chat,
repro_context,
)
from paperbot.infrastructure.event_log.logging_event_log import LoggingEventLog
from paperbot.infrastructure.event_log.composite_event_log import CompositeEventLog
Expand Down Expand Up @@ -71,6 +72,7 @@ async def health_check():
app.include_router(harvest.router, prefix="/api", tags=["Harvest"])
app.include_router(model_endpoints.router, prefix="/api", tags=["Model Endpoints"])
app.include_router(studio_chat.router, prefix="/api", tags=["Studio Chat"])
app.include_router(repro_context.router, prefix="/api/research/repro/context", tags=["P2C"])


@app.on_event("startup")
Expand Down
270 changes: 270 additions & 0 deletions src/paperbot/api/routes/repro_context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
"""
P2C (Paper-to-Context) API Route

Endpoints:
POST /api/research/repro/context/generate — generate context pack (SSE)
GET /api/research/repro/context — list packs for a user
GET /api/research/repro/context/{pack_id} — get full pack detail
POST /api/research/repro/context/{pack_id}/session — create repro session from pack
DELETE /api/research/repro/context/{pack_id} — soft-delete pack
"""

from __future__ import annotations

import asyncio
import uuid
from dataclasses import asdict as _asdict
from typing import Literal, Optional

from fastapi import APIRouter, HTTPException
from fastapi.responses import StreamingResponse
from pydantic import BaseModel

from paperbot.api.streaming import StreamEvent, wrap_generator
from paperbot.application.services.p2c.models import (
GenerateContextRequest as P2CRequest,
new_context_pack_id,
)
from paperbot.application.services.p2c.orchestrator import ExtractionOrchestrator
from paperbot.infrastructure.stores.repro_context_store import SqlAlchemyReproContextStore

router = APIRouter()

_store = SqlAlchemyReproContextStore()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a global instance of SqlAlchemyReproContextStore makes testing more difficult and couples this module to a specific implementation. It's a good practice in FastAPI to use the dependency injection system. This would allow you to easily swap the store implementation for tests or other environments.

For example, you could define a dependency:

def get_store() -> SqlAlchemyReproContextStore:
    return SqlAlchemyReproContextStore()

And then use it in your route functions:

@router.get("/{pack_id}")
async def get_context_pack(pack_id: str, store: SqlAlchemyReproContextStore = Depends(get_store)):
    pack = store.get(pack_id)
    # ...



# ------------------------------------------------------------------ #
# Request / Response schemas #
# ------------------------------------------------------------------ #

class GenerateContextPackRequest(BaseModel):
paper_id: str
user_id: str = "default"
project_id: Optional[str] = None
track_id: Optional[int] = None
depth: Literal["fast", "standard", "deep"] = "standard"


class CreateSessionRequest(BaseModel):
executor_preference: Literal["auto", "claude_code", "codex", "local"] = "auto"
override_env: Optional[dict] = None
override_roadmap: Optional[list] = None


# ------------------------------------------------------------------ #
# POST /generate (SSE) #
# ------------------------------------------------------------------ #

async def _generate_stream(request: GenerateContextPackRequest):
"""SSE generator for context pack generation via Module 1 ExtractionOrchestrator."""
pack_id = new_context_pack_id()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There seems to be a redundant generation of pack_id. A pack_id is generated here, but the ExtractionOrchestrator also generates its own pack_id internally. This route then overwrites the orchestrator's ID with its own on line 147. This is confusing and could lead to subtle bugs.

A cleaner approach would be to generate the ID once in this route and pass it to the orchestrator to use. This would likely require a small change to the ExtractionOrchestrator.run method to accept an optional pack_id.


# Persist initial "running" record so the frontend can poll status.
_store.save(
pack_id=pack_id,
user_id=request.user_id,
paper_id=request.paper_id,
depth=request.depth,
pack_data={},
project_id=request.project_id,
confidence_overall=0.0,
warning_count=0,
)

yield StreamEvent(type="status", data={"pack_id": pack_id, "status": "running"})

# asyncio.Queue bridges the on_stage_complete callback → SSE generator.
queue: asyncio.Queue = asyncio.Queue()
_DONE = object()
_ERROR = object()

total_stages = 2 if request.depth == "fast" else 6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The total_stages is hardcoded here. However, the actual number of stages depends not only on the depth but also on the paper_type, which is determined inside the ExtractionOrchestrator. This means the progress calculation (stages_done / total_stages) on line 105 can be incorrect.

To fix this, the orchestrator should be the source of truth for the total number of stages. You could modify the on_stage_complete callback to receive the current stage index and the total number of stages from the orchestrator, which has access to the full stage sequence.

stages_done = 0

async def on_stage_complete(stage_name: str, observations: list, warnings: list) -> None:
nonlocal stages_done
stages_done += 1
confidence = (
sum(o.confidence for o in observations) / len(observations)
if observations
else 0.0
)
_store.save_stage_result(
pack_id=pack_id,
stage_name=stage_name,
status="completed",
result_data={"observations": [o.to_full() for o in observations]},
confidence=confidence,
duration_ms=0,
)
await queue.put(
StreamEvent(
type="progress",
data={
"stage": stage_name,
"progress": stages_done / total_stages,
"message": f"Completed {stage_name}",
},
)
)

p2c_request = P2CRequest(
paper_id=request.paper_id,
user_id=request.user_id,
project_id=request.project_id,
track_id=request.track_id,
depth=request.depth,
)
orchestrator = ExtractionOrchestrator()
result_holder: list = []

async def _run() -> None:
try:
pack = await orchestrator.run(p2c_request, on_stage_complete=on_stage_complete)
result_holder.append(pack)
await queue.put(_DONE)
except Exception as exc: # noqa: BLE001
result_holder.append(exc)
await queue.put(_ERROR)
Comment on lines +126 to +128

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Catching a broad Exception and then converting it to a string via str(result_holder[0]) (on line 139) loses the traceback, which is critical for debugging. This makes it very hard to diagnose problems in the orchestrator. Please consider logging the full exception with its traceback here, for example, by using logging.exception(...).


asyncio.create_task(_run())

# Drain queue, yielding progress events until the orchestrator finishes.
while True:
item = await queue.get()
if item is _DONE:
break
elif item is _ERROR:
_store.update_status(pack_id, status="failed")
yield StreamEvent(type="error", data={"message": str(result_holder[0])})
return
else:
yield item # StreamEvent from on_stage_complete

# Serialize and persist the final pack.
pack = result_holder[0]
pack_dict = _asdict(pack)
pack_dict["context_pack_id"] = pack_id # align with our DB record

_store.update_status(
pack_id,
status="completed",
pack_data=pack_dict,
confidence_overall=pack.confidence.overall,
warning_count=len(pack.warnings),
objective=pack.objective,
)

yield StreamEvent(
type="result",
data={
"context_pack_id": pack_id,
"status": "completed",
"summary": f"Context pack created for {request.paper_id}",
"confidence": _asdict(pack.confidence),
"warnings": pack.warnings,
"next_action": "create_repro_session",
},
)


@router.post("/generate")
async def generate_context_pack(request: GenerateContextPackRequest):
Comment on lines +171 to +172

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The generate_context_pack endpoint allows a user to specify any user_id in the request body. This user_id is used directly to associate the newly created context pack with a user in the database. Without verifying that the user_id matches the authenticated user, an attacker could create context packs on behalf of other users, potentially leading to data pollution or unauthorized resource consumption.

"""Generate a P2C context pack for the given paper. Returns SSE stream."""
return StreamingResponse(
wrap_generator(
_generate_stream(request),
workflow="p2c_generate",
),
media_type="text/event-stream",
headers={"Cache-Control": "no-cache", "Connection": "keep-alive"},
)


# ------------------------------------------------------------------ #
# GET / (list) #
# ------------------------------------------------------------------ #

@router.get("")
async def list_context_packs(
user_id: str = "default",
Comment on lines +189 to +190

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The list_context_packs endpoint accepts a user_id as a query parameter and uses it to filter context packs without any authorization check. This is a classic Insecure Direct Object Reference (IDOR) vulnerability, allowing any user to list context packs belonging to any other user by simply providing their user_id.

paper_id: Optional[str] = None,
project_id: Optional[str] = None,
limit: int = 20,
offset: int = 0,
):
"""List context packs for a user, with optional filters."""
items, total = _store.list_by_user(
user_id=user_id,
paper_id=paper_id,
project_id=project_id,
limit=limit,
offset=offset,
)
return {"items": items, "total": total}


# ------------------------------------------------------------------ #
# GET /{pack_id} #
# ------------------------------------------------------------------ #

@router.get("/{pack_id}")
async def get_context_pack(pack_id: str):
Comment on lines +211 to +212

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The get_context_pack endpoint retrieves a context pack by its pack_id from the URL path but fails to verify if the requesting user is the owner of that pack. This allows unauthorized users to access potentially sensitive research data and LLM-generated content in any context pack if they can obtain the pack_id.

"""Return the full context pack detail."""
pack = _store.get(pack_id)
if pack is None:
raise HTTPException(status_code=404, detail="Context pack not found.")
return pack


# ------------------------------------------------------------------ #
# POST /{pack_id}/session #
# ------------------------------------------------------------------ #

@router.post("/{pack_id}/session")
async def create_repro_session(pack_id: str, request: CreateSessionRequest):
Comment on lines +224 to +225

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The create_repro_session endpoint allows creating a reproduction session for any pack_id without verifying ownership. An attacker could use this to trigger session creation for other users' context packs and access the resulting roadmap and prompt data.

"""
Convert a context pack into a runbook session.

TODO: integrate with existing runbook creation once Module 1 is wired.
"""
pack = _store.get(pack_id)
if pack is None:
raise HTTPException(status_code=404, detail="Context pack not found.")

session_id = f"sess_{uuid.uuid4().hex[:16]}"
runbook_id = f"rb_{uuid.uuid4().hex[:12]}"

roadmap = pack.get("pack", {}).get("task_roadmap", [])
initial_steps = [
{
"step_id": f"S{i + 1}",
"title": cp.get("title", f"Step {i + 1}"),
"command": "",
"status": "pending",
}
for i, cp in enumerate(roadmap)
] or [{"step_id": "S1", "title": "Setup environment", "command": "", "status": "pending"}]

return {
"session_id": session_id,
"runbook_id": runbook_id,
"initial_steps": initial_steps,
"initial_prompt": (
f"Based on the reproduction context pack for paper {pack.get('paper_id', '')}, "

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The initial_prompt is constructed by embedding the user-supplied paper_id directly into a string. Since paper_id is not validated or sanitized, this is a potential source for prompt injection. If this prompt is subsequently processed by an LLM, a malicious paper_id could manipulate the LLM's behavior.

"implement the code step by step following the roadmap."
),
}


# ------------------------------------------------------------------ #
# DELETE /{pack_id} #
# ------------------------------------------------------------------ #

@router.delete("/{pack_id}")
async def delete_context_pack(pack_id: str):
Comment on lines +264 to +265

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The delete_context_pack endpoint allows any user to soft-delete any context pack by providing its pack_id. There is no verification that the requesting user owns the pack, leading to an IDOR vulnerability that allows unauthorized data deletion.

"""Soft-delete a context pack."""
deleted = _store.soft_delete(pack_id)
if not deleted:
raise HTTPException(status_code=404, detail="Context pack not found.")
return {"status": "deleted", "context_pack_id": pack_id}
Loading
Loading