Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 39 additions & 32 deletions .github/ISSUE_TEMPLATE/plot-update.yml
Original file line number Diff line number Diff line change
@@ -1,66 +1,73 @@
name: Plot Update
description: Request changes or improvements to an existing plot
title: "Update: "
labels: ["plot-update"]
description: Request updates or regeneration of an existing plot
title: "[update] "
labels: ["plot-request", "update"]
body:
- type: markdown
attributes:
value: |
## Plot Update Request

Use this form to request changes to an existing plot implementation.
This will be linked as a sub-issue to the original plot request.
Use this form to update or regenerate an existing plot.

- type: input
id: parent_issue
attributes:
label: Original Plot Request
description: "Issue number of the original plot-request (e.g., #24)"
placeholder: "#24"
validations:
required: true
**Title Format:**
- `[update] scatter-basic` → Regenerate all 8 libraries
- `[update:seaborn] scatter-basic` → Regenerate only seaborn

After submitting, a maintainer will add the `approved` label to trigger regeneration.

- type: input
id: spec_id
attributes:
label: Spec ID
description: "The spec ID of the plot to update (e.g., bar-basic)"
placeholder: "bar-basic"
description: "The spec ID of the plot to update (will be appended to title)"
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The description states "will be appended to title" but GitHub issue templates don't automatically append form field values to the title. Users must manually type the spec ID into the title after the [update] prefix.

Suggestion: Update the description to be clearer:

description: "The spec ID of the plot to update (must be manually added to the title above)"
Suggested change
description: "The spec ID of the plot to update (will be appended to title)"
description: "The spec ID of the plot to update (must be manually added to the title above)"

Copilot uses AI. Check for mistakes.
placeholder: "scatter-basic"
validations:
required: true

- type: dropdown
id: target_library
attributes:
label: Target Library (optional)
description: Update only a specific library, or leave empty for all
options:
- All libraries
- matplotlib
- seaborn
- plotly
- bokeh
- altair
- plotnine
- pygal
- highcharts
validations:
required: false
Comment thread
MarkusNeusinger marked this conversation as resolved.

- type: dropdown
id: update_type
attributes:
label: Type of Update
description: What kind of change is this?
options:
- Regenerate (use current spec)
- Spec Update (describe changes below)
- Bug Fix (plot doesn't render correctly)
- Visual Improvement (styling, colors, layout)
- Feature Addition (new parameter, option)
- Performance (faster rendering, less memory)
- Code Quality (refactoring, better practices)
- Documentation (docstrings, comments)
validations:
required: true

- type: textarea
id: current_behavior
id: changes
attributes:
label: Current Behavior
description: Describe what the plot currently does
placeholder: "The bar chart currently shows labels overlapping when there are many categories..."
label: Requested Changes
description: Describe the changes to the spec or implementation (Claude will update the spec first if needed)
placeholder: |
- Add grid lines for better readability
- Change default color scheme to colorblind-friendly
- Include legend for all data series
validations:
required: true

- type: textarea
id: desired_behavior
attributes:
label: Desired Behavior
description: Describe what you want the plot to do instead
placeholder: "Labels should be rotated 45 degrees to prevent overlap, or use a horizontal layout..."
validations:
required: true
required: false

- type: textarea
id: additional_context
Expand Down
35 changes: 35 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,41 @@ Examples: `scatter-basic`, `scatter-color-mapped`, `bar-grouped-horizontal`, `he
- **`tests/unit/`**: Unit tests mirroring source structure
- **`docs/`**: Architecture and workflow documentation

## GitHub Issue Labels

### Workflow Status Labels

- **`plot-request`** - Main plot request issue
- **`plot-request:impl`** - Library implementation sub-issue (child of main)
- **`generating`** - Code is being generated
- **`testing`** - Tests are running
- **`reviewing`** - Quality review in progress
- **`merged`** - Successfully merged to main
- **`not-feasible`** - 3x failed, not implementable in this library
- **`completed`** - All library implementations complete
- **`update`** - Update request for existing spec
- **`test`** - Test issue, not a real plot request

### Updating Existing Plots

To update an existing plot:
1. Create issue with title: `[update] {spec-id}` (all libraries) or `[update:library] {spec-id}` (single library)
2. Add label: `plot-request`
3. Issue body can contain spec changes (Claude updates spec first)
4. Maintainer adds `approved` label
5. Workflow regenerates specified implementations

**Issue Lifecycle:**
```
[open] plot-request → approved → in-progress → completed [closed]
```

**Sub-Issue Lifecycle:**
```
[open] generating → testing → reviewing → merged [closed]
→ not-feasible [closed]
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

[nitpick] The Sub-Issue Lifecycle documentation shows the transition to not-feasible but doesn't show the intermediate retry states (with ai-rejected label). According to the workflow files in the codebase, the actual flow includes retry attempts with ai-rejected before reaching not-feasible after 3 failures.

Consider updating to show:

[open] generating → testing → reviewing → merged [closed]
                                        → ai-rejected (retry) → ...
                                        → not-feasible [closed] (after 3 attempts)
Suggested change
not-feasible [closed]
ai-rejected (retry) → ...
→ not-feasible [closed] (after 3 attempts)

Copilot uses AI. Check for mistakes.
```

## Code Standards

### Python Style
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/bot-sync-status.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ jobs:
ISSUE_NUM="${{ github.event.issue.number }}"
LABELS="${{ join(github.event.issue.labels.*.name, ',') }}"

# Check if this is a sub-issue
if echo "$LABELS" | grep -q "sub-issue"; then
# Check if this is a plot-request implementation issue
if echo "$LABELS" | grep -q "plot-request:impl"; then
# Find parent from issue body
BODY=$(gh issue view $ISSUE_NUM --json body -q '.body')
PARENT_NUM=$(echo "$BODY" | grep -oP '\*\*Parent Issue:\*\* #\K\d+' | head -1 || echo "")
Expand Down
80 changes: 69 additions & 11 deletions .github/workflows/gen-new-plot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ jobs:
outputs:
should_run: ${{ steps.check.outputs.should_run }}
spec_id: ${{ steps.extract_spec.outputs.spec_id }}
is_update: ${{ steps.extract_spec.outputs.is_update }}
target_library: ${{ steps.extract_spec.outputs.target_library }}

steps:
- name: Check conditions
Expand Down Expand Up @@ -63,6 +65,29 @@ jobs:
ISSUE_BODY: ${{ github.event.issue.body }}
ISSUE_TITLE: ${{ github.event.issue.title }}
run: |
# Check for [update] or [update:library] prefix in title
IS_UPDATE="false"
TARGET_LIBRARY=""

if echo "$ISSUE_TITLE" | grep -qiP '^\[update(:[a-z]+)?\]'; then
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The regex pattern '^\[update(:[a-z]+)?\]' should be more permissive to match any case in the library name since the code already normalizes the extracted library with tr '[:upper:]' '[:lower:]' on line 75.

Consider changing to: '^\[update(:[a-zA-Z]+)?\]' to accept mixed-case library names before normalization.

Suggested change
if echo "$ISSUE_TITLE" | grep -qiP '^\[update(:[a-z]+)?\]'; then
if echo "$ISSUE_TITLE" | grep -qiP '^\[update(:[a-zA-Z]+)?\]'; then

Copilot uses AI. Check for mistakes.
IS_UPDATE="true"
# Extract target library if specified (e.g., [update:seaborn])
TARGET_LIBRARY=$(echo "$ISSUE_TITLE" | grep -oiP '^\[update:\K[a-z]+(?=\])' | tr '[:upper:]' '[:lower:]' || echo "")
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The regex pattern allows only lowercase letters [a-z]+ but library names in the codebase use lowercase only. However, the issue template dropdown includes library names like "matplotlib", "seaborn", etc., which users could manually type with different cases. While there's a tr '[:upper:]' '[:lower:]' to handle this, the regex should be more permissive to match the input before normalization.

Consider changing the pattern to: '^\[update:\K[a-zA-Z]+(?=\])' to accept any case, since you're already normalizing with tr '[:upper:]' '[:lower:]'.

Suggested change
TARGET_LIBRARY=$(echo "$ISSUE_TITLE" | grep -oiP '^\[update:\K[a-z]+(?=\])' | tr '[:upper:]' '[:lower:]' || echo "")
TARGET_LIBRARY=$(echo "$ISSUE_TITLE" | grep -oiP '^\[update:\K[a-zA-Z]+(?=\])' | tr '[:upper:]' '[:lower:]' || echo "")

Copilot uses AI. Check for mistakes.
# Remove [update...] prefix from title for spec ID extraction
CLEAN_TITLE=$(echo "$ISSUE_TITLE" | sed 's/^\[update[^]]*\]\s*//')
echo "::notice::Update mode detected. Target library: ${TARGET_LIBRARY:-all}"
else
CLEAN_TITLE="$ISSUE_TITLE"
fi

Comment thread
MarkusNeusinger marked this conversation as resolved.
# Validate TARGET_LIBRARY if specified
if [ -n "$TARGET_LIBRARY" ]; then
VALID_LIBS="matplotlib seaborn plotly bokeh altair plotnine pygal highcharts"
if ! echo "$VALID_LIBS" | grep -qw "$TARGET_LIBRARY"; then
echo "::error::Invalid library '$TARGET_LIBRARY'. Must be one of: $VALID_LIBS"
exit 1
fi
fi
# Check if issue was marked as duplicate
COMMENTS=$(gh issue view ${{ github.event.issue.number }} --json comments -q '.comments[].body')

Expand All @@ -71,8 +96,15 @@ jobs:
exit 1
fi

# For updates, try to extract spec ID from cleaned title first
if [ "$IS_UPDATE" == "true" ]; then
SPEC_ID=$(echo "$CLEAN_TITLE" | grep -oiP '^[a-z]+-[a-z]+(-[a-z0-9]+)?' | tr '[:upper:]' '[:lower:]' || echo "")
fi

# Extract spec ID from issue comments (assigned by validate-plot-request workflow)
SPEC_ID=$(echo "$COMMENTS" | grep -oP '\*\*Assigned ID:\*\* `\K[a-z0-9-]+(?=`)' | tail -1 || echo "")
if [ -z "$SPEC_ID" ]; then
SPEC_ID=$(echo "$COMMENTS" | grep -oP '\*\*Assigned ID:\*\* `\K[a-z0-9-]+(?=`)' | tail -1 || echo "")
fi

if [ -z "$SPEC_ID" ]; then
SPEC_ID=$(echo "$COMMENTS" | grep -oP '\*\*Existing Spec:\*\* `\K[a-z0-9-]+(?=`)' | tail -1 || echo "")
Expand All @@ -92,7 +124,9 @@ jobs:
fi

echo "spec_id=$SPEC_ID" >> $GITHUB_OUTPUT
echo "::notice::Extracted spec ID: $SPEC_ID"
echo "is_update=$IS_UPDATE" >> $GITHUB_OUTPUT
echo "target_library=$TARGET_LIBRARY" >> $GITHUB_OUTPUT
echo "::notice::Extracted spec ID: $SPEC_ID (update=$IS_UPDATE, target=$TARGET_LIBRARY)"

# ============================================================================
# Step 2: Create sub-issues for each library
Expand Down Expand Up @@ -169,7 +203,7 @@ jobs:
SUB_ISSUE=$(gh issue create \
--title "[$SPEC_ID] $LIBRARY implementation" \
--body "$SUB_BODY" \
--label "library:$LIBRARY,sub-issue,generating")
--label "library:$LIBRARY,plot-request:impl,generating")

SUB_NUM=$(echo "$SUB_ISSUE" | grep -oP '\d+$')
SUB_NODE_ID=$(gh api repos/${{ github.repository }}/issues/$SUB_NUM --jq '.node_id')
Expand All @@ -191,7 +225,10 @@ jobs:
# ============================================================================
generate-matplotlib:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'matplotlib')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -204,7 +241,10 @@ jobs:

generate-seaborn:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'seaborn')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -217,7 +257,10 @@ jobs:

generate-plotly:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'plotly')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -230,7 +273,10 @@ jobs:

generate-bokeh:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'bokeh')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -243,7 +289,10 @@ jobs:

generate-altair:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'altair')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -256,7 +305,10 @@ jobs:

generate-plotnine:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'plotnine')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -269,7 +321,10 @@ jobs:

generate-pygal:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'pygal')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand All @@ -282,7 +337,10 @@ jobs:

generate-highcharts:
needs: [check-conditions, create-sub-issues]
if: needs.check-conditions.outputs.should_run == 'true'
if: |
needs.check-conditions.outputs.should_run == 'true' &&
(needs.check-conditions.outputs.target_library == '' ||
needs.check-conditions.outputs.target_library == 'highcharts')
uses: ./.github/workflows/gen-library-impl.yml
with:
spec_id: ${{ needs.check-conditions.outputs.spec_id }}
Expand Down
29 changes: 28 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,13 +339,15 @@ bash .github/scripts/setup-labels.sh

### Workflow Status Labels

- **`sub-issue`** - Library-specific sub-issue (child of main plot-request)
- **`plot-request:impl`** - Library implementation sub-issue (child of main plot-request)
- **`generating`** - Code is being generated
- **`testing`** - Tests are running
- **`reviewing`** - Quality review in progress
- **`merged`** - Successfully merged to main
- **`not-feasible`** - 3x failed, not implementable in this library
- **`completed`** - All library implementations complete
- **`update`** - Update request for existing spec (use with plot-request)
- **`test`** - Test issue, not a real plot request

### Approval Labels (Human vs AI distinction)

Expand Down Expand Up @@ -399,6 +401,31 @@ bash .github/scripts/setup-labels.sh
- Partial success (5/8 can merge while 3/8 retry)
- Per-library dependency isolation

### Updating Existing Plots

To update an existing plot implementation:

1. Create issue with title: `[update] {spec-id}` (all libraries) or `[update:{library}] {spec-id}` (single library)
- Example: `[update] scatter-basic` - regenerate all 8 libraries
- Example: `[update:seaborn] scatter-basic` - regenerate only seaborn
2. Add label: `plot-request`
3. Issue body can contain spec changes (Claude will update `specs/{spec-id}.md` first)
4. Maintainer adds `approved` label
5. Workflow regenerates specified implementations

**Issue Lifecycle:**
```
[open] plot-request → approved → in-progress → completed [closed]
```

**Sub-Issue Lifecycle:**
```
[open] generating → testing → reviewing → merged [closed]
→ not-feasible [closed]
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

[nitpick] The Sub-Issue Lifecycle documentation shows the transition to not-feasible but doesn't show the intermediate retry states (with ai-rejected label). According to the workflow files in the codebase, the actual flow includes retry attempts with ai-rejected before reaching not-feasible after 3 failures.

Consider updating to show:

[open] generating → testing → reviewing → merged [closed]
                                        → ai-rejected (retry) → ...
                                        → not-feasible [closed] (after 3 attempts)
Suggested change
not-feasible [closed]
ai-rejected (retry) → ...
→ not-feasible [closed] (after 3 attempts)

Copilot uses AI. Check for mistakes.
```

**Test Issues:** When creating issues for testing workflows, add the `test` label to exclude them from production searches.

## Environment Variables

Required in `.env`:
Expand Down
Loading
Loading