v2.0 Autonomous SDLC — Phases 73-78#133
Draft
TinDang97 wants to merge 48 commits into
Draft
Conversation
…ayout.tsx
- Import Inter and Geist_Mono from next/font/google
- Add inter const with variable --font-inter (400, 600 weights)
- Add geistMono const with variable --font-geist-mono (400 weight)
- Add ${inter.variable} and ${geistMono.variable} to body className
- Update light mode themeColor from #ffffff to #f7f7f5
…ns + batch_run_issues - Add source_note_id UUID FK on issues → notes(id) ON DELETE SET NULL - Create batch_run_status enum (pending/running/paused/completed/failed) - Create batch_run_issue_status enum (pending/queued/running/completed/failed/cancelled) - Create batch_runs table with workspace_id, cycle_id, status, triggered_by_id, progress counters - Create batch_run_issues table with batch_run_id, issue_id, status, pr_url, cost_cents - RLS policies (workspace isolation + service_role bypass) for both new tables - Alembic chain: 109 → 110 (single head verified)
…bals.css - Update --font-sans to use var(--font-inter) as first preference - Update --font-mono to use var(--font-geist-mono) as first preference - Shift light-mode backgrounds: --background #ffffff→#f7f7f5, --background-subtle #f7f7f5→#f1f1ef, --background-muted #f1f1ef→#eae8e4 - Replace all 5 shadow classes from neutral hsl(0 0% 0%/...) to warm rgba(120, 100, 80, ...) - Dark theme and all other :root values unchanged
…d FK on Issue - batch_run.py: BatchRunStatus enum (5 values) + BatchRun model with cycle_id, triggered_by_id, status, progress counters, selectin relationships - batch_run_issue.py: BatchRunIssueStatus enum (6 values) + BatchRunIssue model with batch_run_id, issue_id, worktree_path, pr_url, cost_cents, selectin relationships - issue.py: Add source_note_id UUID FK → notes.id ON DELETE SET NULL with index and source_note selectin relationship; Note added to TYPE_CHECKING imports - __init__.py: Export BatchRun, BatchRunStatus, BatchRunIssue, BatchRunIssueStatus - pyright: 0 errors on new model files
…PI schemas - Add acceptance_criteria and source_note_id to IssueCreateRequest - Add acceptance_criteria, source_note_id, clear_source_note to IssueUpdateRequest - Add acceptance_criteria and source_note_id to IssueResponse - Wire both fields in IssueResponse.from_issue() mapping from ORM model
…e services - Add acceptance_criteria and source_note_id to CreateIssuePayload dataclass - Pass both fields to Issue() constructor in CreateIssueService.execute() - Add source_note_id and clear_source_note to UpdateIssuePayload dataclass - Add source_note_id update logic with clear_source_note flag in UpdateIssueService - Fix test_issue_response_note_links to set acceptance_criteria and source_note_id on mock
- Add KGDecision, RelatedPR, SprintPeer schema classes to implement_context.py - Add cycle_id field to IssueDetail with from_issue() population - Extend ImplementContextResponse with kg_decisions, related_prs, sprint_peers, context_budget_used_pct fields (all with defaults for backward compatibility) - Add IntegrationLinkRepository.get_pull_requests_for_issues() batch method - Update __all__ exports to include new schema classes
- Tests cover KG decisions, related PRs, sprint peers enrichment - Tests cover graceful degradation when enrichment fails - Tests cover budget manager truncation order - Tests cover TTL cache deduplication - Tests cover sprint peer exclusion of target issue - Tests cover 5 PR limit and 10 sprint peer limit
…er and TTL cache - Create rich_context_assembler.py with RichContextAssembler class - Class-level TTLCache with 1-hour TTL and 128 max size - Cache key is (issue_id, workspace_id) tuple; second call returns from_cache=True - _recall_kg_context: multi-query (title + description + labels), deduplicates by node_id, limits to 10 - _fetch_related_prs: traverses issue links, filters to done states, queries PR URLs, limits to 5 - _fetch_sprint_peers: checks cycle_id, excludes target issue, limits to 10 - Budget manager: 480k char budget, truncates sprint_peers then related_prs then kg_decisions - asyncio.gather with return_exceptions=True for all concurrent calls - Graceful degradation: failed enrichments log warning and return empty list - Deferred ImplementContextResponse import to avoid circular dependency - Export RichContextAssembler, RichContextPayload, RichContextResult from __init__.py
- Register RichContextAssembler as providers.Factory in container.py (after memory_recall_service) - Add RichContextAssemblerDep to dependencies_pilot.py and __all__ - Swap GetImplementContextServiceDep -> RichContextAssemblerDep in issue_implement.py router - Swap GetImplementContextPayload -> RichContextPayload in router - Extend _normalize_ctx to extract kgDecisions/relatedPrs/sprintPeers from API response - Pass kg_decisions, related_prs, sprint_peers to template.render() in _inject_claude_md - Add 3 conditional Jinja2 sections to CLAUDE_MD_TEMPLATE.md before '## Your Task'
- CTX-01: KG decisions included / empty graceful (2 tests) - CTX-02: Related PRs included / empty graceful (2 tests) - CTX-03: Budget truncation priority + pct reporting (2 tests) - CTX-04: Sprint peers included / no cycle (2 tests) - CTX-05: Base fields unchanged + KG failure degradation (2 tests) - All dependencies mocked, TTLCache cleared between tests
Fixes 9 pre-existing test failures caused by MagicMock returning MagicMock for new nullable fields, which fails Pydantic UUID validation.
…oposal - Add 'issue_batch_proposal' to SSEEventType union in events.ts - Add ProposedIssue and IssueBatchProposalEvent interfaces - Add isIssueBatchProposalEvent type guard in event-guards.ts - Extend ChatMessage with batchProposal field in conversation.ts - Add currentAssistantMessage getter to PilotSpaceStore - Dispatch issue_batch_proposal events to ChatMessage.batchProposal in stream handler
…mission
- Add _format_batch_sse() module-level SSE formatter
- Add _call_llm_for_issues() for structured issue generation via LLM
- Add generate_issues_from_description tool to create_issue_tools_server()
- Emits issue_batch_proposal SSE event with issues, sourceNoteId, projectId
- Handles null note_id gracefully (chatting from homepage)
- Each issue has acceptance_criteria as [{criterion, met}] list
- All 6 tests pass
… approval gate
- IssuePreviewItem: collapsible per-issue card (React.memo, NOT observer)
- Priority badge, editable title/description, AC checklist, priority selector
- Remove button with 150ms fade-out, aria-expanded/aria-controls, 44px touch target
- BatchPreviewCard: stacked collapsible list with edit/remove/create-all
- React.memo + useState only, NO observer()
- DD-003 gate: 'Create All' calls onCreateAll which routes through requestApproval
- Empty batch shows 'No issues could be extracted' (locked decision copy)
- Created/creating/error states with proper UI feedback
- motion/react 300ms slide-up + fade, respects prefers-reduced-motion
- AssistantMessage: BatchPreviewCardInline wrapper wires store.requestApproval()
- DD-003: approval callback fires issuesApi.createBatch only after PM confirms
- Render branch: message.batchProposal && <BatchPreviewCardInline>
- PilotSpaceStore: requestApproval() + executeLocalApprovalCallback() for local DD-003 gate
- PilotSpaceActions: local approval bypass (requestId starts with 'local_')
- issuesApi.createBatch(): POST /workspaces/{slug}/issues/batch
- 13 unit tests pass (BatchPreviewCard.test.tsx)
…dling
- Add BatchCreateIssuesService with per-item exception catching (CIP-05)
- Add BatchCreateIssueRequest/Response schemas using BaseSchema for camelCase
- Add batch_create_issues_service DI registration in container.py
- Add BatchCreateIssuesServiceDep to dependencies.py
- Add POST /{workspace_id}/issues/batch endpoint to workspace_issues router
- Each issue gets acceptance_criteria and source_note_id propagated
- 6 service-layer unit tests cover all behaviors including partial failure
…Block access - Guard hasattr(first_block, 'text') before accessing .text attribute - Avoids union-attr pyright error for non-TextBlock content types
…mponent - Test 1: renders nothing when totalTokens is 0 - Test 2: teal (--primary) fill when usage < 80% - Test 3: amber (--warning) fill when usage 80-95% - Test 4: red (--destructive) fill when usage > 95% - Test 5: center label formatted (4.2k for 4200) - Test 6: role=progressbar with aria attributes - Test 7: tooltip with token count and session limit warning
- React.memo pure component, 32px diameter, radius 13 stroke-width 3 - Color thresholds: primary (<80%), warning (80-95%), destructive (>=95%) - Center label: formatted as '4.2k' for >=1000 tokens, raw below - Tooltip: locale-formatted count and approaching-limit copy when >=80% - role=progressbar aria-valuenow/min/max for accessibility - Hidden (returns null) when totalTokens is 0 or falsy - 400ms ease-out CSS transition on stroke-dashoffset (motion-reduce:transition-none)
…riant, warm palette on ChatView
- ChatHeader: import and render TokenRing between ModelSelector and action buttons
- ChatHeader: add totalTokens prop (number, default 0) wired from store.sessionState.totalTokens
- ChatView: pass totalTokens={store.sessionState?.totalTokens ?? 0} to ChatHeader
- ToolCallCard: add variant prop ('default' | 'card'), default unchanged
- ToolCallCard variant='card': bg-ai-bg border border-ai-border rounded-md px-3 py-2
- ChatView root already uses bg-background (#f7f7f5 warm palette from Phase 73)
…nCard, RecentConversationCard - Full rewrite of HomepageHub: chat-first hero, 2x2 quick action grid, recent conversations scroll - New ChatHeroInput: 56px warm input, navigates to /chat?prefill=..., submit on Enter or button click - New QuickActionCard: 4 cards with exact UI-SPEC copy, keyboard accessible, hover transitions - New RecentConversationCard: 200x72px fixed card, relative timestamp, navigates to /chat?session=... - Removed DailyBrief+ChatView 2-panel layout; buildContextualPrompts retained for API compatibility - 20 tests passing (9 new + 11 pre-existing)
- Add migration 111: nullable current_stage TEXT column on batch_run_issues - Add current_stage Mapped[str | None] field to BatchRunIssue model - Add QueueName.BATCH_IMPL = 'batch_impl' for downstream plans 02 + 03 - Alembic chain: 111 correctly revises 110 (single head verified)
…ical sort - Add BatchRunError, BatchRunCycleDetectedError, BatchRunNotFoundError to domain/exceptions.py - BatchRunRepository: get_by_id_with_items, get_batch_run_issues, get_dispatchable_issues (dependency gate), update_issue_status, cancel_pending_issues, increment_completed/failed - BatchRunService: kahn_topological_sort pure function + create_batch_run, cancel_batch_run, cancel_issue, get_dag_preview - First-wave issues (execution_order=0) set to QUEUED; rest stay PENDING - 15 unit tests pass: linear chain, parallel tracks, diamond, cycles, cancel cascade
- Register BatchRunRepository as providers.Factory in Container - Register BatchRunService as providers.Factory in Container - Add BatchRunServiceDep type alias to dependencies.py - Export BatchRunServiceDep in __all__ list
…tion
- Create BatchImplHandler managing subprocess exec, stdout streaming, and PR URL regex
- 30-minute timeout with SIGTERM + SIGKILL fallback
- Failure cascade via cancel_pending_issues for dependent issues
- Redis pub/sub status events on batch_runs:{batch_run_id} channel
- Unit tests: PR regex, timeout kill, failure cascade, success with PR URL, active_procs cleanup
…rtbeat - Create BatchImplWorker polling pgmq batch_impl queue for sprint batch triggers - asyncio.Semaphore(3) limits concurrent pilot implement subprocesses - VT heartbeat task extends pgmq visibility every 90s for 30-minute jobs - Dispatch loop polls get_dispatchable_issues every 3s until all terminal - cancel_issue() and cancel_batch() send SIGTERM via _active_procs dict - RLS context set per session using actor_user_id from BatchRun - pyright passes with 0 errors
- POST /workspaces/{id}/batch-runs — creates BatchRun + enqueues BATCH_IMPL trigger
- GET /workspaces/{id}/batch-runs/{id} — returns full status with per-issue details
- GET /workspaces/{id}/batch-runs/preview/{cycle_id} — DAG preview without creation
- GET /workspaces/{id}/batch-runs/{id}/stream — SSE via Redis pub/sub fan-out
- POST /workspaces/{id}/batch-runs/{id}/cancel — cancel + publish batch_cancelled event
- POST /workspaces/{id}/batch-runs/{id}/issues/{id}/cancel — cancel single issue
- All schemas use BaseSchema (camelCase), all routes have session: DbSession
- Register batch_runs_router in __init__.py and main.py
…t_sprint
- preview_sprint_implementation: returns DAG preview (execution_order, parallel tracks, cycle warnings) for PM review before approval
- implement_sprint: creates BatchRun, commits, enqueues BATCH_IMPL pgmq trigger (non-fatal if queue unavailable)
- Both tools use @register_tool('issue') pattern consistent with existing ai/tools/ directory
- Follows DD-003 approval gate: preview must be called before implement_sprint
…SSE hook, cancel buttons
- ImplementationStatusBadge: 7 statuses with color tokens, motion-safe pulse for 'implementing'
- SprintDAGPreview: accessible HTML table with caption, scope=col, parallel grouping
- useBatchRunStream: EventSource SSE hook with TanStack Query invalidation on events
- useBatchRun: TanStack Query hooks for batch run CRUD and preview
- CancelBatchButton: Dialog confirmation before POST /batch-runs/{id}/cancel
- CancelIssueButton: Radix Popover inline confirm for single-issue cancel
- All components use React.memo, NOT observer() — safe for TipTap context
- Export BatchImplWorker from ai/workers/__init__.py - Create batch_impl pgmq queue on startup alongside other queues - Start BatchImplWorker alongside MemoryWorker and NotificationWorker (raw Redis guard) - Shutdown: call batch_impl_worker.stop() + cancel task for graceful termination - pilot CLI availability checked at startup with warning (non-fatal)
…ogressPanel - PropertyBlockView: Implementation row (plain fn, NOT observer) showing status badge + PR link - ActivityTimeline: routes implementation_started/pr_created/implementation_failed to BatchRunActivityEntry - BatchRunActivityEntry: new activity entry variant for 3 implementation event types - BatchRunProgressPanel: real-time progress bar + per-issue list + cancel controls via SSE
- Button visible only for active cycles, min 44px touch target - Dialog opens SprintDAGPreview for PM to review execution order before approving - useBatchRunPreview fetches DAG on dialog open (lazy, only when needed) - useCreateBatchRun mutation starts batch run on approval with sonner toast feedback - Loading skeleton while preview fetches; error toast on start failure
- Add get_dashboard_data() to BatchRunRepository with selectinload of items + issues (no N+1)
- Add DashboardIssueStatus, AttentionItemResponse, DashboardResponse schemas to batch_runs router
- Add GET /workspaces/{workspace_id}/batch-runs/{batch_run_id}/dashboard endpoint
- Dashboard returns: progress stats (DSH-01), per-issue statuses with cost_cents (DSH-02),
attention feed with pr_ready/blocked/pending_approval items (DSH-03), sprint cost total (DSH-04)
- Bring in Phase 76 batch run foundation files (batch models, service, worker, handler, DI wiring)
- Create frontend/src/features/dashboard/types.ts with DashboardData, DashboardIssueStatus, AttentionItem, IssueCostEntry interfaces - Create frontend/src/features/dashboard/hooks/use-dashboard.ts with useDashboard hook polling every 30s for active sprints - Bring in Phase 76 frontend files: implementation-status-badge, use-batch-run, use-batch-run-stream (needed for ImplementationStatus type import) - TypeScript passes with no errors
Both phases modify batch_runs.py and batch_run_repository.py. Phase 77 adds dashboard schemas + endpoint, Phase 76 had the base. Accept both.
…ssue cards, ETA - SprintProgressRing: SVG circle with stroke-dashoffset animation, Fraunces font, ARIA progressbar - StatusStatCard: color-coded left border via CSS var token, read-only count display - LiveIssueCard: status-driven border color, ImplementationStatusBadge, stage text fallback - EtaDisplay: average-duration ETA computation, Tooltip, returns null when sprint complete - All 4 components use React.memo, zero observer imports
- AttentionItem: accessible clickable row using <a> for PR, Link for internal nav - AttentionFeedSection: 3 categories (PR/Blocked/Approvals), 'None right now' empty per copywriting - CostBreakdownPanel: Geist Mono cost figures, per-issue table sorted by cost desc, aria-label - DashboardPage: 12-col grid layout, loading/error/empty states, SSE cache invalidation - Sidebar: Dashboard nav item with BarChart3 icon, dashboardAttention badge key - Zero observer imports across all 7 new dashboard components
…import - attention-feed-section.tsx: local variable for bucket prevents TS2532 - batch_runs.py: map 'completed' → 'done' to match frontend ImplementationStatus - batch_run_repository.py: remove unused UTC import
…nd spec-annotations endpoints
- Migration 112: add spec_annotations JSONB column (server_default '[]'::jsonb)
- Note model: spec_annotations Mapped column (avoids collision with annotations relationship)
- NoteRepository: get_linked_issues() query with state + batch status; append_spec_annotation()
- Schemas: LinkedIssueResponse + SpecAnnotationResponse for Living Specs sidebar
- Router: GET /notes/{id}/linked-issues and GET /notes/{id}/spec-annotations
- useLivingSpec: TanStack Query, 30s poll, linked-issues endpoint - useSpecAnnotations: TanStack Query, 30s poll, spec-annotations endpoint - useTocHeadings: extracts h1/h2/h3 from TipTap editor JSON via update event - IssueStatusRow: identifier + truncated title tooltip + batchStatus/state badge - LinkedIssuesPanel: collapsible, ScrollArea when >8 items, empty state - DeviationCard: amber left border, AlertTriangle icon, RFC-formatted timestamp - DecisionLogCard: dusty-blue border, CheckCircle2/XCircle, Geist Mono timestamp - AnnotationsPanel: sorted desc, role=feed, aria-busy, error + empty states - TableOfContentsPanel: hidden when <3 headings, nav aria-label, scroll-to-heading - TocItem: <a> anchor, indentation by level, aria-current=location when active - notesKeys extended with linkedIssues + specAnnotations keys - notesApi extended with getLinkedIssues + getSpecAnnotations methods - All components: React.memo (NOT observer) per TipTap CLAUDE.md constraint
…_worker dispatch - DeviationAnalysisHandler: compares PR vs source note via Haiku model, appends deviation annotation - memory_worker: adds TASK_DEVIATION_ANALYSIS dispatch routing to DeviationAnalysisHandler - batch_impl_handler: enqueues deviation_analysis to AI_NORMAL after PR success (non-fatal) - batch_create_issues_service: appends decision record to spec_annotations on batch approval - BatchImplHandler: accepts optional queue_client for deviation_analysis enqueue
…tion - LivingSpecSidebar: 280px collapsible right panel, React.memo NOT observer - Toggle button: 24x24px ghost, ChevronLeft/Right, aria-expanded + aria-controls - Tooltip: "Collapse spec panel" / "View living spec" per UI-SPEC Copywriting Contract - role=complementary, aria-label=Living spec, aria-hidden when collapsed - motion-safe:transition-all duration-200 ease-out on width change - SidebarContent: separated into inner component so hooks only run when open - Active heading tracking via scroll position (IntersectionObserver-free) - NoteDetailPage: flex row layout — NoteCanvas (flex-1) + sidebar sibling - Sidebar default: open on lg (1024px+), collapsed on md, hidden sm via hidden md:flex - bg-[#f7f7f5] warm palette applied to outer container - Version history panel takes precedence (sidebar returns null when showVersionHistory)
Without queue_client, the guard `if pr_url and self._queue is not None` was permanently False, so deviation analysis jobs were never enqueued.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a sprint batch implementation feature: DB migrations and models for batch runs/issues and spec annotations; backend services, repositories, queue handlers, and a worker to orchestrate parallel issue implementation; new APIs (including SSE), MCP tools for batch proposals, and comprehensive frontend dashboard and living-spec UI with related tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Frontend (Cycles Page)
participant API as API Server
participant Service as BatchRunService
participant Repo as BatchRunRepository
participant DB as Database
participant Queue as Queue (pgmq)
participant Worker as BatchImplWorker
User->>Frontend: Click "Implement Sprint"
Frontend->>API: POST /workspaces/{ws}/batch-runs
API->>Service: create_batch_run(payload)
Service->>Repo: load cycle issues & blocks links
Service->>Service: kahn_topological_sort
Service->>DB: insert BatchRun + BatchRunIssue rows (PENDING/QUEUED)
Service-->>API: Return BatchRunResponse
API->>Queue: Enqueue BATCH_IMPL trigger
Frontend->>Frontend: Subscribe SSE / open dashboard stream
Queue-->>Worker: Deliver BATCH_IMPL message
Worker->>Repo: set batch run status RUNNING
Worker->>Worker: dispatch dispatchable issues (concurrency limit)
loop per issue
Worker->>Worker: spawn subprocess (pilot implement ...)
Worker->>Worker: read stdout, extract PR URL
Worker->>Repo: update BatchRunIssue status/fields
Worker->>API: publish Redis SSE update
Frontend->>Frontend: receive SSE, refresh UI
end
Worker->>Repo: finalize batch run status (COMPLETED/FAILED)
Worker->>API: publish final SSE event
sequenceDiagram
participant Assembler as RichContextAssembler
participant Base as GetImplementContextService
participant Memory as MemoryRecallService
participant IssueRepo as IssueLinkRepository
participant Integrations as IntegrationLinkRepository
participant CycleRepo as CycleRepository
participant Cache as TTL Cache
Assembler->>Cache: lookup (issue_id, workspace_id)
alt cache hit
Cache-->>Assembler: return cached result (from_cache=true)
else cache miss
Assembler->>Base: execute(payload)
par concurrent enrichment
Assembler->>Memory: multi_recall -> KG items
Assembler->>IssueRepo: linked completed issues
Assembler->>Integrations: pull requests for issues
Assembler->>CycleRepo: sprint peers
end
Assembler->>Assembler: enforce char budget, truncate enrichments
Assembler->>Cache: store result (1h TTL)
Assembler-->>Caller: enriched result (from_cache=false)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Phase 72 migration 109 added append-only RLS on audit_log, which blocks CASCADE deletes from workspace. Truncate audit_log first during re-seed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/pilot_space/infrastructure/database/models/note.py (1)
7-27:⚠️ Potential issue | 🟠 MajorCI blocker: import block must be sorted/formatted.
This file is failing
I001in CI. Please normalize the import block ordering/formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/infrastructure/database/models/note.py` around lines 7 - 27, The import block is not normalized, causing I001 CI failure; reorder and format imports into standard groups (standard library, third-party, local application) and ensure alphabetical order within each group and proper blank lines between groups. Specifically, group "uuid" and "typing" (standard lib) first, then "sqlalchemy" and "sqlalchemy.dialects.postgresql" and "sqlalchemy.orm" (third-party), and finally local imports like WorkspaceScopedModel and JSONBCompat from pilot_space.infrastructure.database; also collapse multiline sqlalchemy imports consistently and remove extra blank lines so the top-level names (UUID, Mapped, mapped_column, relationship, Boolean, CheckConstraint, ForeignKey, Index, Integer, String, Text, text) are ordered alphabetically within their group.
🟡 Minor comments (25)
frontend/src/features/homepage/components/ChatHeroInput.tsx-63-63 (1)
63-63:⚠️ Potential issue | 🟡 MinorAdd a fallback for Firefox and older browsers that lack
field-sizing: contentsupport.The
field-sizing: contentCSS property is unsupported in Firefox (all versions through 152) and older browsers. Although Chrome has supported it since v123 and Safari since 26.2, Firefox users will not benefit from auto-sizing. Implement a fallback usingmin-widthandmax-widthconstraints instead of relying solely onfield-sizing:Suggested approach
style={{ fieldSizing: 'content', minWidth: '7ch', maxWidth: '100%' } as React.CSSProperties}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/homepage/components/ChatHeroInput.tsx` at line 63, Update the inline style on the ChatHeroInput component where it currently sets style={{ fieldSizing: 'content' } as React.CSSProperties} to include fallbacks for browsers that don't support field-sizing: add minWidth (e.g., '7ch') and maxWidth (e.g., '100%') alongside fieldSizing so the input auto-sizes in Chrome/Safari but has sensible constraints in Firefox/older browsers; keep the existing fieldSizing property and cast to React.CSSProperties as before.frontend/src/features/issues/components/batch-run-activity-entry.tsx-35-51 (1)
35-51:⚠️ Potential issue | 🟡 MinorHarden
formatRelativeTimeagainst invalid timestamps.At Line 37, invalid date input yields
NaN, and the current path can render malformed text. Add a finite check and clamp negative diffs.Suggested fix
function formatRelativeTime(dateStr: string): string { - const now = Date.now(); const then = new Date(dateStr).getTime(); - const diffMs = now - then; + if (!Number.isFinite(then)) return 'Unknown time'; + const now = Date.now(); + const diffMs = Math.max(0, now - then); const diffMin = Math.floor(diffMs / 60_000); const diffHr = Math.floor(diffMs / 3_600_000); const diffDay = Math.floor(diffMs / 86_400_000); @@ - const date = new Date(dateStr); + const date = new Date(then); const months = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']; return `${months[date.getMonth()]} ${date.getDate()}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/issues/components/batch-run-activity-entry.tsx` around lines 35 - 51, The formatRelativeTime function doesn't guard against invalid date strings — new Date(dateStr).getTime() can be NaN or produce a negative diff; update formatRelativeTime to validate the parsed timestamp (e.g., const then = new Date(dateStr).getTime(); if (!isFinite(then)) return a safe fallback like 'just now' or a formatted date), clamp diffMs to a minimum of 0 before computing diffMin/diffHr/diffDay, and ensure all downstream calculations (diffMin, diffHr, diffDay) operate on finite numbers so the component renders predictable text.frontend/src/features/dashboard/components/status-stat-card.tsx-36-39 (1)
36-39:⚠️ Potential issue | 🟡 MinorExpose the count to assistive tech directly.
Hiding the numeric value with
aria-hiddencan make the announced value depend on label behavior. Keep the count readable for screen readers to ensure consistent output.Suggested fix
- <p - className="text-[20px] font-semibold leading-tight text-foreground" - aria-hidden="true" - > + <p className="text-[20px] font-semibold leading-tight text-foreground"> {count} </p>As per coding guidelines, "Ensure accessibility is WCAG 2.2 AA baseline. Never use color as sole meaning carrier. Implement keyboard-first interaction patterns".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/dashboard/components/status-stat-card.tsx` around lines 36 - 39, The numeric count in the status-stat-card component is currently hidden from screen readers via aria-hidden on the <p className="text-[20px] font-semibold leading-tight text-foreground"> element; remove aria-hidden (or set it to false) so the count is exposed directly to assistive tech and, if needed, add a clear aria-label on that same <p> (or a wrapping element) that includes the count plus context to ensure consistent announcements for screen readers.frontend/src/stores/ai/PilotSpaceActions.ts-210-230 (1)
210-230:⚠️ Potential issue | 🟡 MinorLocal approval falls through to backend if callback is missing.
When
requestId.startsWith('local_')butexecuteLocalApprovalCallbackreturnsfalse(callback not found), the code falls through to the backend API call at line 232. This will result in a 404 error since backend has no record oflocal_*request IDs.Consider returning early with an error when the local callback isn't found:
🔧 Proposed fix
if (requestId.startsWith('local_')) { try { const executed = await this.store.executeLocalApprovalCallback(requestId); if (executed) { runInAction(() => { this.store.pendingApprovals = this.store.pendingApprovals.filter( (r) => r.requestId !== requestId ); }); return; } + // Callback not found for local_ request — programming error, don't fall through to backend + console.error(`Local approval callback not found for ${requestId}`); + runInAction(() => { + this.store.error = 'Approval action not found'; + }); + return; } catch (err) { runInAction(() => { this.store.error = err instanceof Error ? err.message : 'Failed to execute action'; }); return; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/ai/PilotSpaceActions.ts` around lines 210 - 230, When requestId.startsWith('local_') but this.store.executeLocalApprovalCallback(requestId) returns false, the code currently falls through to the backend call causing a 404; change the flow in PilotSpaceActions so that if executed is false you set this.store.error to a clear message (e.g. "Local approval callback not found for {requestId}"), optionally remove any stale entry from this.store.pendingApprovals, and return early instead of continuing to the backend API call; update the try block around executeLocalApprovalCallback and the surrounding logic to handle the false case by setting the error and returning.backend/src/pilot_space/api/v1/dependencies.py-72-75 (1)
72-75:⚠️ Potential issue | 🟡 MinorImport block ordering needs cleanup to pass CI.
This file is failing
I001(unsorted/unformatted imports). Please apply the repo’s import formatting to the full header block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/api/v1/dependencies.py` around lines 72 - 75, The import block in dependencies.py is not sorted per the repo style (I001); reorder the header imports into the proper groups (standard library, third-party, local application) and alphabetize within each group, ensuring multi-line imports use parentheses and that local imports like BatchRunService and BatchCreateIssuesService are placed in the local/application group in alphabetical order; run the project's isort/formatter or manually adjust the import lines so they conform to the repo import formatting.backend/src/pilot_space/container/container.py-80-85 (1)
80-85:⚠️ Potential issue | 🟡 MinorFix import ordering in this module to unblock CI.
The current import block fails
I001in CI. Please run the project’s import sorter/formatter for this file and commit the normalized order.Also applies to: 188-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/container/container.py` around lines 80 - 85, The import block in container.py is failing the I001 import-order check; reorder or run the project's import sorter/formatter on this file so imports are normalized. Specifically adjust the grouped imports that include BatchRunService, BatchCreateIssuesService, RichContextAssembler, and McpOAuthService to follow the project's import ordering rules (standard/library, third-party, first-party) and apply the same normalization to the other import block referenced (the block that contains the same or similar imports further down). After sorting, run the linter/formatter locally and commit the updated file.backend/tests/infrastructure/queue/handlers/test_batch_impl_handler.py-99-110 (1)
99-110:⚠️ Potential issue | 🟡 MinorFix ARG005: Rename unused
selfto_in lambda.The
selfparameter is required by the__aiter__protocol but unused. Rename to_to suppress the linter warning.🔧 Proposed fix
- proc.stdout.__aiter__ = lambda self: _aiter_stdout() + proc.stdout.__aiter__ = lambda _: _aiter_stdout()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/infrastructure/queue/handlers/test_batch_impl_handler.py` around lines 99 - 110, The lambda assigned to proc.stdout.__aiter__ currently declares an unused parameter named self which triggers ARG005; change the lambda parameter name from self to _ (e.g., proc.stdout.__aiter__ = lambda _: _aiter_stdout()) so it still satisfies the __aiter__ protocol but avoids the unused-argument linter warning; update the assignment in test_batch_impl_handler where proc.stdout.__aiter__ is set to ensure it returns the _aiter_stdout async generator.backend/tests/ai/mcp/test_issue_server_generate.py-120-132 (1)
120-132:⚠️ Potential issue | 🟡 MinorFix PT001: Remove empty parentheses from
@pytest.fixture.Per pytest conventions, use
@pytest.fixturewithout parentheses when no arguments are needed.🔧 Proposed fix
-@pytest.fixture() +@pytest.fixture def publisher_and_queue() -> tuple[EventPublisher, asyncio.Queue[str]]: return _make_publisher() -@pytest.fixture() +@pytest.fixture def tool_context_with_note() -> ToolContext: return _make_tool_context(note_id=str(uuid4())) -@pytest.fixture() +@pytest.fixture def tool_context_no_note() -> ToolContext: return _make_tool_context(note_id=None)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/ai/mcp/test_issue_server_generate.py` around lines 120 - 132, The fixtures publisher_and_queue, tool_context_with_note, and tool_context_no_note use `@pytest.fixture`() with empty parentheses; change each decorator to `@pytest.fixture` (remove the empty parentheses) so they follow pytest convention when no arguments are provided, i.e., update the decorators above the functions publisher_and_queue, tool_context_with_note, and tool_context_no_note accordingly.backend/tests/infrastructure/queue/handlers/test_batch_impl_handler.py-128-132 (1)
128-132:⚠️ Potential issue | 🟡 MinorFix ARG005: Same unused
selfissue in blocking iterator lambda.🔧 Proposed fix
- proc.stdout.__aiter__ = lambda self: _blocking_aiter() + proc.stdout.__aiter__ = lambda _: _blocking_aiter()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/infrastructure/queue/handlers/test_batch_impl_handler.py` around lines 128 - 132, The lambda assigned to proc.stdout.__aiter__ declares an unused parameter "self" (ARG005); replace it with a no-arg callable or assign the async generator directly. For example, change proc.stdout.__aiter__ = lambda self: _blocking_aiter() to either proc.stdout.__aiter__ = lambda: _blocking_aiter() or simply proc.stdout.__aiter__ = _blocking_aiter to remove the unused "self" parameter (refer to _blocking_aiter and proc.stdout.__aiter__).backend/tests/ai/mcp/test_issue_server_generate.py-14-26 (1)
14-26:⚠️ Potential issue | 🟡 MinorFix pipeline failures: import sorting and unused import.
The CI pipeline flagged several issues in this block:
- I001: Imports are unsorted. Reorder per
isortconventions.- F401:
UUID(Line 20) is imported but never used — onlyuuid4is needed.🔧 Proposed fix
from __future__ import annotations import asyncio import json from typing import Any -from unittest.mock import AsyncMock, MagicMock, patch -from uuid import UUID, uuid4 +from unittest.mock import AsyncMock, MagicMock +from uuid import uuid4 import pytest from pilot_space.ai.mcp.event_publisher import EventPublisher from pilot_space.ai.mcp.issue_server import create_issue_tools_server from pilot_space.ai.tools.mcp_server import ToolContext🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/ai/mcp/test_issue_server_generate.py` around lines 14 - 26, Reorder and clean up the imports at the top of the test module to satisfy isort and remove the unused symbol: keep "from __future__ import annotations" first, then standard-library imports (asyncio, json, typing.Any, and import only uuid4 from uuid—remove the unused UUID), then third-party imports (pytest, unittest.mock symbols AsyncMock, MagicMock, patch), and finally local package imports (EventPublisher, create_issue_tools_server, ToolContext); ensure there are blank-line group separations between these groups and no leftover unused imports.backend/tests/infrastructure/queue/handlers/test_batch_impl_handler.py-9-20 (1)
9-20:⚠️ Potential issue | 🟡 MinorFix pipeline failure: Import block unsorted.
The CI pipeline flagged I001 for unsorted imports.
🔧 Proposed fix
from __future__ import annotations import asyncio -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock from uuid import uuid4 import pytest from pilot_space.infrastructure.queue.handlers.batch_impl_handler import ( + PR_URL_PATTERN, BatchImplHandler, - PR_URL_PATTERN, )Note: The exact ordering depends on your
isortconfiguration. Runisortto auto-fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/infrastructure/queue/handlers/test_batch_impl_handler.py` around lines 9 - 20, The import block is unsorted causing I001; reorder the imports so __future__ stays first, then standard library imports (asyncio, uuid.uuid4, unittest.mock names: AsyncMock, MagicMock, patch), then third-party imports (pytest), and finally the local module import that brings in PR_URL_PATTERN and BatchImplHandler; run isort or manually reorder the import lines accordingly to satisfy the linter.frontend/src/features/dashboard/hooks/use-dashboard.ts-22-26 (1)
22-26:⚠️ Potential issue | 🟡 MinorFix the API endpoint URL to include the workspace slug.
The hook accepts
workspaceSlugas a parameter and uses it in theenabledcondition, but the actual API call on line 25 constructs/batch-runs/${batchRunId}/dashboardwithout the workspace prefix. This deviates from the documented endpoint (GET /workspaces/{workspace_id}/batch-runs/{batch_run_id}/dashboard) in the JSDoc comment and from the codebase pattern where all workspace-scoped endpoints include the workspace identifier.🔧 Fix
- queryFn: () => apiClient.get<DashboardData>(`/batch-runs/${batchRunId}/dashboard`), + queryFn: () => apiClient.get<DashboardData>(`/workspaces/${workspaceSlug}/batch-runs/${batchRunId}/dashboard`),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/dashboard/hooks/use-dashboard.ts` around lines 22 - 26, The API call in useDashboard uses the wrong endpoint: update the queryFn in useDashboard so apiClient.get is called against the workspace-scoped endpoint (e.g., `/workspaces/${workspaceSlug}/batch-runs/${batchRunId}/dashboard`) instead of `/batch-runs/${batchRunId}/dashboard`; keep the existing enabled guard (enabled: !!workspaceSlug && !!batchRunId) so batchRunId is non-null when the request runs, and leave the queryKey generation via dashboardKeys.detail(batchRunId) unchanged.frontend/src/features/issues/hooks/use-batch-run-stream.ts-38-48 (1)
38-48:⚠️ Potential issue | 🟡 MinorFix ESLint
react-hooks/set-state-in-effecterror and remove unused ref.Two issues flagged by pipeline and review:
- Line 44:
setConnectionError(null)triggers ESLint warning. Wrap in a condition or defer to avoid synchronous setState at effect start.- Lines 38, 48, 75:
eventSourceRefis written but never read — it's dead code.🔧 Proposed fix
- const eventSourceRef = useRef<EventSource | null>(null); - useEffect(() => { if (!batchRunId) return; - // Clear any previous error when reconnecting - setConnectionError(null); - const url = `${API_BASE}/batch-runs/${batchRunId}/stream`; const eventSource = new EventSource(url, { withCredentials: true }); - eventSourceRef.current = eventSource; + + // Clear any previous error when reconnecting (deferred to avoid sync setState) + queueMicrotask(() => setConnectionError(null)); eventSource.addEventListener('batch_status_update', (event: MessageEvent) => { // ... existing code }); eventSource.onerror = () => { setConnectionError('Live updates paused. Refresh to reconnect.'); }; return () => { eventSource.close(); - eventSourceRef.current = null; }; }, [batchRunId, queryClient]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/issues/hooks/use-batch-run-stream.ts` around lines 38 - 48, The useEffect in useBatchRunStream is causing a react-hooks/set-state-in-effect lint error and holds an unused ref; fix by changing the unconditional setConnectionError(null) to a guarded update (e.g., if (connectionError !== null) setConnectionError(null)) inside the effect so state isn't set every render, and remove the dead eventSourceRef useRef by using a local const eventSource = new EventSource(...) and referencing that local in the cleanup (eventSource.close()) and anywhere else currently writing to eventSourceRef; update useBatchRunStream to delete eventSourceRef declaration and all assignments/reads of eventSourceRef while keeping the same EventSource creation and cleanup logic.frontend/src/features/notes/components/living-spec/decision-log-card.tsx-42-44 (1)
42-44:⚠️ Potential issue | 🟡 MinorDon't map every non-
approvedaction toRejected.
actionis nullable here, sonullor any unexpected value will render as a rejection in the audit trail. Handle'rejected'explicitly and fall back to a neutral label/icon for unknown states.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/notes/components/living-spec/decision-log-card.tsx` around lines 42 - 44, The current logic maps any non-'approved' value (including null) to "Rejected"; update the decision-log rendering to explicitly check for 'approved' and 'rejected' instead of treating every other value as rejected: keep const isApproved = action === 'approved'; add const isRejected = action === 'rejected'; compute actionLabel (and corresponding icon/state) to be 'Approved' when isApproved, 'Rejected' when isRejected, and a neutral fallback label/icon (e.g., 'Pending' or 'Unknown') when action is null or any other unexpected value so the audit trail isn't misleading.frontend/src/features/dashboard/components/sprint-progress-ring.tsx-67-71 (1)
67-71:⚠️ Potential issue | 🟡 MinorThe ring still animates for reduced-motion users.
The
motion-safeclass is on the<svg>, but the actual animation lives in the inlinetransitionon this<circle>, soprefers-reduced-motionis ignored. Move the transition to amotion-safe:class (with amotion-reduce:fallback) or gate it before setting the style.As per coding guidelines, "Ensure accessibility is WCAG 2.2 AA baseline. Never use color as sole meaning carrier. Implement keyboard-first interaction patterns."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/dashboard/components/sprint-progress-ring.tsx` around lines 67 - 71, The inline transition on the <circle> in SprintProgressRing ignores users' prefers-reduced-motion setting; move the animation off the inline style and into a Tailwind motion utility (e.g., add a class like "motion-safe:transition-[stroke-dashoffset_500ms_ease-out] motion-reduce:transition-none" on the <circle>) or conditionally set the style in the SprintProgressRing component based on a prefers-reduced-motion check before applying the transition property; update the <circle> element (the symbol named "style" object used on the circle) to use the className or conditional style so reduced-motion users do not get the animation.frontend/src/features/dashboard/components/attention-feed-section.tsx-59-91 (1)
59-91:⚠️ Potential issue | 🟡 MinorFix list semantics in the feed container.
<ul>currently contains<div>/<Separator>wrappers instead of only<li>children, which weakens list accessibility semantics.♿ Suggested structural fix
- const feedContent = ( - <ul - role="list" - className={cn('flex flex-col gap-0', className)} - > + const feedContent = ( + <div className={cn('flex flex-col gap-0', className)}> {CATEGORIES.map((category, index) => { const items = categorized[category.key] ?? []; return ( - <React.Fragment key={category.key}> + <section key={category.key} aria-labelledby={`attention-${category.key}`}> {index > 0 && <Separator className="my-3" />} <div className="flex flex-col gap-1"> - {/* Section header */} - <p className="text-[12px] font-semibold text-muted-foreground uppercase tracking-wide px-3 py-1"> + <p + id={`attention-${category.key}`} + className="text-[12px] font-semibold text-muted-foreground uppercase tracking-wide px-3 py-1" + > {category.label} </p> {items.length > 0 ? ( - items.map((item) => ( - <AttentionItem - key={item.issueId + item.type} - item={item} - workspaceSlug={workspaceSlug} - /> - )) + <ul role="list" className="flex flex-col gap-0"> + {items.map((item) => ( + <AttentionItem + key={item.issueId + item.type} + item={item} + workspaceSlug={workspaceSlug} + /> + ))} + </ul> ) : ( <p className="text-[14px] font-normal text-muted-foreground px-3 py-2"> None right now </p> )} </div> - </React.Fragment> + </section> ); })} - </ul> + </div> );As per coding guidelines, "Ensure accessibility is WCAG 2.2 AA baseline... Implement keyboard-first interaction patterns".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/dashboard/components/attention-feed-section.tsx` around lines 59 - 91, The UL contains non-LI children which breaks list semantics; update the rendering so each category section is an LI (use the category.key as the LI key) and move the section header, items mapping (AttentionItem), and empty state inside that LI; remove the wrapping <div> and replace the <Separator> with either an LI acting as a separator (with role="separator" or an aria-hidden HR inside an LI) or apply spacing via the LI's margin so the UL's immediate children are only LIs; keep existing references to CATEGORIES, categorized, AttentionItem and workspaceSlug and preserve unique keys for list items (e.g., LI keyed by category.key and AttentionItem keyed by item.issueId+item.type).backend/tests/application/services/issue/test_rich_context_assembler.py-508-509 (1)
508-509:⚠️ Potential issue | 🟡 MinorAdd
strict=Truetozip()call (B905).The pipeline flags
zip()without explicitstrict=parameter. Addingstrict=Trueensures both lists have equal length and prevents silent data loss.🧹 Proposed fix
- for pr, rid in zip(pr_links, related_ids): + for pr, rid in zip(pr_links, related_ids, strict=True): pr.issue_id = rid🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/application/services/issue/test_rich_context_assembler.py` around lines 508 - 509, In the loop that assigns related IDs to PR objects (the for loop iterating "for pr, rid in zip(pr_links, related_ids):" in test_rich_context_assembler.py), change the zip call to include strict=True so mismatched lengths raise an error; update the zip invocation to zip(pr_links, related_ids, strict=True) to ensure both sequences have equal length and prevent silent truncation when assigning pr.issue_id = rid.backend/src/pilot_space/infrastructure/database/models/batch_run_issue.py-139-144 (1)
139-144:⚠️ Potential issue | 🟡 MinorReplace ambiguous multiplication sign (×) with 'x' (RUF003).
The pipeline flags the Unicode multiplication sign. Use ASCII 'x' instead.
🧹 Proposed fix
- # AI cost tracking (cents × 100 to avoid floating point — e.g., 125 = $0.0125) + # AI cost tracking (cents x 100 to avoid floating point — e.g., 125 = $0.0125) cost_cents: Mapped[int] = mapped_column(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/infrastructure/database/models/batch_run_issue.py` around lines 139 - 144, Replace the Unicode multiplication sign in the inline comment on the cost_cents field with an ASCII 'x' to avoid the RUF003 lint warning; update the comment that currently reads "cents × 100" to "cents x 100" near the cost_cents: Mapped[int] = mapped_column(Integer, nullable=False, default=0) declaration so the meaning stays the same but uses only ASCII characters.backend/tests/application/services/issue/test_rich_context_assembler.py-438-446 (1)
438-446:⚠️ Potential issue | 🟡 MinorReplace
elifwithifafterreturnstatement (RET505).The pipeline flags unnecessary
elifafterreturn. Since the first branch returns, the subsequent conditions don't needelif.🧹 Proposed fix
async def side_effect_recall(recall_payload): nonlocal call_count call_count += 1 if call_count == 1: return RecallResult(items=[low_score_item], cache_hit=False, elapsed_ms=0) - elif call_count == 2: + if call_count == 2: return RecallResult(items=[high_score_item, unique_item], cache_hit=False, elapsed_ms=0) - else: - return RecallResult(items=[], cache_hit=False, elapsed_ms=0) + return RecallResult(items=[], cache_hit=False, elapsed_ms=0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/application/services/issue/test_rich_context_assembler.py` around lines 438 - 446, The side_effect_recall helper uses an unnecessary `elif` after a prior `return`; update the control flow in async def side_effect_recall to replace `elif call_count == 2:` with `if call_count == 2:` (leaving the first `if call_count == 1:` and the final `else` as-is) so subsequent branches are plain conditionals after the early return.backend/tests/application/services/issue/test_rich_context_assembler.py-11-45 (1)
11-45:⚠️ Potential issue | 🟡 MinorFix unused imports flagged by CI pipeline.
The pipeline reports unused imports that should be removed:
asyncio(line 13)dataclass,field(line 14)KGDecision(line 25)RelatedPR(line 27)SprintPeer(line 29)🧹 Proposed fix — remove unused imports
-import asyncio -from dataclasses import dataclass, field from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from uuid import UUID, uuid4 import pytest from pilot_space.api.v1.schemas.implement_context import ( ImplementContextResponse, IssueDetail, IssueStateDetail, - KGDecision, ProjectContext, - RelatedPR, RepositoryContext, - SprintPeer, WorkspaceContext, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/application/services/issue/test_rich_context_assembler.py` around lines 11 - 45, Remove the unused imports flagged by CI from the test module: delete asyncio, dataclass, and field from the top imports and remove KGDecision, RelatedPR, and SprintPeer from the pilot_space.api.v1.schemas.implement_context import list; ensure only the actually used symbols (e.g., ImplementContextResponse, IssueDetail, IssueStateDetail, ProjectContext, RepositoryContext, WorkspaceContext) remain so tests and imports in test_rich_context_assembler.py compile cleanly.frontend/src/features/issues/components/batch-run-progress-panel.tsx-108-112 (1)
108-112:⚠️ Potential issue | 🟡 MinorInclude cancelled issues in the completion percentage.
completionPctonly counts completed + failed. Cancelled issues are terminal as well, so a batch with cancelled items can stay below 100% even after everything is finished.Also applies to: 163-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/issues/components/batch-run-progress-panel.tsx` around lines 108 - 112, The completionPct calculation in batch-run-progress-panel's React.useMemo only sums batchRun.completedIssues and batchRun.failedIssues, omitting terminal cancelled issues; update the logic in the completionPct computation (and the duplicate logic around the other occurrence noted) to include batchRun.cancelledIssues in the numerator so the percentage becomes ((completedIssues + failedIssues + cancelledIssues) / totalIssues) * 100, and keep the existing zero guard for missing batchRun or totalIssues === 0.backend/tests/unit/services/test_rich_context_assembler.py-18-18 (1)
18-18:⚠️ Potential issue | 🟡 MinorRemove the unused imports so CI can pass.
Ruff is already failing on
patch,IssueLabelDetail,IssueStateDetail, andStateGroupbeing unused in this module.Also applies to: 42-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/services/test_rich_context_assembler.py` at line 18, Remove the unused imports causing Ruff failures by deleting or pruning any imports not referenced in this test module: remove patch from the unittest.mock import line (keeping AsyncMock and MagicMock if used), and remove the unused model/type imports IssueLabelDetail, IssueStateDetail, and StateGroup from wherever they are imported in this file (lines referenced around 42–50); ensure only actually used symbols remain to satisfy the linter.backend/src/pilot_space/infrastructure/queue/handlers/deviation_analysis_handler.py-220-221 (1)
220-221:⚠️ Potential issue | 🟡 MinorDon't use
ValueErrorfor not-found paths in backend code.Missing issue/note is not UUID parsing, enum parsing, or
int()conversion. These branches should use a domain not-found signal instead ofValueError.As per coding guidelines "Services MUST raise domain exceptions from pilot_space.domain.exceptions (NotFoundError, ForbiddenError, ConflictError, ValidationError, AppError), NOT ValueError or HTTPException" and "Keep
ValueErroronly for UUID parsing (UUID(slug) fallback), enum parsing, and int() conversion."Also applies to: 238-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/infrastructure/queue/handlers/deviation_analysis_handler.py` around lines 220 - 221, Replace use of built-in ValueError for missing domain records with the domain-specific NotFoundError: import NotFoundError from pilot_space.domain.exceptions and change the raise in the "if row is None:" branch (and the similar branch at the other occurrence) to raise NotFoundError(f"Issue {issue_id} not found") (or the appropriate entity id/message). Update both occurrences in deviation_analysis_handler.py (the check using "row is None") to use NotFoundError so the handler follows the domain exception convention.backend/src/pilot_space/application/services/issue/rich_context_assembler.py-489-495 (1)
489-495:⚠️ Potential issue | 🟡 MinorRead sprint-peer acceptance criteria from the new persisted field.
This summary still looks in
peer.ai_metadata, but this PR promotes acceptance criteria toIssue.acceptance_criteria. For newly created/updated issues,acceptance_criteria_summarywill stay empty unless you switch this extraction to the first-class field.Possible fix
- ai_metadata = getattr(peer, "ai_metadata", None) or {} - if isinstance(ai_metadata, dict): - ac_list = ai_metadata.get("acceptance_criteria", []) - if ac_list: - ac_summary = str(ac_list[0])[:_AC_SUMMARY_CHARS] + ac_list = getattr(peer, "acceptance_criteria", None) or [] + if ac_list: + first_item = ac_list[0] + if isinstance(first_item, dict): + ac_summary = str(first_item.get("text") or first_item.get("title") or first_item)[ + :_AC_SUMMARY_CHARS + ] + else: + ac_summary = str(first_item)[:_AC_SUMMARY_CHARS]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/application/services/issue/rich_context_assembler.py` around lines 489 - 495, The code currently extracts the first acceptance criterion from peer.ai_metadata (ai_metadata -> acceptance_criteria) into ac_summary; change the logic in rich_context_assembler.py (around ac_summary/ai_metadata) to prefer the new first-class field Issue.acceptance_criteria (e.g., peer.acceptance_criteria or peer.acceptance_criteria[0]) and truncate to _AC_SUMMARY_CHARS, falling back to the existing ai_metadata extraction only if the new field is missing or empty, and ensure acceptance_criteria_summary is populated from this updated ac_summary.backend/src/pilot_space/api/v1/routers/batch_runs.py-346-347 (1)
346-347:⚠️ Potential issue | 🟡 MinorDrop the no-op
CancelledErrorhandler.This
except asyncio.CancelledError: raisedoes not change behavior and is already failing CI with TRY203.Suggested change
- except asyncio.CancelledError: - raise finally: await pubsub.unsubscribe(channel) await pubsub.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/pilot_space/api/v1/routers/batch_runs.py` around lines 346 - 347, Remove the no-op except asyncio.CancelledError: raise handler in batch_runs.py so CancelledError can propagate naturally; if there is a broader catch (e.g., a following except Exception or bare except) that would swallow cancellations, update that handler to re-raise asyncio.CancelledError (check isinstance and raise) instead of handling it—this eliminates the redundant clause and fixes the TRY203 failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7cb82a9-ee00-42f3-9c39-cb2bf72a89d0
📒 Files selected for processing (110)
backend/alembic/versions/110_data_foundations_source_note_batch_runs.pybackend/alembic/versions/111_batch_run_current_stage.pybackend/alembic/versions/112_add_spec_annotations_column.pybackend/src/pilot_space/ai/mcp/issue_server.pybackend/src/pilot_space/ai/tools/issue_server.pybackend/src/pilot_space/ai/workers/__init__.pybackend/src/pilot_space/ai/workers/batch_impl_worker.pybackend/src/pilot_space/ai/workers/memory_worker.pybackend/src/pilot_space/api/v1/dependencies.pybackend/src/pilot_space/api/v1/dependencies_pilot.pybackend/src/pilot_space/api/v1/routers/__init__.pybackend/src/pilot_space/api/v1/routers/batch_runs.pybackend/src/pilot_space/api/v1/routers/issue_implement.pybackend/src/pilot_space/api/v1/routers/workspace_issues.pybackend/src/pilot_space/api/v1/routers/workspace_notes.pybackend/src/pilot_space/api/v1/schemas/implement_context.pybackend/src/pilot_space/api/v1/schemas/issue.pybackend/src/pilot_space/api/v1/schemas/note.pybackend/src/pilot_space/application/services/batch_run_service.pybackend/src/pilot_space/application/services/issue/__init__.pybackend/src/pilot_space/application/services/issue/batch_create_issues_service.pybackend/src/pilot_space/application/services/issue/create_issue_service.pybackend/src/pilot_space/application/services/issue/rich_context_assembler.pybackend/src/pilot_space/application/services/issue/update_issue_service.pybackend/src/pilot_space/container/container.pybackend/src/pilot_space/domain/exceptions.pybackend/src/pilot_space/infrastructure/database/models/__init__.pybackend/src/pilot_space/infrastructure/database/models/batch_run.pybackend/src/pilot_space/infrastructure/database/models/batch_run_issue.pybackend/src/pilot_space/infrastructure/database/models/issue.pybackend/src/pilot_space/infrastructure/database/models/note.pybackend/src/pilot_space/infrastructure/database/repositories/batch_run_repository.pybackend/src/pilot_space/infrastructure/database/repositories/integration_link_repository.pybackend/src/pilot_space/infrastructure/database/repositories/note_repository.pybackend/src/pilot_space/infrastructure/queue/handlers/batch_impl_handler.pybackend/src/pilot_space/infrastructure/queue/handlers/deviation_analysis_handler.pybackend/src/pilot_space/infrastructure/queue/models.pybackend/src/pilot_space/main.pybackend/tests/ai/mcp/__init__.pybackend/tests/ai/mcp/test_issue_server_generate.pybackend/tests/api/v1/test_issues_batch.pybackend/tests/application/services/issue/__init__.pybackend/tests/application/services/issue/test_rich_context_assembler.pybackend/tests/application/services/test_batch_run_service.pybackend/tests/infrastructure/queue/handlers/test_batch_impl_handler.pybackend/tests/unit/schemas/test_issue_response_note_links.pybackend/tests/unit/services/test_get_implement_context_service.pybackend/tests/unit/services/test_rich_context_assembler.pycli/src/pilot_cli/commands/implement.pycli/src/pilot_cli/templates/CLAUDE_MD_TEMPLATE.mdfrontend/src/app/(workspace)/[workspaceSlug]/dashboard/page.tsxfrontend/src/app/(workspace)/[workspaceSlug]/notes/[noteId]/page.tsxfrontend/src/app/(workspace)/[workspaceSlug]/projects/[projectId]/cycles/[cycleId]/page.tsxfrontend/src/app/globals.cssfrontend/src/app/layout.tsxfrontend/src/components/layout/sidebar.tsxfrontend/src/features/ai/ChatView/ChatHeader.tsxfrontend/src/features/ai/ChatView/ChatView.tsxfrontend/src/features/ai/ChatView/MessageList/AssistantMessage.tsxfrontend/src/features/ai/ChatView/MessageList/BatchPreviewCard.tsxfrontend/src/features/ai/ChatView/MessageList/IssuePreviewItem.tsxfrontend/src/features/ai/ChatView/MessageList/ToolCallCard.tsxfrontend/src/features/ai/ChatView/MessageList/__tests__/BatchPreviewCard.test.tsxfrontend/src/features/ai/ChatView/TokenRing.tsxfrontend/src/features/ai/ChatView/__tests__/TokenRing.test.tsxfrontend/src/features/dashboard/components/attention-feed-section.tsxfrontend/src/features/dashboard/components/attention-item.tsxfrontend/src/features/dashboard/components/cost-breakdown-panel.tsxfrontend/src/features/dashboard/components/eta-display.tsxfrontend/src/features/dashboard/components/live-issue-card.tsxfrontend/src/features/dashboard/components/sprint-progress-ring.tsxfrontend/src/features/dashboard/components/status-stat-card.tsxfrontend/src/features/dashboard/hooks/use-dashboard.tsfrontend/src/features/dashboard/types.tsfrontend/src/features/homepage/__tests__/HomepageHub.test.tsxfrontend/src/features/homepage/components/ChatHeroInput.tsxfrontend/src/features/homepage/components/HomepageHub.tsxfrontend/src/features/homepage/components/QuickActionCard.tsxfrontend/src/features/homepage/components/RecentConversationCard.tsxfrontend/src/features/issues/components/activity-timeline.tsxfrontend/src/features/issues/components/batch-run-activity-entry.tsxfrontend/src/features/issues/components/batch-run-progress-panel.tsxfrontend/src/features/issues/components/cancel-batch-button.tsxfrontend/src/features/issues/components/cancel-issue-button.tsxfrontend/src/features/issues/components/implementation-status-badge.tsxfrontend/src/features/issues/components/property-block-view.tsxfrontend/src/features/issues/components/sprint-dag-preview.tsxfrontend/src/features/issues/hooks/use-batch-run-stream.tsfrontend/src/features/issues/hooks/use-batch-run.tsfrontend/src/features/notes/components/living-spec-sidebar.tsxfrontend/src/features/notes/components/living-spec/annotations-panel.tsxfrontend/src/features/notes/components/living-spec/decision-log-card.tsxfrontend/src/features/notes/components/living-spec/deviation-card.tsxfrontend/src/features/notes/components/living-spec/issue-status-row.tsxfrontend/src/features/notes/components/living-spec/linked-issues-panel.tsxfrontend/src/features/notes/components/living-spec/table-of-contents-panel.tsxfrontend/src/features/notes/components/living-spec/toc-item.tsxfrontend/src/features/notes/hooks/index.tsfrontend/src/features/notes/hooks/use-living-spec.tsfrontend/src/features/notes/hooks/use-note-annotations.tsfrontend/src/features/notes/hooks/use-toc-headings.tsfrontend/src/features/notes/hooks/useNotes.tsfrontend/src/services/api/issues.tsfrontend/src/services/api/notes.tsfrontend/src/stores/ai/PilotSpaceActions.tsfrontend/src/stores/ai/PilotSpaceStore.tsfrontend/src/stores/ai/PilotSpaceStreamHandler.tsfrontend/src/stores/ai/types/conversation.tsfrontend/src/stores/ai/types/event-guards.tsfrontend/src/stores/ai/types/events.ts
Comment on lines
+205
to
+213
| async def create_batch_run( | ||
| workspace_id: WorkspaceIdPath, | ||
| body: CreateBatchRunRequest, | ||
| session: DbSession, | ||
| service: BatchRunServiceDep, | ||
| queue: QueueClientDep, | ||
| current_user_id: CurrentUserId, | ||
| _member: WorkspaceMemberId, | ||
| ) -> BatchRunResponse: |
There was a problem hiding this comment.
Use SessionDep in these DI-backed route signatures.
These handlers still declare session: DbSession. In this codebase, the DI wiring expects session: SessionDep; leaving DbSession here risks the request-scoped session context not being populated for the injected service.
Suggested change
-from pilot_space.dependencies import DbSession, QueueClientDep, RedisDep
+from pilot_space.dependencies import QueueClientDep, RedisDep, SessionDep
...
- session: DbSession,
+ session: SessionDep,As per coding guidelines, "Every route using a DI-provided service MUST declare session: SessionDep in its signature to avoid RuntimeError: No session in current context".
Also applies to: 263-269, 289-295, 368-375, 401-409
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/pilot_space/api/v1/routers/batch_runs.py` around lines 205 - 213,
The route handlers (e.g., create_batch_run) currently type their DB injection as
DbSession; change these signatures to use SessionDep so the request-scoped
session context is provided to DI-backed services. Update the function parameter
list for create_batch_run and the other DI-backed handlers mentioned (the
handlers at the ranges referenced) to replace session: DbSession with session:
SessionDep, then ensure any imports/annotations reference SessionDep and not
DbSession so the injected session is properly populated for BatchRunServiceDep,
QueueClientDep, and other DI services.
Comment on lines
+263
to
+281
| async def get_batch_run( | ||
| workspace_id: WorkspaceIdPath, | ||
| batch_run_id: BatchRunIdPath, | ||
| session: DbSession, | ||
| service: BatchRunServiceDep, | ||
| _member: WorkspaceMemberId, | ||
| ) -> BatchRunResponse: | ||
| """Return current status of a batch run with all per-issue statuses.""" | ||
| from pilot_space.domain.exceptions import NotFoundError | ||
| from pilot_space.infrastructure.database.repositories.batch_run_repository import ( | ||
| BatchRunRepository, | ||
| ) | ||
|
|
||
| repo = BatchRunRepository(session) | ||
| batch_run = await repo.get_by_id_with_items(batch_run_id) | ||
| if batch_run is None: | ||
| raise NotFoundError(f"BatchRun {batch_run_id} not found.") | ||
|
|
||
| return _build_batch_run_response(batch_run) |
There was a problem hiding this comment.
Scope batch-run reads and mutations to workspace_id.
require_workspace_member only proves the caller belongs to the workspace from the path. These handlers then fetch, stream, and cancel by bare batch_run_id, so a member of workspace A can read or act on workspace B's batch if they know the UUID. Resolve the batch through a workspace-scoped service method and reject mismatches before returning data, opening the SSE stream, or mutating state.
Also applies to: 315-360, 368-393, 445-468
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/pilot_space/api/v1/routers/batch_runs.py` around lines 263 - 281,
The handler currently fetches batch runs by bare
BatchRunRepository.get_by_id_with_items(batch_run_id) allowing cross-workspace
access; change to resolve the batch via a workspace-scoped lookup and reject
mismatches before returning or mutating state. Replace direct repo calls in
get_batch_run (and the other handlers that stream or cancel) with a
workspace-aware service/repository call (e.g.,
BatchRunServiceDep.get_by_workspace_and_id(workspace_id, batch_run_id) or
BatchRunRepository.get_by_workspace_and_id(session, workspace_id,
batch_run_id)), verify the returned batch_run belongs to workspace_id, and raise
NotFoundError or an authorization error if not found/mismatched; do the same for
SSE open/stream and cancel endpoints so all reads and mutations are scoped to
workspace_id.
Comment on lines
+401
to
+437
| async def cancel_batch_run_issue( | ||
| workspace_id: WorkspaceIdPath, | ||
| batch_run_id: BatchRunIdPath, | ||
| issue_id: IssueIdPath, | ||
| session: DbSession, | ||
| service: BatchRunServiceDep, | ||
| redis: RedisDep, | ||
| _member: WorkspaceMemberId, | ||
| ) -> dict[str, str]: | ||
| """Cancel a single BatchRunIssue. | ||
|
|
||
| Publishes an ``issue_cancelled`` event to the Redis channel so connected | ||
| SSE clients can update the individual issue card in the UI. | ||
| """ | ||
| await service.cancel_issue(issue_id) | ||
|
|
||
| # Publish event so SSE stream reflects the cancellation immediately | ||
| channel = f"batch_runs:{batch_run_id}" | ||
| try: | ||
| await redis.publish( | ||
| channel, | ||
| json.dumps( | ||
| { | ||
| "event": "issue_cancelled", | ||
| "batch_run_id": str(batch_run_id), | ||
| "issue_id": str(issue_id), | ||
| } | ||
| ), | ||
| ) | ||
| except Exception: | ||
| logger.exception( | ||
| "issue_cancel_publish_failed", | ||
| batch_run_id=str(batch_run_id), | ||
| issue_id=str(issue_id), | ||
| ) | ||
|
|
||
| return {"status": "cancelled"} |
There was a problem hiding this comment.
Validate that issue_id belongs to the path batch_run_id before cancelling.
Line 415 cancels by issue_id alone, then Line 418 publishes to the channel derived from the path batch_run_id. A mismatched pair can cancel one batch-run issue while notifying subscribers of a different batch. Pass both ids into the service and fail if they do not belong to the same batch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/pilot_space/api/v1/routers/batch_runs.py` around lines 401 - 437,
The handler cancel_batch_run_issue currently calls
service.cancel_issue(issue_id) without verifying the issue belongs to the
batch_run_id path; update the logic in cancel_batch_run_issue to first validate
ownership (e.g. call a service method like BatchRunService.get_issue(issue_id)
or add a check in service.cancel_issue to accept both ids) and fail with an
appropriate error if the issue.batch_run_id != batch_run_id, then call the
cancellation using both identifiers (e.g. service.cancel_issue(batch_run_id,
issue_id)) and only publish the Redis event to channel
f"batch_runs:{batch_run_id}" after successful validation and cancellation.
Ensure the unique symbols mentioned (cancel_batch_run_issue,
service.cancel_issue, and any get_issue/get_by_id method you add) are updated
accordingly.
|
|
||
| @property | ||
| def progress_percent(self) -> float: | ||
| """Calculate completion percentage (0.0–100.0).""" |
There was a problem hiding this comment.
CI blocker: replace ambiguous en dash in docstring.
RUF002 is failing CI on this line.
✅ Minimal fix
- """Calculate completion percentage (0.0–100.0)."""
+ """Calculate completion percentage (0.0-100.0)."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| """Calculate completion percentage (0.0–100.0).""" | |
| """Calculate completion percentage (0.0-100.0).""" |
🧰 Tools
🪛 GitHub Actions: CI
[error] 163-163: RUF002 Docstring contains ambiguous en dash (–). Did you mean '-'?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/pilot_space/infrastructure/database/models/batch_run.py` at line
163, Replace the ambiguous en dash character in the docstring that reads
"Calculate completion percentage (0.0–100.0)." with a normal ASCII hyphen so it
becomes "(0.0-100.0)"; update the docstring in batch_run.py for the
completion-percentage method/property to use the hyphen character to satisfy
RUF002.
Comment on lines
+87
to
+201
| async def handle(self, payload: dict[str, Any]) -> dict[str, Any]: | ||
| """Process one deviation_analysis queue message. Never raises. | ||
|
|
||
| Args: | ||
| payload: Queue message payload dict. | ||
|
|
||
| Returns: | ||
| Result dict with success flag and metadata. | ||
| """ | ||
| try: | ||
| p = _DeviationPayload.from_dict(payload) | ||
| except (KeyError, ValueError) as exc: | ||
| logger.warning( | ||
| "DeviationAnalysisHandler: invalid payload %r — %s", | ||
| payload, | ||
| exc, | ||
| ) | ||
| return {"success": False, "error": "invalid_payload"} | ||
|
|
||
| if self._llm_gateway is None: | ||
| logger.info( | ||
| "DeviationAnalysisHandler: no LLMGateway wired — skipping " | ||
| "(workspace=%s issue=%s)", | ||
| p.workspace_id, | ||
| p.issue_id, | ||
| ) | ||
| return {"success": True, "skipped": "no_llm_gateway"} | ||
|
|
||
| # 1. Load issue details | ||
| try: | ||
| issue_title, issue_description = await self._load_issue(p.issue_id) | ||
| except Exception: | ||
| logger.exception( | ||
| "DeviationAnalysisHandler: failed to load issue %s — abort", | ||
| p.issue_id, | ||
| ) | ||
| return {"success": False, "error": "issue_load_failed"} | ||
|
|
||
| # 2. Load note content (plain text) | ||
| try: | ||
| note_text = await self._load_note_text(p.source_note_id) | ||
| except Exception: | ||
| logger.exception( | ||
| "DeviationAnalysisHandler: failed to load note %s — abort", | ||
| p.source_note_id, | ||
| ) | ||
| return {"success": False, "error": "note_load_failed"} | ||
|
|
||
| if not note_text.strip(): | ||
| logger.info( | ||
| "DeviationAnalysisHandler: note %s has no plain text — skip", | ||
| p.source_note_id, | ||
| ) | ||
| return {"success": True, "skipped": "empty_note"} | ||
|
|
||
| # 3. LLM call — cheap Haiku-tier model, non-fatal | ||
| deviation_text = await self._detect_deviation( | ||
| workspace_id=p.workspace_id, | ||
| actor_user_id=p.actor_user_id, | ||
| pr_url=p.pr_url, | ||
| note_text=note_text, | ||
| issue_title=issue_title, | ||
| issue_description=issue_description or "", | ||
| ) | ||
|
|
||
| if deviation_text is None: | ||
| return {"success": False, "error": "llm_failed"} | ||
|
|
||
| if deviation_text == _NO_DEVIATION_SENTINEL: | ||
| logger.info( | ||
| "DeviationAnalysisHandler: no deviation detected " | ||
| "(workspace=%s issue=%s pr=%s)", | ||
| p.workspace_id, | ||
| p.issue_id, | ||
| p.pr_url, | ||
| ) | ||
| return {"success": True, "deviation_detected": False} | ||
|
|
||
| # 4. Append annotation to source note — non-fatal | ||
| try: | ||
| appended = await self._append_annotation( | ||
| note_id=p.source_note_id, | ||
| issue_id=p.issue_id, | ||
| pr_url=p.pr_url, | ||
| deviation_text=deviation_text, | ||
| ) | ||
| except Exception: | ||
| logger.exception( | ||
| "DeviationAnalysisHandler: failed to append annotation " | ||
| "(note=%s issue=%s) — non-fatal", | ||
| p.source_note_id, | ||
| p.issue_id, | ||
| ) | ||
| return {"success": False, "error": "annotation_write_failed"} | ||
|
|
||
| if not appended: | ||
| logger.warning( | ||
| "DeviationAnalysisHandler: note %s not found — annotation skipped", | ||
| p.source_note_id, | ||
| ) | ||
| return {"success": False, "error": "note_not_found"} | ||
|
|
||
| logger.info( | ||
| "DeviationAnalysisHandler: deviation annotation appended " | ||
| "(workspace=%s note=%s issue=%s)", | ||
| p.workspace_id, | ||
| p.source_note_id, | ||
| p.issue_id, | ||
| ) | ||
| return { | ||
| "success": True, | ||
| "deviation_detected": True, | ||
| "note_id": str(p.source_note_id), | ||
| "issue_id": str(p.issue_id), | ||
| } |
There was a problem hiding this comment.
Scope the worker session before any DB access.
workspace_id is parsed from the payload, but none of the reads/writes use it and handle() never sets the RLS context. In a worker session with elevated privileges, a malformed queue message can load an issue/note from one workspace and append an annotation to another tenant's note.
Minimal direction
+from pilot_space.infrastructure.database.rls import set_rls_context
...
try:
p = _DeviationPayload.from_dict(payload)
except (KeyError, ValueError) as exc:
...
+
+ await set_rls_context(
+ self._session,
+ user_id=p.actor_user_id,
+ workspace_id=p.workspace_id,
+ )Also applies to: 207-243, 295-320
🧰 Tools
🪛 GitHub Actions: CI
[error] 87-87: PLR0911 Too many return statements (10 > 6).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/pilot_space/infrastructure/queue/handlers/deviation_analysis_handler.py`
around lines 87 - 201, After parsing the payload in
DeviationAnalysisHandler.handle, set the request/DB session's RLS/tenant context
using the parsed workspace_id before any DB access (i.e. before calls to
_load_issue, _load_note_text, _detect_deviation, or _append_annotation);
implement this by invoking the existing project helper for scoping worker
sessions (for example a context manager or setter like
self._set_workspace_context(p.workspace_id) or
self._scoped_workspace(p.workspace_id)) at the top of handle (after
_DeviationPayload.from_dict) and ensure the context is cleared/closed on exit so
all subsequent reads/writes honor the workspace RLS; apply the same change to
the other handlers mentioned (the blocks that call
_load_issue/_load_note_text/_append_annotation in lines ~207-243 and ~295-320).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/scripts/seed_demo.py`:
- Around line 155-158: The current TRUNCATE statement wipes audit_log for all
workspaces; change the session.execute(text("TRUNCATE audit_log CASCADE")) usage
to a scoped DELETE that only removes rows for the demo workspace (e.g.,
session.execute(text("DELETE FROM audit_log WHERE workspace_id = :ws_id"),
{"ws_id": demo_workspace.id})) and pass the demo workspace id variable used
earlier in the script; additionally gate this destructive action behind an
explicit human confirmation (prompt or an explicit confirm flag/env var) per
DD-003 so the script aborts unless the operator explicitly approves the purge.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a652b435-7074-4790-95e2-9e356c7fced1
📒 Files selected for processing (1)
backend/scripts/seed_demo.py
Comment on lines
+155
to
+158
| # Truncate audit_log first — RLS policy blocks DELETE on cascaded rows | ||
| await session.execute( | ||
| text("TRUNCATE audit_log CASCADE"), | ||
| ) |
There was a problem hiding this comment.
Scope the audit purge to demo data; avoid full-table truncate.
Line 157 runs TRUNCATE audit_log CASCADE, which wipes audit history for all workspaces, not only the demo workspace. That’s overly destructive for a reseed path.
Proposed safer change
- # Truncate audit_log first — RLS policy blocks DELETE on cascaded rows
- await session.execute(
- text("TRUNCATE audit_log CASCADE"),
- )
+ # Purge only demo-workspace audit rows before deleting workspace/user
+ await session.execute(text("SET LOCAL app.audit_purge = 'true'"))
+ await session.execute(
+ text("DELETE FROM audit_log WHERE workspace_id = :workspace_id"),
+ {"workspace_id": DEMO_WORKSPACE_ID},
+ )Based on learnings: All destructive actions require human-in-the-loop approval (DD-003).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/scripts/seed_demo.py` around lines 155 - 158, The current TRUNCATE
statement wipes audit_log for all workspaces; change the
session.execute(text("TRUNCATE audit_log CASCADE")) usage to a scoped DELETE
that only removes rows for the demo workspace (e.g.,
session.execute(text("DELETE FROM audit_log WHERE workspace_id = :ws_id"),
{"ws_id": demo_workspace.id})) and pass the demo workspace id variable used
earlier in the script; additionally gate this destructive action behind an
explicit human confirmation (prompt or an explicit confirm flag/env var) per
DD-003 so the script aborts unless the operator explicitly approves the purge.
Implement Homepage v2 per design.md spec — a 4-section dashboard (Artifacts → Routines → Sprint Progress → Conversations) with Gemini-style AI Prompt Hero, wired to real PilotSpace stores. ## Design tokens (globals.css) - Add --surface-chatbox (#f0f4f9) and --border-toolbar (#c8d1db) - Add signature radius (22px) and chatbox radius (28px) to scale - Map tokens to Tailwind via @theme inline block ## ChatHeroInput — Gemini-style two-zone chatbox - Borderless #f0f4f9 surface, 28px radius - Zone 1: textarea + 34px pill send button - Zone 2: bare icons (+/sliders) + connector pills + model pill + Plan selector - Wired to AISettingsStore.availableModels + PilotSpaceStore.selectedModel - Wired to MCPServersStore.servers for real connector display - Submit navigates to /chat?prefill= (full ChatInput takes over) ## Recent Artifacts (RecentWorkSection) - Rework to type-specific gradient cards: HTML/DOCX/CODE - 22px signature radius, 110px gradient top, JetBrains Mono type badges - Rotate through all 3 types for visual variety - Wired to useHomepageActivity() → GET /homepage/activity ## Active Routines (new) - Messenger-source circles (34px Telegram/Claude/WhatsApp) - Status badges: Running (pulse) / Scheduled (timer) / Done (check) - 16px card radius with inline detail metadata - Demo data (no backend /routines API yet) ## Sprint Progress (new) - Project-colored progress bars, 16px card radius - Status derivation: Ahead / On Track / At Risk from completion% + days - Wired to projectsApi.list() + cyclesApi.getActive() per project - Health alerts above cards: stale issues, blocked deps from digest - Demo fallback when no active cycles exist ## Recent Conversations (restored) - Horizontal scroll of up to 5 AI chat sessions - Wired to SessionListStore for resume functionality ## HomepageHub layout - Remove: WorkspaceAlerts, QuickActionCarousel (replaced by new sections) - Keep: ExamplePrompts below chatbox for new-user discoverability - Spacing: 36px pill→greeting, 36px greeting→chatbox, 80px chatbox→artifacts - Hero greeting: Fraunces 24px/400 with -1px tracking (was text-4xl/600) ## Documentation - Update CLAUDE.md with v2 design system references and UI-SPEC path - Update .impeccable.md with three-influence aesthetic (Cohere/Gemini/PilotSpace) - Add Pencil .pen gotcha — use only mcp__pencil__* tools for .pen files author: Tin Dang
- Introduced AgentPill, AttachmentPill, SkillPill, ToolPill, and VoicePill for a consistent toolbar experience. - Created Pill component as a base for visual consistency across pills. - Developed ContinueCard for resuming last conversations on the homepage. - Implemented HomepageV3 layout with greeting, chat input, and suggested prompts. - Added RedFlagRow to display workspace alerts and statuses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Milestone: v2.0 — Autonomous SDLC
Goal: Make Pilot Space the enterprise control plane for AI-powered software development — PM chats, AI builds, humans review.
Status: All 6 phases verified ✓
This PR delivers the complete v2.0 milestone: a PM describes work in chat, AI generates implementation-ready issues, Claude Code implements autonomously via
pilot implement, and the PM monitors progress on a real-time dashboard. Notes become living documents that track implementation reality.Changes
Phase 73: Data Foundations + Design Tokens (3 plans)
source_note_idFK on issues,batch_runs+batch_run_issuestables with RLSacceptance_criteria+source_note_idthrough create/update/response#f7f7f5surfaces, warm-tinted shadowsPhase 74: Rich Context Engine (2 plans)
RichContextAssemblerwrappingGetImplementContextServicewith 3 enrichment layersimplement.pypassthrough for enrichment fieldsPhase 75: Chat to Issue Pipeline (4 plans)
generate_issues_from_descriptionMCP tool on issue_server withissue_batch_proposalSSEBatchPreviewCard+IssuePreviewItem(React.memo, NOT observer) with DD-003 approval gateTokenRingcircular progress in ChatHeader,ToolCallCardwarm variantPhase 76: Sprint Batch Implementation (5 plans)
BatchRunServicewith Kahn's topological sort for dependency DAGBatchImplWorkerwithasyncio.Semaphore(3), VT heartbeat, PR URL extractionpreview_sprint_implementation,implement_sprint) + worker lifecyclePhase 77: Implementation Dashboard (2 plans)
GET /batch-runs/{id}/dashboardaggregation endpointPhase 78: Living Specs (2 plans)
spec_annotationsJSONB column on notesDeviationAnalysisHandlerbackground job comparing PR diff to source noteLivingSpecSidebarwith linked issues panel, AI annotations, auto-generated TOCRequirements Addressed
Verification
Test Plan
cd backend && uv run pytest tests/ -q(25+ new tests across phases)cd frontend && pnpm test(50+ new tests across phases)cd frontend && pnpm type-check+cd backend && uv run pyright#f7f7f5surface, Inter/Geist Mono fonts renderingSummary by CodeRabbit
New Features
Improvements