Skip to content

Commit faf526c

Browse files
hyunhee-joclaude
andcommitted
refactor: apply review feedback (round 1)
Objective: Code review identified 6 issues across CI workflow, shell scripts, SKILL.md references, and .gitignore. Approach: Accept 6 items, reject 4 items with technical reasoning. - CI: remove nonexistent --fix flag from error message, fix unreachable exit-code logic with set +e - Shell: add executable bit (100755) to detect-env.sh, hybrid-health.sh - SKILL.md: add eval-metrics.md to reference table, fix quick-eval.py usage from --input/--reference flags to positional args - .gitignore: remove duplicate __pycache__ entry (already covered by **/__pycache__/ on line 53) Evidence: git ls-tree confirms 100755 for .sh files. grep confirms no remaining --fix references in CI. SKILL.md quick-eval.py example matches argparse positional interface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 14cf497 commit faf526c

File tree

5 files changed

+6
-7
lines changed

5 files changed

+6
-7
lines changed

.github/workflows/skill-drift-check.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ jobs:
2525

2626
- name: Check skill drift
2727
run: |
28+
set +e
2829
python skills/odl-pdf/scripts/sync-skill-refs.py
29-
if [ $? -ne 0 ]; then
30+
EXIT_CODE=$?
31+
if [ $EXIT_CODE -ne 0 ]; then
3032
echo ""
3133
echo "Drift detected: skill references are out of sync with options.json."
32-
echo "Run 'python skills/odl-pdf/scripts/sync-skill-refs.py --fix' locally to update them."
34+
echo "Update skills/odl-pdf/references/options-matrix.md to match options.json."
3335
exit 1
3436
fi

.gitignore

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,3 @@ logs/
7575
# Configuration files
7676
.claude/settings.local.json
7777
.claude/plans/
78-
79-
skills/odl-pdf/scripts/__pycache__/

skills/odl-pdf/SKILL.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,7 @@ When extraction quality is inadequate. Start with measurement, then escalate.
363363
Run the quick evaluation script against your output:
364364

365365
```bash
366-
python skills/odl-pdf/scripts/quick-eval.py \
367-
--input output/document.json \
368-
--reference ground-truth.json
366+
python skills/odl-pdf/scripts/quick-eval.py output/document.md ground-truth.md
369367
```
370368

371369
Or run the full benchmark to get NID, TEDS, and MHS scores:
@@ -686,6 +684,7 @@ Load these files progressively — only when entering the relevant topic. Do not
686684
| `references/options-matrix.md` | User needs detailed option documentation, defaults, or interactions |
687685
| `references/hybrid-guide.md` | User needs hybrid server setup, server-side flags, or remote deployment |
688686
| `references/format-guide.md` | User needs output format comparison, format-specific behavior, or format selection |
687+
| `references/eval-metrics.md` | User needs detailed metric definitions (NID, TEDS, MHS), benchmark scores, or diagnostic steps by metric |
689688
| `scripts/detect-env.sh` | Phase 1 environment detection — run at session start |
690689
| `scripts/quick-eval.py` | Phase 4 quality measurement — run when diagnosing extraction quality |
691690
| `evals/` | Benchmark baselines and regression thresholds |

skills/odl-pdf/scripts/detect-env.sh

100644100755
File mode changed.

skills/odl-pdf/scripts/hybrid-health.sh

100644100755
File mode changed.

0 commit comments

Comments
 (0)