From 511c7db8601442949119ee6364943f121d14f895 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 11 Jan 2026 00:50:02 +0100 Subject: [PATCH 1/5] fix: code quality improvements from /refactor analysis - Remove incorrect await on SQLAlchemy delete() (repositories.py) - Extract duplicate workflow prompts to dedicated files - Add ErrorBoundary component to React frontend - Fix test warnings for unawaited coroutines - Delete deprecated prompt files Co-Authored-By: Claude Opus 4.5 --- .github/workflows/impl-generate.yml | 156 ++---------------- .github/workflows/impl-repair.yml | 138 ++-------------- app/src/components/ErrorBoundary.tsx | 122 ++++++++++++++ app/src/router.tsx | 9 +- core/database/repositories.py | 6 +- .../generate-implementation.md | 58 ------- .../workflow-prompts/impl-generate-claude.md | 81 +++++++++ .../workflow-prompts/impl-repair-claude.md | 74 +++++++++ .../workflow-prompts/improve-from-feedback.md | 57 ------- tests/unit/api/test_analytics.py | 68 +++++--- 10 files changed, 354 insertions(+), 415 deletions(-) create mode 100644 app/src/components/ErrorBoundary.tsx delete mode 100644 prompts/workflow-prompts/generate-implementation.md create mode 100644 prompts/workflow-prompts/impl-generate-claude.md create mode 100644 prompts/workflow-prompts/impl-repair-claude.md delete mode 100644 prompts/workflow-prompts/improve-from-feedback.md diff --git a/.github/workflows/impl-generate.yml b/.github/workflows/impl-generate.yml index df26cc421b..c3e5eca1cc 100644 --- a/.github/workflows/impl-generate.yml +++ b/.github/workflows/impl-generate.yml @@ -253,78 +253,12 @@ jobs: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} claude_args: "--model opus" prompt: | - ## Task: Generate ${{ steps.inputs.outputs.library }} Implementation - - You are generating the **${{ steps.inputs.outputs.library }}** implementation for **${{ steps.inputs.outputs.specification_id }}**. - - **Regeneration:** ${{ steps.existing.outputs.is_regeneration }} - - ### Step 1: Read required files - 1. `prompts/plot-generator.md` - Base generation rules (IMPORTANT: Read the "Regeneration" section!) - 2. `prompts/default-style-guide.md` - Visual style requirements - 3. `prompts/quality-criteria.md` - Quality requirements - 4. `prompts/library/${{ steps.inputs.outputs.library }}.md` - Library-specific rules - 5. `plots/${{ steps.inputs.outputs.specification_id }}/specification.md` - The specification - - ### Step 1b: If Regeneration, read previous feedback - If this is a regeneration (${{ steps.existing.outputs.is_regeneration }} == true): - 1. Read `plots/${{ steps.inputs.outputs.specification_id }}/metadata/${{ steps.inputs.outputs.library }}.yaml` - - Look at `review.strengths` (keep these aspects!) - - Look at `review.weaknesses` (fix these problems - decide HOW yourself) - - Look at `review.image_description` (understand what was generated visually) - - Look at `review.criteria_checklist` (see exactly which criteria failed) - - Focus on categories with low scores (e.g., visual_quality.score < visual_quality.max) - - Check items with `passed: false` - these need fixing - - VQ-XX items for visual issues - - SC-XX items for spec compliance - - CQ-XX items for code quality - 2. Read `plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py` - - Understand what was done before - - Keep what worked, fix what didn't - - ### Step 2: Generate implementation - Create: `plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py` - - The script MUST: - - Save as `plot.png` in the current directory - - For interactive libraries (plotly, bokeh, altair, highcharts, pygal, letsplot): also save `plot.html` - - ### Step 3: Test and fix (up to 3 attempts) - Run the implementation: - ```bash - source .venv/bin/activate - cd plots/${{ steps.inputs.outputs.specification_id }}/implementations - MPLBACKEND=Agg python ${{ steps.inputs.outputs.library }}.py - ``` - - If it fails, fix and try again (max 3 attempts). - - ### Step 4: Visual self-check - Look at the generated `plot.png`: - - Does it match the specification? - - Are axes labeled correctly? - - Is the visualization clear? - - ### Step 5: Format the code - ```bash - source .venv/bin/activate - ruff format plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py - ruff check --fix plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py - ``` - - ### Step 6: Commit - ```bash - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - git add plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py - git commit -m "feat(${{ steps.inputs.outputs.library }}): implement ${{ steps.inputs.outputs.specification_id }}" - git push -u origin implementation/${{ steps.inputs.outputs.specification_id }}/${{ steps.inputs.outputs.library }} - ``` - - ### Report result - Print exactly one line: - - `GENERATION_SUCCESS` - if everything worked - - `GENERATION_FAILED: ` - if it failed + Read `prompts/workflow-prompts/impl-generate-claude.md` and follow those instructions. + + Variables for this run: + - LIBRARY: ${{ steps.inputs.outputs.library }} + - SPEC_ID: ${{ steps.inputs.outputs.specification_id }} + - IS_REGENERATION: ${{ steps.existing.outputs.is_regeneration }} - name: Retry Claude (on failure) if: steps.claude.outcome == 'failure' @@ -335,78 +269,12 @@ jobs: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} claude_args: "--model opus" prompt: | - ## Task: Generate ${{ steps.inputs.outputs.library }} Implementation - - You are generating the **${{ steps.inputs.outputs.library }}** implementation for **${{ steps.inputs.outputs.specification_id }}**. - - **Regeneration:** ${{ steps.existing.outputs.is_regeneration }} - - ### Step 1: Read required files - 1. `prompts/plot-generator.md` - Base generation rules (IMPORTANT: Read the "Regeneration" section!) - 2. `prompts/default-style-guide.md` - Visual style requirements - 3. `prompts/quality-criteria.md` - Quality requirements - 4. `prompts/library/${{ steps.inputs.outputs.library }}.md` - Library-specific rules - 5. `plots/${{ steps.inputs.outputs.specification_id }}/specification.md` - The specification - - ### Step 1b: If Regeneration, read previous feedback - If this is a regeneration (${{ steps.existing.outputs.is_regeneration }} == true): - 1. Read `plots/${{ steps.inputs.outputs.specification_id }}/metadata/${{ steps.inputs.outputs.library }}.yaml` - - Look at `review.strengths` (keep these aspects!) - - Look at `review.weaknesses` (fix these problems - decide HOW yourself) - - Look at `review.image_description` (understand what was generated visually) - - Look at `review.criteria_checklist` (see exactly which criteria failed) - - Focus on categories with low scores (e.g., visual_quality.score < visual_quality.max) - - Check items with `passed: false` - these need fixing - - VQ-XX items for visual issues - - SC-XX items for spec compliance - - CQ-XX items for code quality - 2. Read `plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py` - - Understand what was done before - - Keep what worked, fix what didn't - - ### Step 2: Generate implementation - Create: `plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py` - - The script MUST: - - Save as `plot.png` in the current directory - - For interactive libraries (plotly, bokeh, altair, highcharts, pygal, letsplot): also save `plot.html` - - ### Step 3: Test and fix (up to 3 attempts) - Run the implementation: - ```bash - source .venv/bin/activate - cd plots/${{ steps.inputs.outputs.specification_id }}/implementations - MPLBACKEND=Agg python ${{ steps.inputs.outputs.library }}.py - ``` - - If it fails, fix and try again (max 3 attempts). - - ### Step 4: Visual self-check - Look at the generated `plot.png`: - - Does it match the specification? - - Are axes labeled correctly? - - Is the visualization clear? - - ### Step 5: Format the code - ```bash - source .venv/bin/activate - ruff format plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py - ruff check --fix plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py - ``` - - ### Step 6: Commit - ```bash - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - git add plots/${{ steps.inputs.outputs.specification_id }}/implementations/${{ steps.inputs.outputs.library }}.py - git commit -m "feat(${{ steps.inputs.outputs.library }}): implement ${{ steps.inputs.outputs.specification_id }}" - git push -u origin implementation/${{ steps.inputs.outputs.specification_id }}/${{ steps.inputs.outputs.library }} - ``` - - ### Report result - Print exactly one line: - - `GENERATION_SUCCESS` - if everything worked - - `GENERATION_FAILED: ` - if it failed + Read `prompts/workflow-prompts/impl-generate-claude.md` and follow those instructions. + + Variables for this run: + - LIBRARY: ${{ steps.inputs.outputs.library }} + - SPEC_ID: ${{ steps.inputs.outputs.specification_id }} + - IS_REGENERATION: ${{ steps.existing.outputs.is_regeneration }} # ======================================================================== # Create metadata file (before PR) diff --git a/.github/workflows/impl-repair.yml b/.github/workflows/impl-repair.yml index 1bc279eb88..bb04e55582 100644 --- a/.github/workflows/impl-repair.yml +++ b/.github/workflows/impl-repair.yml @@ -117,68 +117,13 @@ jobs: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} claude_args: "--model opus" prompt: | - ## Task: Repair ${{ inputs.library }} Implementation for ${{ inputs.specification_id }} - - This is **repair attempt ${{ inputs.attempt }}/3**. The previous implementation was rejected. - - ### Step 1: Read the AI review feedback - Read both sources to understand what needs to be fixed: - 1. `/tmp/ai_feedback.md` - Full review from PR comments - 2. `plots/${{ inputs.specification_id }}/metadata/${{ inputs.library }}.yaml` - Look at: - - `review.strengths` (keep these aspects!) - - `review.weaknesses` (fix these problems - decide HOW yourself) - - `review.image_description` (understand what was generated visually) - - `review.criteria_checklist` (see exactly which criteria failed) - - Look for items with `passed: false` - these need fixing - - Focus on categories with low scores (e.g., visual_quality.score < visual_quality.max) - - VQ-XX items for visual issues - - SC-XX items for spec compliance - - CQ-XX items for code quality - - ### Step 2: Read reference files - 1. `prompts/library/${{ inputs.library }}.md` - Library-specific rules - 2. `plots/${{ inputs.specification_id }}/specification.md` - The specification - 3. `prompts/quality-criteria.md` - Quality requirements - - ### Step 3: Read current implementation - `plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py` - - ### Step 4: Fix the issues - Based on the AI feedback, fix: - - Visual quality issues - - Code quality issues - - Spec compliance issues - - ### Step 5: Test the fix - ```bash - source .venv/bin/activate - cd plots/${{ inputs.specification_id }}/implementations - MPLBACKEND=Agg python ${{ inputs.library }}.py - ``` - - ### Step 6: Visual self-check - View `plot.png` and verify fixes are correct. - - ### Step 7: Format the code - ```bash - source .venv/bin/activate - ruff format plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py - ruff check --fix plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py - ``` - - ### Step 8: Commit and push - ```bash - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - git add plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py - git commit -m "fix(${{ inputs.library }}): address review feedback for ${{ inputs.specification_id }} - - Attempt ${{ inputs.attempt }}/3 - fixes based on AI review" - git push origin ${{ env.branch }} - ``` - - ### Report result - Print: `REPAIR_SUCCESS` or `REPAIR_FAILED: ` + Read `prompts/workflow-prompts/impl-repair-claude.md` and follow those instructions. + + Variables for this run: + - LIBRARY: ${{ inputs.library }} + - SPEC_ID: ${{ inputs.specification_id }} + - ATTEMPT: ${{ inputs.attempt }} + - BRANCH: ${{ env.branch }} - name: Retry Claude (on failure) if: steps.claude.outcome == 'failure' @@ -189,68 +134,13 @@ jobs: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} claude_args: "--model opus" prompt: | - ## Task: Repair ${{ inputs.library }} Implementation for ${{ inputs.specification_id }} - - This is **repair attempt ${{ inputs.attempt }}/3**. The previous implementation was rejected. - - ### Step 1: Read the AI review feedback - Read both sources to understand what needs to be fixed: - 1. `/tmp/ai_feedback.md` - Full review from PR comments - 2. `plots/${{ inputs.specification_id }}/metadata/${{ inputs.library }}.yaml` - Look at: - - `review.strengths` (keep these aspects!) - - `review.weaknesses` (fix these problems - decide HOW yourself) - - `review.image_description` (understand what was generated visually) - - `review.criteria_checklist` (see exactly which criteria failed) - - Look for items with `passed: false` - these need fixing - - Focus on categories with low scores (e.g., visual_quality.score < visual_quality.max) - - VQ-XX items for visual issues - - SC-XX items for spec compliance - - CQ-XX items for code quality - - ### Step 2: Read reference files - 1. `prompts/library/${{ inputs.library }}.md` - Library-specific rules - 2. `plots/${{ inputs.specification_id }}/specification.md` - The specification - 3. `prompts/quality-criteria.md` - Quality requirements - - ### Step 3: Read current implementation - `plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py` - - ### Step 4: Fix the issues - Based on the AI feedback, fix: - - Visual quality issues - - Code quality issues - - Spec compliance issues - - ### Step 5: Test the fix - ```bash - source .venv/bin/activate - cd plots/${{ inputs.specification_id }}/implementations - MPLBACKEND=Agg python ${{ inputs.library }}.py - ``` - - ### Step 6: Visual self-check - View `plot.png` and verify fixes are correct. - - ### Step 7: Format the code - ```bash - source .venv/bin/activate - ruff format plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py - ruff check --fix plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py - ``` - - ### Step 8: Commit and push - ```bash - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - git add plots/${{ inputs.specification_id }}/implementations/${{ inputs.library }}.py - git commit -m "fix(${{ inputs.library }}): address review feedback for ${{ inputs.specification_id }} - - Attempt ${{ inputs.attempt }}/3 - fixes based on AI review" - git push origin ${{ env.branch }} - ``` - - ### Report result - Print: `REPAIR_SUCCESS` or `REPAIR_FAILED: ` + Read `prompts/workflow-prompts/impl-repair-claude.md` and follow those instructions. + + Variables for this run: + - LIBRARY: ${{ inputs.library }} + - SPEC_ID: ${{ inputs.specification_id }} + - ATTEMPT: ${{ inputs.attempt }} + - BRANCH: ${{ env.branch }} - name: Process repaired image if: success() diff --git a/app/src/components/ErrorBoundary.tsx b/app/src/components/ErrorBoundary.tsx new file mode 100644 index 0000000000..0c8b543179 --- /dev/null +++ b/app/src/components/ErrorBoundary.tsx @@ -0,0 +1,122 @@ +import { Component, ReactNode } from 'react'; +import { Box, Alert, Button, Typography } from '@mui/material'; +import RefreshIcon from '@mui/icons-material/Refresh'; +import HomeIcon from '@mui/icons-material/Home'; + +interface Props { + children: ReactNode; + fallback?: ReactNode; +} + +interface State { + hasError: boolean; + error: Error | null; +} + +/** + * Error boundary component that catches rendering errors in child components. + * Displays a user-friendly error message with recovery options. + */ +export class ErrorBoundary extends Component { + constructor(props: Props) { + super(props); + this.state = { hasError: false, error: null }; + } + + static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; + } + + componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { + console.error('ErrorBoundary caught an error:', error, errorInfo); + } + + handleReload = (): void => { + window.location.reload(); + }; + + handleGoHome = (): void => { + window.location.href = '/'; + }; + + handleRetry = (): void => { + this.setState({ hasError: false, error: null }); + }; + + render(): ReactNode { + if (this.state.hasError) { + if (this.props.fallback) { + return this.props.fallback; + } + + return ( + + + + Something went wrong + + + An unexpected error occurred. Please try reloading the page. + + {import.meta.env.DEV && this.state.error && ( + + {this.state.error.message} + + )} + + + + + + + + ); + } + + return this.props.children; + } +} diff --git a/app/src/router.tsx b/app/src/router.tsx index 1146167825..98eec870c0 100644 --- a/app/src/router.tsx +++ b/app/src/router.tsx @@ -1,6 +1,7 @@ import { createBrowserRouter, RouterProvider } from 'react-router-dom'; import { HelmetProvider } from 'react-helmet-async'; import { Layout, AppDataProvider } from './components/Layout'; +import { ErrorBoundary } from './components/ErrorBoundary'; import { HomePage } from './pages/HomePage'; import { SpecPage } from './pages/SpecPage'; import { CatalogPage } from './pages/CatalogPage'; @@ -27,9 +28,11 @@ const router = createBrowserRouter([ export function AppRouter() { return ( - - - + + + + + ); } diff --git a/core/database/repositories.py b/core/database/repositories.py index 5ca8285dfa..d5bece0c09 100644 --- a/core/database/repositories.py +++ b/core/database/repositories.py @@ -144,7 +144,7 @@ async def delete(self, spec_id: str) -> bool: if not spec: return False - await self.session.delete(spec) + self.session.delete(spec) await self.session.commit() return True @@ -259,7 +259,7 @@ async def delete(self, library_id: str) -> bool: if not library: return False - await self.session.delete(library) + self.session.delete(library) await self.session.commit() return True @@ -398,7 +398,7 @@ async def delete(self, impl_id: str) -> bool: if not impl: return False - await self.session.delete(impl) + self.session.delete(impl) await self.session.commit() return True diff --git a/prompts/workflow-prompts/generate-implementation.md b/prompts/workflow-prompts/generate-implementation.md deleted file mode 100644 index 2c6e5aceb0..0000000000 --- a/prompts/workflow-prompts/generate-implementation.md +++ /dev/null @@ -1,58 +0,0 @@ -# Generate Library Implementation - -Generate a **${LIBRARY}** implementation for the plot specification `${SPEC_ID}`. - -## Instructions - -You are generating ONLY the **${LIBRARY}** implementation. Focus exclusively on this library. - -### Step 1: Read Required Files - -1. `prompts/plot-generator.md` - Base generation rules -2. `prompts/quality-criteria.md` - Quality requirements -3. `prompts/library/${LIBRARY}.md` - Library-specific rules -4. `plots/${SPEC_ID}/spec.md` - The specification - -### Step 2: Check for Previous Attempts - -${PREVIOUS_ATTEMPTS_CONTEXT} - -### Step 3: Generate Implementation - -Create the implementation file at the correct path: -``` -plots/${SPEC_ID}/implementations/${LIBRARY}.py -``` - -### Step 4: Test the Implementation - -Run the implementation to verify it works: -```bash -source .venv/bin/activate -MPLBACKEND=Agg python plots/${SPEC_ID}/implementations/${LIBRARY}.py -``` - -### Step 5: Create PR - -Only if the implementation is successful: - -- **Branch:** `auto/${SPEC_ID}/${LIBRARY}` -- **Title:** `feat(${LIBRARY}): implement ${SPEC_ID}` -- **Body:** -```markdown -## Summary -Implements `${SPEC_ID}` for **${LIBRARY}** library. - -**Parent Issue:** #${MAIN_ISSUE_NUMBER} -**Sub-Issue:** #${SUB_ISSUE_NUMBER} -**Attempt:** ${ATTEMPT}/3 - -## Implementation -- `plots/${SPEC_ID}/implementations/${LIBRARY}.py` -``` - -## Important Notes - -- Focus ONLY on ${LIBRARY} - do not generate code for other libraries -- If you cannot implement this plot type in ${LIBRARY}, explain why in the PR body -- Document any limitations or workarounds in code comments diff --git a/prompts/workflow-prompts/impl-generate-claude.md b/prompts/workflow-prompts/impl-generate-claude.md new file mode 100644 index 0000000000..5c003d434c --- /dev/null +++ b/prompts/workflow-prompts/impl-generate-claude.md @@ -0,0 +1,81 @@ +# Generate Implementation + +You are generating the **{LIBRARY}** implementation for **{SPEC_ID}**. + +**Regeneration:** {IS_REGENERATION} + +## Step 1: Read required files + +1. `prompts/plot-generator.md` - Base generation rules (IMPORTANT: Read the "Regeneration" section!) +2. `prompts/default-style-guide.md` - Visual style requirements +3. `prompts/quality-criteria.md` - Quality requirements +4. `prompts/library/{LIBRARY}.md` - Library-specific rules +5. `plots/{SPEC_ID}/specification.md` - The specification + +## Step 1b: If Regeneration, read previous feedback + +If this is a regeneration ({IS_REGENERATION} == true): + +1. Read `plots/{SPEC_ID}/metadata/{LIBRARY}.yaml` + - Look at `review.strengths` (keep these aspects!) + - Look at `review.weaknesses` (fix these problems - decide HOW yourself) + - Look at `review.image_description` (understand what was generated visually) + - Look at `review.criteria_checklist` (see exactly which criteria failed) + - Focus on categories with low scores (e.g., visual_quality.score < visual_quality.max) + - Check items with `passed: false` - these need fixing + - VQ-XX items for visual issues + - SC-XX items for spec compliance + - CQ-XX items for code quality +2. Read `plots/{SPEC_ID}/implementations/{LIBRARY}.py` + - Understand what was done before + - Keep what worked, fix what didn't + +## Step 2: Generate implementation + +Create: `plots/{SPEC_ID}/implementations/{LIBRARY}.py` + +The script MUST: +- Save as `plot.png` in the current directory +- For interactive libraries (plotly, bokeh, altair, highcharts, pygal, letsplot): also save `plot.html` + +## Step 3: Test and fix (up to 3 attempts) + +Run the implementation: +```bash +source .venv/bin/activate +cd plots/{SPEC_ID}/implementations +MPLBACKEND=Agg python {LIBRARY}.py +``` + +If it fails, fix and try again (max 3 attempts). + +## Step 4: Visual self-check + +Look at the generated `plot.png`: +- Does it match the specification? +- Are axes labeled correctly? +- Is the visualization clear? + +## Step 5: Format the code + +```bash +source .venv/bin/activate +ruff format plots/{SPEC_ID}/implementations/{LIBRARY}.py +ruff check --fix plots/{SPEC_ID}/implementations/{LIBRARY}.py +``` + +## Step 6: Commit + +```bash +git config user.name "github-actions[bot]" +git config user.email "github-actions[bot]@users.noreply.github.com" +git add plots/{SPEC_ID}/implementations/{LIBRARY}.py +git commit -m "feat({LIBRARY}): implement {SPEC_ID}" +git push -u origin implementation/{SPEC_ID}/{LIBRARY} +``` + +## Report result + +Print exactly one line: +- `GENERATION_SUCCESS` - if everything worked +- `GENERATION_FAILED: ` - if it failed diff --git a/prompts/workflow-prompts/impl-repair-claude.md b/prompts/workflow-prompts/impl-repair-claude.md new file mode 100644 index 0000000000..ca4ca5d063 --- /dev/null +++ b/prompts/workflow-prompts/impl-repair-claude.md @@ -0,0 +1,74 @@ +# Repair Implementation + +You are repairing the **{LIBRARY}** implementation for **{SPEC_ID}**. + +This is **repair attempt {ATTEMPT}/3**. The previous implementation was rejected. + +## Step 1: Read the AI review feedback + +Read both sources to understand what needs to be fixed: + +1. `/tmp/ai_feedback.md` - Full review from PR comments +2. `plots/{SPEC_ID}/metadata/{LIBRARY}.yaml` - Look at: + - `review.strengths` (keep these aspects!) + - `review.weaknesses` (fix these problems - decide HOW yourself) + - `review.image_description` (understand what was generated visually) + - `review.criteria_checklist` (see exactly which criteria failed) + - Look for items with `passed: false` - these need fixing + - Focus on categories with low scores (e.g., visual_quality.score < visual_quality.max) + - VQ-XX items for visual issues + - SC-XX items for spec compliance + - CQ-XX items for code quality + +## Step 2: Read reference files + +1. `prompts/library/{LIBRARY}.md` - Library-specific rules +2. `plots/{SPEC_ID}/specification.md` - The specification +3. `prompts/quality-criteria.md` - Quality requirements + +## Step 3: Read current implementation + +`plots/{SPEC_ID}/implementations/{LIBRARY}.py` + +## Step 4: Fix the issues + +Based on the AI feedback, fix: +- Visual quality issues +- Code quality issues +- Spec compliance issues + +## Step 5: Test the fix + +```bash +source .venv/bin/activate +cd plots/{SPEC_ID}/implementations +MPLBACKEND=Agg python {LIBRARY}.py +``` + +## Step 6: Visual self-check + +View `plot.png` and verify fixes are correct. + +## Step 7: Format the code + +```bash +source .venv/bin/activate +ruff format plots/{SPEC_ID}/implementations/{LIBRARY}.py +ruff check --fix plots/{SPEC_ID}/implementations/{LIBRARY}.py +``` + +## Step 8: Commit and push + +```bash +git config user.name "github-actions[bot]" +git config user.email "github-actions[bot]@users.noreply.github.com" +git add plots/{SPEC_ID}/implementations/{LIBRARY}.py +git commit -m "fix({LIBRARY}): address review feedback for {SPEC_ID} + +Attempt {ATTEMPT}/3 - fixes based on AI review" +git push origin {BRANCH} +``` + +## Report result + +Print: `REPAIR_SUCCESS` or `REPAIR_FAILED: ` diff --git a/prompts/workflow-prompts/improve-from-feedback.md b/prompts/workflow-prompts/improve-from-feedback.md deleted file mode 100644 index a38b000e79..0000000000 --- a/prompts/workflow-prompts/improve-from-feedback.md +++ /dev/null @@ -1,57 +0,0 @@ -# Improve Implementation from Feedback - -You are improving a **${LIBRARY}** plot implementation based on AI review feedback. - -## Context - -- **Library:** ${LIBRARY} -- **Spec ID:** ${SPEC_ID} -- **Attempt:** ${ATTEMPT}/3 -- **Plot File:** ${PLOT_FILE} - -## Current Code - -```python -${CURRENT_CODE} -``` - -## Specification - -${SPEC_CONTENT} - -## Library-Specific Rules - -${LIBRARY_RULES} - -## Previous Attempts and Feedback - -**IMPORTANT:** Learn from previous attempts to avoid repeating the same mistakes. - -${ALL_ATTEMPTS} - -## Latest AI Review Feedback - -${LATEST_FEEDBACK} - -## Task - -Improve the code to address ALL feedback from the latest review. - -### Priority Checklist - -Address issues by area (see `prompts/quality-criteria.md`): - -1. **Spec Compliance (SC-xx)** - Fix plot type, data mapping, missing features, title format -2. **Visual Quality (VQ-xx)** - Fix labels, overlaps, colors, layout issues -3. **Code Quality (CQ-xx)** - Fix structure, imports, reproducibility, idioms -4. **Previous Mistakes** - Review previous attempts to avoid repeating the same errors -5. **All Feedback** - Implement all suggestions from the latest review - -## Output - -Return ONLY the improved Python code. - -- No explanations -- No markdown formatting -- No ```python blocks -- Just the raw Python code diff --git a/tests/unit/api/test_analytics.py b/tests/unit/api/test_analytics.py index 40a9261c31..41068b5251 100644 --- a/tests/unit/api/test_analytics.py +++ b/tests/unit/api/test_analytics.py @@ -137,75 +137,91 @@ def mock_request(self) -> MagicMock: request.client.host = "127.0.0.1" return request - def test_creates_async_task(self, mock_request: MagicMock) -> None: + @pytest.fixture + def mock_create_task(self) -> MagicMock: + """Create a mock for asyncio.create_task that properly closes coroutines. + + When asyncio.create_task is mocked, the coroutine passed to it is never + executed. This causes 'coroutine was never awaited' warnings. This fixture + creates a mock that closes the coroutine to prevent the warning. + """ + + def close_coroutine(coro: object) -> MagicMock: + """Close the coroutine to prevent 'never awaited' warning.""" + if hasattr(coro, "close"): + coro.close() + return MagicMock() + + mock = MagicMock(side_effect=close_coroutine) + return mock + + def test_creates_async_task(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should create background task without blocking.""" - with patch("api.analytics.asyncio.create_task") as mock_create_task: + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(mock_request, page="home") mock_create_task.assert_called_once() - def test_home_page_url(self, mock_request: MagicMock) -> None: + def test_home_page_url(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should build correct URL for home page.""" - with patch("api.analytics.asyncio.create_task") as mock_create_task: + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(mock_request, page="home") - call_args = mock_create_task.call_args[0][0] - # The coroutine should be called with home URL - assert call_args is not None + # Verify create_task was called with a coroutine + mock_create_task.assert_called_once() - def test_catalog_page_url(self, mock_request: MagicMock) -> None: + def test_catalog_page_url(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should build correct URL for catalog page.""" - with patch("api.analytics._send_plausible_event", new_callable=AsyncMock): - with patch("api.analytics.asyncio.create_task") as mock_create_task: - track_og_image(mock_request, page="catalog") - mock_create_task.assert_called_once() + with patch("api.analytics.asyncio.create_task", mock_create_task): + track_og_image(mock_request, page="catalog") + mock_create_task.assert_called_once() - def test_spec_overview_url(self, mock_request: MagicMock) -> None: + def test_spec_overview_url(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should build correct URL for spec overview.""" - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): # Should not raise even with spec_overview page track_og_image(mock_request, page="spec_overview", spec="scatter-basic") - def test_spec_detail_url(self, mock_request: MagicMock) -> None: + def test_spec_detail_url(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should build correct URL for spec detail.""" - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(mock_request, page="spec_detail", spec="scatter-basic", library="matplotlib") - def test_fallback_url_when_spec_none(self, mock_request: MagicMock) -> None: + def test_fallback_url_when_spec_none(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should fallback to home URL when spec is None for spec-based page.""" - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): # Should not raise - falls back to home URL track_og_image(mock_request, page="spec_overview", spec=None) - def test_includes_filter_props(self, mock_request: MagicMock) -> None: + def test_includes_filter_props(self, mock_request: MagicMock, mock_create_task: MagicMock) -> None: """Should include filter parameters in props.""" - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(mock_request, page="home", filters={"lib": "plotly", "dom": "statistics"}) - def test_uses_x_forwarded_for(self) -> None: + def test_uses_x_forwarded_for(self, mock_create_task: MagicMock) -> None: """Should use X-Forwarded-For header for client IP.""" request = MagicMock() request.headers = {"user-agent": "Twitterbot/1.0", "x-forwarded-for": "5.6.7.8"} request.client = None - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(request, page="home") - def test_fallback_to_client_host(self) -> None: + def test_fallback_to_client_host(self, mock_create_task: MagicMock) -> None: """Should fallback to client.host when X-Forwarded-For not present.""" request = MagicMock() request.headers = {"user-agent": "Twitterbot/1.0"} request.client = MagicMock() request.client.host = "10.0.0.1" - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(request, page="home") - def test_handles_missing_client(self) -> None: + def test_handles_missing_client(self, mock_create_task: MagicMock) -> None: """Should handle missing client gracefully.""" request = MagicMock() request.headers = {"user-agent": "Twitterbot/1.0"} request.client = None - with patch("api.analytics.asyncio.create_task"): + with patch("api.analytics.asyncio.create_task", mock_create_task): track_og_image(request, page="home") From 906268722b3a5cd7bfcadd44e54d64e1df028500 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 11 Jan 2026 00:50:58 +0100 Subject: [PATCH 2/5] refactor: improve code quality and structure - Enhance project documentation for better understanding - Introduce ErrorBoundary component for error handling in React - Refactor repository methods to remove unnecessary await - Update test cases to improve clarity and maintainability --- .claude/commands/check.md | 30 +++++ .claude/commands/prime.md | 49 ++++++-- .claude/commands/refactor.md | 217 +++++++++++++++++++++++++++++++++++ 3 files changed, 287 insertions(+), 9 deletions(-) create mode 100644 .claude/commands/check.md create mode 100644 .claude/commands/refactor.md diff --git a/.claude/commands/check.md b/.claude/commands/check.md new file mode 100644 index 0000000000..9fd7ddbba3 --- /dev/null +++ b/.claude/commands/check.md @@ -0,0 +1,30 @@ +# Quick Code Check + +> Fast code quality check for recent changes. Use `/refactor` for comprehensive analysis. + +## Run Checks + +```bash +# Lint check +uv run ruff check . + +# Format check +uv run ruff format . --check && echo "Formatting OK" || echo "Formatting issues found" + +# Quick unit tests +uv run pytest tests/unit -x -q --tb=short 2>/dev/null | tail -30 +``` + +## Check Recent Changes + +```bash +# Files changed vs main +git diff --name-only main... 2>/dev/null || git diff --name-only HEAD~5 +``` + +## Summary + +Provide a brief summary: +1. Any lint/format issues found +2. Test status +3. Quick recommendations for the changed files diff --git a/.claude/commands/prime.md b/.claude/commands/prime.md index b497f04fc8..09ff533fdb 100644 --- a/.claude/commands/prime.md +++ b/.claude/commands/prime.md @@ -1,21 +1,52 @@ # Prime -> Execute the following sections to understand the codebase then summarize your understanding. +> Quickly understand the pyplots codebase - structure, rules, and current state. -## Run +## Project Rules -git ls-files +@CLAUDE.md -## Read +## Project Config -@README.md @pyproject.toml -@docs/concepts/vision.md -@docs/workflows/overview.md -@docs/reference/repository.md -## Read and Execute +## Quick Stats +```bash +# Repository overview +echo "=== Repository Stats ===" +echo "Plot specifications: $(ls -d plots/*/ 2>/dev/null | wc -l)" +echo "Python files: $(find api core automation tests -name '*.py' 2>/dev/null | wc -l)" +echo "Frontend files: $(find app/src -name '*.tsx' -o -name '*.ts' 2>/dev/null | wc -l)" +echo "Workflows: $(ls .github/workflows/*.yml 2>/dev/null | wc -l)" +echo "Prompts: $(find prompts -name '*.md' 2>/dev/null | wc -l)" +``` +## Structure +```bash +# Key directories +ls -la api/ core/ app/src/ prompts/ .github/workflows/ 2>/dev/null | head -50 +# Documentation +find docs -name "*.md" 2>/dev/null | sort +``` + +## Current State + +```bash +# Git status +git status --short + +# Recent activity +git log --oneline -10 +``` + +## Summarize + +After reading, provide: +1. **Purpose**: What does this project do? +2. **Architecture**: Key components and how they connect +3. **Workflow**: How specs become implementations +4. **Tech Stack**: Languages, frameworks, infrastructure +5. **Key Rules**: Critical constraints from CLAUDE.md diff --git a/.claude/commands/refactor.md b/.claude/commands/refactor.md new file mode 100644 index 0000000000..60560e8090 --- /dev/null +++ b/.claude/commands/refactor.md @@ -0,0 +1,217 @@ +# Code Quality Refactor + +> Comprehensive code quality analysis for the pyplots repository. Checks readability, performance, code smells, consistency, and modernization opportunities across all components. + +## Context + +@CLAUDE.md +@pyproject.toml + +## Analysis Scope + +Analyze the following areas systematically. For each area, identify: +- **Code Smells**: Dead code, duplication, overly complex functions, god classes +- **Readability**: Unclear naming, missing/outdated comments, inconsistent formatting +- **Performance**: Inefficient patterns, N+1 queries, unnecessary computations +- **Modernization**: Deprecated APIs, old Python patterns, outdated dependencies +- **Consistency**: Naming conventions, import styles, error handling patterns +- **Type Safety**: Missing type hints, incorrect types, Any overuse +- **Test Quality**: Missing tests, flaky tests, poor assertions + +## Discover Structure + +```bash +# Overview of repository structure +find . -type f -name "*.py" | grep -E "^\./(api|core|automation|tests)/" | head -80 + +# Frontend structure +find ./app/src -type f -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -40 + +# Workflow files +ls -la .github/workflows/*.yml 2>/dev/null + +# Prompt files +find ./prompts -type f -name "*.md" 2>/dev/null + +# Documentation +find ./docs -type f -name "*.md" 2>/dev/null +``` + +## Run Quality Checks + +```bash +# Linting issues +uv run ruff check . + +# Format check (dry-run) +uv run ruff format . --check --diff +``` + +## Analyze Backend (api/) + +Check for: +- Router organization and REST conventions +- Dependency injection patterns +- Error handling consistency +- Response schema completeness +- Async/await correctness +- Caching strategies + +```bash +ls -la api/*.py api/**/*.py 2>/dev/null +``` + +## Analyze Core Logic (core/) + +Check for: +- Repository pattern implementation +- Database model design +- Configuration management +- Utility function organization +- Type definitions + +```bash +ls -la core/*.py core/**/*.py 2>/dev/null +``` + +## Analyze Automation (automation/) + +Check for: +- Script modularity +- Error handling in workflows +- CLI argument parsing +- Logging consistency + +```bash +ls -la automation/**/*.py 2>/dev/null +``` + +## Analyze Frontend (app/src/) + +Check for: +- Component structure and reusability +- Hook patterns and custom hooks +- TypeScript strictness (no `any`, proper interfaces) +- Performance (memo, useCallback only where truly needed) +- Accessibility (aria-labels, keyboard navigation, focus management) +- Consistent error handling and error boundaries +- Loading states and skeletons +- Unused imports and dead code + +```bash +ls -la app/src/**/*.tsx app/src/**/*.ts 2>/dev/null | head -30 +``` + +## Analyze Tests (tests/) + +Check for: +- Test coverage gaps +- Fixture organization +- Mock patterns +- Assertion quality +- Test naming conventions + +```bash +find tests -name "*.py" -type f | head -30 +``` + +## Analyze GitHub Workflows (.github/workflows/) + +Check for: +- Workflow consistency and naming +- Job dependencies and parallelization +- Secret handling and security +- Error handling and failure modes +- Concurrency settings +- Reusable workflows vs duplication +- Environment variable consistency +- Trigger conditions (labels, paths, branches) + +```bash +ls -la .github/workflows/*.yml +``` + +## Analyze AI Prompts (prompts/) + +Check for: +- Prompt clarity and structure +- Consistency across prompt files +- Outdated instructions or references +- Missing edge cases +- Template completeness +- Library-specific rules alignment + +```bash +find prompts -name "*.md" -o -name "*.yaml" | sort +``` + +## Analyze Documentation (docs/) + +Check for: +- Outdated information vs actual code +- Missing documentation for new features +- Broken internal links +- Inconsistent formatting +- Sync with CLAUDE.md + +```bash +find docs -name "*.md" | sort +``` + +## Cross-Check: Consistency + +Verify alignment across components: +- Do workflows reference existing prompt files? +- Are library names consistent across `prompts/library/`, `core/constants.py`, workflows? +- Do workflow labels match documentation? +- Are schema definitions in sync between API and frontend types? + +```bash +# Check prompt references in workflows +grep -r "prompts/" .github/workflows/ 2>/dev/null | head -20 + +# Library definitions +ls prompts/library/ 2>/dev/null | sed 's/.md//' | sort +grep -E "SUPPORTED_LIBRARIES|LIBRARIES" core/constants.py 2>/dev/null | head -5 +``` + +## Run Test Suite + +```bash +# Quick test run to check for failures +uv run pytest tests/unit -x -q --tb=no 2>/dev/null | tail -20 +``` + +## Output Format + +After analysis, provide a structured report: + +### 1. Critical Issues (Must Fix) +Issues that could cause bugs, security problems, or break functionality. + +### 2. High Priority (Should Fix) +Code smells, performance issues, or maintainability concerns. + +### 3. Medium Priority (Consider) +Modernization opportunities, style improvements, minor inconsistencies. + +### 4. Low Priority (Nice to Have) +Minor improvements, documentation updates, cosmetic changes. + +### 5. Positive Patterns +Good practices found that should be maintained or expanded. + +For each issue, provide: +- **Location**: File path and line number +- **Problem**: Clear description of the issue +- **Impact**: Why this matters +- **Solution**: Specific fix recommendation +- **Effort**: Low/Medium/High + +## Exclusions + +Do NOT flag: +- Plot implementations in `plots/` (AI-generated, different style rules) +- Generated files or lock files (`uv.lock`, `yarn.lock`, etc.) +- Third-party code +- Issues already tracked in pyproject.toml exclusions From b929ba98efb7482547793761a8105ca4a97a6998 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 11 Jan 2026 00:56:25 +0100 Subject: [PATCH 3/5] fix: revert await removal - AsyncSession.delete() is async Co-Authored-By: Claude Opus 4.5 --- core/database/repositories.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/database/repositories.py b/core/database/repositories.py index d5bece0c09..5ca8285dfa 100644 --- a/core/database/repositories.py +++ b/core/database/repositories.py @@ -144,7 +144,7 @@ async def delete(self, spec_id: str) -> bool: if not spec: return False - self.session.delete(spec) + await self.session.delete(spec) await self.session.commit() return True @@ -259,7 +259,7 @@ async def delete(self, library_id: str) -> bool: if not library: return False - self.session.delete(library) + await self.session.delete(library) await self.session.commit() return True @@ -398,7 +398,7 @@ async def delete(self, impl_id: str) -> bool: if not impl: return False - self.session.delete(impl) + await self.session.delete(impl) await self.session.commit() return True From a23422e5ad478a7f11d7cea5fd29a01b0b03f43a Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 11 Jan 2026 00:58:45 +0100 Subject: [PATCH 4/5] fix: use handleRetry method in ErrorBoundary Add "Try Again" button that calls handleRetry to reset error state without a full page reload. Addresses Copilot review comment about unused method. Co-Authored-By: Claude Opus 4.5 --- app/src/components/ErrorBoundary.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/src/components/ErrorBoundary.tsx b/app/src/components/ErrorBoundary.tsx index 0c8b543179..9500260744 100644 --- a/app/src/components/ErrorBoundary.tsx +++ b/app/src/components/ErrorBoundary.tsx @@ -1,6 +1,7 @@ import { Component, ReactNode } from 'react'; import { Box, Alert, Button, Typography } from '@mui/material'; import RefreshIcon from '@mui/icons-material/Refresh'; +import ReplayIcon from '@mui/icons-material/Replay'; import HomeIcon from '@mui/icons-material/Home'; interface Props { @@ -95,9 +96,17 @@ export class ErrorBoundary extends Component { )} - + +