Skip to content

Commit 04a6939

Browse files
fix(workflows): address Copilot review feedback
Fixes 3 issues identified by Copilot code review: 1. **sync-labels.yml (spec detection)**: Use GitHub event context - Changed from `git diff HEAD~1 HEAD` to `${{ github.event.before/after }}` - Handles edge cases: first commit, force pushes, rebases - Added fallback for initial push (SHA = 0000...) 2. **sync-labels.yml (impl detection)**: Same fix applied - Uses GitHub event context for reliability - Prevents failures on edge cases 3. **impl-generate.yml (critical bug)**: Fixed file verification logic - Claude already commits implementation file earlier in workflow - Changed verification from "is file staged?" to "does file exist?" - Uses `git ls-files --error-unmatch` to verify implementation - Only stages and verifies metadata file (which we create) - Prevents false "not staged" errors that would block all workflows Updated CLAUDE.md to reflect the corrected workflow behavior.
1 parent b2dec43 commit 04a6939

3 files changed

Lines changed: 46 additions & 15 deletions

File tree

.github/workflows/impl-generate.yml

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -526,27 +526,35 @@ jobs:
526526
yaml.dump(data, f, default_flow_style=False, sort_keys=False, allow_unicode=True)
527527
"
528528
529-
# Commit and push (ATOMIC: both implementation AND metadata together)
529+
# Commit and push metadata (implementation already committed by Claude)
530530
git config user.name "github-actions[bot]"
531531
git config user.email "github-actions[bot]@users.noreply.github.com"
532532
533533
IMPL_FILE="plots/${SPEC_ID}/implementations/${LIBRARY}.py"
534534
535-
# Add both files in a single commit to prevent partial merges
536-
git add "$METADATA_FILE" "$IMPL_FILE"
535+
# Verify implementation file exists in the repository (Claude should have committed it)
536+
if ! git ls-files --error-unmatch "$IMPL_FILE" >/dev/null 2>&1; then
537+
echo "::error::Implementation file not found in repository - cannot commit"
538+
echo "::error::Expected implementation at: $IMPL_FILE"
539+
echo "::error::This indicates Claude failed to create the implementation file"
540+
exit 1
541+
fi
542+
543+
# Add metadata file
544+
git add "$METADATA_FILE"
537545
538-
# Verify both files are staged
539-
if ! git diff --cached --name-only | grep -q "${LIBRARY}.py"; then
540-
echo "::error::Implementation file not staged - cannot commit"
541-
echo "::error::This indicates the implementation file is missing or not modified"
546+
# Verify metadata file is staged
547+
if ! git diff --cached --name-only | grep -q "$(basename "$METADATA_FILE")"; then
548+
echo "::error::Metadata file not staged - cannot commit"
549+
echo "::error::This indicates the metadata file was not added correctly"
542550
exit 1
543551
fi
544552
545-
git commit -m "feat(${LIBRARY}): implement ${SPEC_ID} with metadata"
553+
git commit -m "chore(${LIBRARY}): add metadata for ${SPEC_ID}"
546554
git push origin "$BRANCH"
547555
548556
echo "::notice::Created metadata file: $METADATA_FILE (py${PYTHON_VERSION}, ${LIBRARY}==${LIBRARY_VERSION})"
549-
echo "::notice::Committed implementation and metadata atomically"
557+
echo "::notice::Implementation verified present, metadata committed"
550558
551559
# ========================================================================
552560
# Process images: optimize + thumbnail

.github/workflows/sync-labels.yml

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,23 @@ jobs:
2929
3030
- name: Detect new specifications merged to main
3131
id: detect
32+
env:
33+
BEFORE: ${{ github.event.before }}
34+
AFTER: ${{ github.event.after }}
3235
run: |
3336
# Get list of specifications added/modified in this push
34-
CHANGED_SPECS=$(git diff --name-only HEAD~1 HEAD | \
35-
grep -oP 'plots/\K[^/]+(?=/specification\.(md|yaml))' | \
36-
sort -u)
37+
# Use GitHub event context for reliability (handles first push, force push, etc.)
38+
if [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then
39+
# First push or new branch: diff-tree on the after commit only
40+
CHANGED_SPECS=$(git diff-tree --no-commit-id --name-only -r "$AFTER" | \
41+
grep -oP 'plots/\K[^/]+(?=/specification\.(md|yaml))' | \
42+
sort -u || true)
43+
else
44+
# Normal push: diff between before and after SHAs
45+
CHANGED_SPECS=$(git diff --name-only "$BEFORE" "$AFTER" | \
46+
grep -oP 'plots/\K[^/]+(?=/specification\.(md|yaml))' | \
47+
sort -u || true)
48+
fi
3749
3850
if [ -z "$CHANGED_SPECS" ]; then
3951
echo "::notice::No specification changes detected"
@@ -102,10 +114,21 @@ jobs:
102114
103115
- name: Detect new implementations merged to main
104116
id: detect
117+
env:
118+
BEFORE: ${{ github.event.before }}
119+
AFTER: ${{ github.event.after }}
105120
run: |
106121
# Get list of implementations added/modified in this push
107-
CHANGED_IMPLS=$(git diff --name-only HEAD~1 HEAD | \
108-
grep -P 'plots/[^/]+/implementations/[^/]+\.py$')
122+
# Use GitHub event context for reliability (handles first push, force push, etc.)
123+
if [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then
124+
# First push or new branch: diff-tree on the after commit only
125+
CHANGED_IMPLS=$(git diff-tree --no-commit-id --name-only -r "$AFTER" | \
126+
grep -P 'plots/[^/]+/implementations/[^/]+\.py$' || true)
127+
else
128+
# Normal push: diff between before and after SHAs
129+
CHANGED_IMPLS=$(git diff --name-only "$BEFORE" "$AFTER" | \
130+
grep -P 'plots/[^/]+/implementations/[^/]+\.py$' || true)
131+
fi
109132
110133
if [ -z "$CHANGED_IMPLS" ]; then
111134
echo "::notice::No implementation changes detected"

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ impl-generate.yml
659659
├── Creates branch: implementation/{specification-id}/{library}
660660
├── Generates implementation code
661661
├── Tests with MPLBACKEND=Agg
662-
├── Commits implementation + metadata atomically (prevents partial PRs)
662+
├── Verifies implementation exists before committing metadata
663663
├── Uploads preview to GCS staging
664664
├── Creates PR: implementation/{specification-id}/{library} → main
665665
└── Triggers impl-review.yml

0 commit comments

Comments
 (0)