From 95b2d48541141ab8a3778c8dae099fc29fb87ff5 Mon Sep 17 00:00:00 2001 From: yyuan308 Date: Wed, 2 Sep 2026 00:55:29 +0800 Subject: [PATCH] feat: add generic private grading delivery workflow --- .agents/skills/grade-homework/SKILL.md | 468 +++++------------- .../references/course-package-template.json | 30 ++ .../references/grading-prompt.md | 253 ++++------ .../scripts/annotate_submission.py | 221 +++++++++ .../skills/grade-homework/scripts/discover.py | 163 ++++-- .../scripts/render_submission.py | 119 +++++ .../skills/grade-homework/scripts/roster.py | 143 ++++++ .../grade-homework/scripts/to_images.py | 95 ++-- .../grade-homework/scripts/write_outputs.py | 426 +++++++++++++--- .claude/skills/grade-homework/SKILL.md | 468 +++++------------- .../references/course-package-template.json | 30 ++ .../references/grading-prompt.md | 253 ++++------ .../scripts/annotate_submission.py | 221 +++++++++ .../skills/grade-homework/scripts/discover.py | 163 ++++-- .../scripts/render_submission.py | 119 +++++ .../skills/grade-homework/scripts/roster.py | 143 ++++++ .../grade-homework/scripts/to_images.py | 95 ++-- .../grade-homework/scripts/write_outputs.py | 426 +++++++++++++--- .gitignore | 6 + benchmark/core/skill_snapshots.py | 9 +- .../generic-grading-delivery-v5-4/plan.json | 32 ++ .../grading-skill-error-book-registry.json | 35 +- ...skill_candidate_v5_4_generic_delivery.json | 15 + .../core/test_candidate_v3_assets.py | 35 +- .../core/test_deduction_trace_contract.py | 9 +- .../test_grade_homework_candidate_skill.py | 227 +++++++-- .../test_grade_homework_skill_contract.py | 82 ++- tests/benchmark/core/test_skill_snapshots.py | 25 +- tests/benchmark/physics/test_skill_sync.py | 27 +- 29 files changed, 2947 insertions(+), 1391 deletions(-) create mode 100644 .agents/skills/grade-homework/references/course-package-template.json create mode 100644 .agents/skills/grade-homework/scripts/annotate_submission.py create mode 100644 .agents/skills/grade-homework/scripts/render_submission.py create mode 100644 .agents/skills/grade-homework/scripts/roster.py create mode 100644 .claude/skills/grade-homework/references/course-package-template.json create mode 100644 .claude/skills/grade-homework/scripts/annotate_submission.py create mode 100644 .claude/skills/grade-homework/scripts/render_submission.py create mode 100644 .claude/skills/grade-homework/scripts/roster.py create mode 100644 experiments/records/generic-grading-delivery-v5-4/plan.json create mode 100644 experiments/skill_versions/skill_candidate_v5_4_generic_delivery.json diff --git a/.agents/skills/grade-homework/SKILL.md b/.agents/skills/grade-homework/SKILL.md index 64248fb..663501e 100644 --- a/.agents/skills/grade-homework/SKILL.md +++ b/.agents/skills/grade-homework/SKILL.md @@ -1,68 +1,49 @@ --- name: grade-homework -description: Use when the user wants to grade a folder of student homework, quiz, or exam submissions against a teacher-provided solutions or rubric document. Handles mixed PDF, image, and DOCX inputs, produces a grades CSV and per-student English feedback, and explicitly flags ambiguous, unreadable, missing, or high-impact grading items for teacher review. Triggers on phrases like "grade the homework", "mark HW9", "batch grade submissions", or "批作业". +description: Use when a teacher or TA needs to grade a batch of scanned homework, quiz, or exam submissions against a course-provided rubric. It groups multi-page scans, keeps the roster private, produces grades and review CSVs, and renders checked annotations on each submission. --- # grade-homework -## When to use +## Scope and privacy boundary -- User wants to grade a folder of student submissions. -- A solutions document is present (either auto-discoverable by filename or provided explicitly). -- Student filenames follow `_..._.`. +This is a cross-course delivery skill. It provides evidence-based grading, +private roster handling, scan rendering, structured outputs, annotations, and +human-review handoff. It does not supply subject knowledge, point allocations, +partial-credit bands, canonical forms, required work, or penalties. Those rules +belong only in the frozen course package for the current assessment. -Do NOT use for: single-file grading (just read it inline), plagiarism detection, or rewriting the solutions doc. +Never import a rule, example, point value, or error pattern from a previous +course or data set. If the current course package does not settle a scoring +question, stop and ask the course owner instead of guessing. ## What this skill produces -- `/grades/grades.csv` — one row per student, per-question columns, total, flags. -- `/grades/feedback/.md` — English feedback, per-question breakdown, flags summary. +- `grades/grades.csv` - opaque ID, local name, local student number, item + scores, total, uncertainties, and flags. +- `grades/review.csv` - one row for every flagged or non-high-confidence leaf. +- `grades/feedback/.md` - concise feedback. +- `grades/annotations/.json` - validated annotation data. +- `grades/marked//` - annotated PNG pages plus a marked PDF. -These are private grading records: choose a directory outside any Git worktree, -or an already ignored private directory. They contain per-person grades, -feedback, and answer-derived evidence; never commit, publish, or copy them into -an experiment's public record. The output helper refuses an unignored directory -inside a Git worktree. +These are private per-person records. They never belong in Git, a public report, +a pull request, or a model prompt. The output helpers reject an unignored +directory inside a Git worktree. -## Prerequisites (conditional on submission formats) +## Private input contract -Only `.docx` submissions need an external conversion toolchain. PDF and image -submissions go through Python alone, so most courses won't need anything -beyond `uv`. +Use a private, ignored working directory with `course-package.json`, +`roster.csv`, and `submissions//` directories. `roster.csv` +must have exactly `submission_id,student_name,student_number`; it is local-only +and must never be included in a model-facing record. Start the course package +from `references/course-package-template.json`, replace every placeholder, and +freeze it before grading begins. -Probe at the start of Step 1 (after `discover.py` reports what's present): +PDFs and ordinary image formats render in Python. DOCX files require +LibreOffice or `soffice`. -```bash -# Only if discover.py finds any .docx submissions -if ls "$PWD"/*.docx >/dev/null 2>&1; then - if ! command -v libreoffice >/dev/null 2>&1 && ! command -v soffice >/dev/null 2>&1; then - if ! { command -v pandoc >/dev/null 2>&1 \ - && { command -v google-chrome >/dev/null 2>&1 \ - || command -v chromium >/dev/null 2>&1; }; }; then - echo "MISSING: docx toolchain (need libreoffice OR pandoc+browser)" - fi - fi -fi -``` - -If the toolchain is missing, ask via `AskUserQuestion` whether to install -before grading begins. Without it, `.docx` submissions are silently flagged -`needs_manual_review` and grading proceeds for the rest — call out exactly -how many students that affects so the user can make an informed choice. - -When the user says yes, figure out the right install command for their -environment at runtime (inspect `uname -s` and which of `brew`/`apt`/`dnf`/ -`pacman`/`winget`/etc. is available — see `bootstrap` Step 0 for the -detection pattern), and propose the commands via `AskUserQuestion` before -running them. Notes on package naming: - -- `libreoffice` may be `libreoffice-fresh` on Arch and `--cask libreoffice` - on Homebrew; on Windows use `TheDocumentFoundation.LibreOffice` via winget. -- Headless browser for the fallback path: `google-chrome` or `chromium` — - either works. -- `inkscape` is only needed by the fallback path when a `.docx` embeds WMF/EMF - images. Install on demand if `to_images.py` flags missing inkscape during a - run; otherwise leave it alone. +If DOCX conversion is unavailable, do not claim that the pages were rendered. +Put that submission in review or install the approved converter before grading. ## Workflow @@ -70,107 +51,27 @@ Skill root: the directory containing this `SKILL.md`. Resolve scripts and references relative to that directory; do not assume a Claude- or Codex-specific home path. -### Candidate grading contract - -Grade from visible evidence, not from assumed intent. For every scored question, -first record the visible equation, statement, diagram feature, answer text, or -blank-answer marker. Assign points only after that evidence is written down. - -For every question, identify the question type and extract -`key_term_evidence`, `concept_evidence`, and `relation_evidence`. Map each -non-overlapping rubric scoring element to exactly one state: `absent`, -`mentioned_only`, `partial_understanding`, `demonstrated`, or -`misused_or_contradicted`. A correctly used relevant keyword may receive the -rubric's limited `mentioned_only` credit; a misused keyword receives no -automatic credit. Treat an unambiguous semantic equivalent as evidence for the -matching element without requiring standard wording, notation, or ordering. - -Award the integer credit for that one state only. Do not award duplicate credit: -a keyword and its explanation belong to one element, and overlapping evidence -cannot receive points twice. Sum non-overlapping element credit into a subtotal. -Use the highest satisfied score band and any material-error cap only as upper -bounds; they cannot raise the subtotal. Full credit requires every required -essential element to be demonstrated or expressed by a semantic equivalent, -required terminology when explicitly requested, and no material contradiction. - -When the final answer is wrong, retain justified process credit for correct -terms, concepts, formulas, substitutions, units, and reasoning unless the -frozen rubric makes the conclusion indispensable. For calculation problems, -arithmetic mistakes should not erase a correct method unless the frozen rubric -requires the exact result. When the final answer is correct and the process is -roughly correct, award full credit when the frozen requirements are met. - -Candidate v3.1 adds four calibration rules for concept, proof, and construction -answers: - -- cap-locality: apply a material-error cap only when the cap condition is directly visible and active; - do not trigger a cap merely because an element is - partial, under-detailed, or expressed through a non-standard but viable route. -- contradiction-locality: when a misconception or contradiction is local to one - element, proof direction, or construction step, preserve unrelated element credit - unless the frozen rubric explicitly defines a question-level cap. -- key-term semantics: key terms are evidence signals, not mandatory wording - unless the rubric or full-credit rule explicitly requires that terminology. - Correctly used key terms can earn limited keyword credit, and semantic - equivalents should still be mapped to the matching rubric element. -- indirect-construction: score valid indirect constructions by mapping visible - steps to rubric elements and required output behavior. Do not require the - standard direct construction when an indirect route demonstrates the same - result. - -Candidate v3.1 r2 adds open-ended adequacy: for open-ended short-answer, proof, -construction, and essay questions, score whether the answer satisfies the task requirement. -Use the standard answer as an anchor, not as an exhaustive whitelist. Award -credit for valid, relevant, non-contradictory approaches, examples, or -constructions that answer the prompt, even when they are not listed in the expected answer or semantic equivalents. - -Candidate v3.2 adds official-style adequacy: grade for official-style adequacy, -not ideal-answer completeness, and avoid being overly harsh. Preserve reasonable -partial credit for demonstrated understanding even when terminology, ordering, -or detail is imperfect. Distinguish missing ideal detail from a visible misconception. -Apply large deductions only for material errors, contradictions, -wrong language/output behavior, or missing required answer behavior. - -Candidate v3.2 is a cross-course contract. Course-specific calibration overlays -belong in that course's frozen rubric and packet, never in this reusable skill. -Do not carry rules for named questions, named languages, named theorems, or a -previous course into another course merely because their labels look similar. - -Classify every question from the prompt and frozen rubric *before* scoring. Use -the most specific applicable type, and record it in the grading record: - -- `objective_selection` (including multiple choice, matching, and true/false): - require a selected option or an unambiguous equivalent. Do not require an - explanation for true/false or other selected-response items unless the prompt - explicitly asks to prove, explain, justify, or show work. -- `calculation`: check the final numeric or symbolic result, method/setup, - transformations or substitutions, intermediate calculation, and any - mathematical or domain reasoning required by the rubric. Retain justified - method credit if the final result is wrong. If the result is correct but - required working is absent, award only the course-frozen answer-only credit; - do not invent a universal amount. -- `calculation_short_answer`: score both the visible derivation and the short - conclusion/classification requested by the question. A valid alternative - derivation is acceptable; the reference solution is an anchor, not a required - route. -- `short_answer` or `conceptual`: combine key-term, concept, and relation - evidence; exact standard-answer wording is not required. -- `algorithm` or `construction`: require a viable method plus relevant steps, - relations, and required output behavior; award credit to valid alternatives. -- `proof` or `explanation`: check each required logical direction/link and - preserve credit for independently completed parts. A missing required part - blocks full credit but does not erase unrelated demonstrated work. -- `diagram`, `geometry`, or `representation`: score the observable required - objects, relations, labels, transformations, and conclusion. Do not assume a - missing diagram feature from accompanying prose. -- `essay` or `open_response`: score distinct valid, relevant, non-contradictory - claims against the task requirement; do not require fixed ordering or - standard phrasing. - -When a question genuinely combines types, use non-overlapping rubric elements -for each required aspect rather than forcing it into a narrower legacy label. -The type controls what evidence is relevant; the frozen rubric controls points, -score increments, and any answer-only allocation. +### Generic grading contract + +Score visible evidence from the complete submission, not assumed intent. Record +a concise evidence note before assigning a score. The frozen course package +controls the question type, score leaves, required evidence, alternatives, +increments, and every partial-credit decision. + +Do not invent subparts, transfer points between leaves, or count one visible +fact twice. Accept an unambiguous valid alternative when it satisfies a +course-package criterion. Do not penalize extra work that is irrelevant to all +declared criteria unless the submission adopts it for the graded conclusion or +the course package explicitly says otherwise. + +The course package owns all scoring detail. It may define different policies for +different question types, methods, representations, answer-only work, and +partial credit. This live skill deliberately does not turn a previous course's +calibration into a universal rule. + +The course package may declare question types when that helps its own rubric. +The type is only a routing aid: the package's explicit criteria, score leaves, +and policies always control the grade. ### Submission-level assembly @@ -222,221 +123,122 @@ frozen rubric; their `points_deducted` values must sum exactly to `max_score - score` for that leaf. This is a short audit record, not hidden reasoning or a chain of thought. -Deduct from the first material error and do not split its dependent downstream -consequences into extra deductions. For selected-response questions, assess an -explanation only when the prompt explicitly requires one. Apply the frozen -answer-only cap when a correct calculation answer lacks required work. A zero -score must state the specific missing or incorrect required evidence. Full-credit -leaves omit `deduction_trace`; a leaf with flags or `low` confidence must add a -brief `attention_note`. Treat bonus leaves as independent leaves: never use a -base-leaf deduction to explain, offset, or replace a bonus decision. +Apply the course package's deduction order and no-double-count policy. A zero +score must state specific visible missing or incorrect evidence. Full-credit +leaves omit `deduction_trace`; a flagged, medium-confidence, or low-confidence +leaf must add a brief `attention_note` and appear in review output. Treat any +course-declared bonus leaf independently from base-leaf scoring. Do not put a name, student identifier, email address, private path, or raw private-file reference in a deduction trace or attention note. -### Calculation calibration guardrails - -For each calculation leaf, classify the first score-affecting issue before -withholding points: absent required work, local notation or arithmetic error, -incorrect formula or method, failed required simplification, or incorrect final -result independent of earlier work. Apply only the frozen rubric criterion or -criteria for that first issue; do not convert its downstream result into a -second deduction. - -Check algebraic equivalence before withholding a symbolic-result or -simplification criterion. Reordered, factored, expanded, or otherwise -unambiguous equivalent expressions satisfy an `equivalent_form_accepted` -requirement. A required canonical simplification can be withheld only when the -course rubric explicitly declares it required. - -When a final-result criterion is separately declared, withhold it when the -reported final result is wrong, even when an earlier local error caused that -result. The no-double-count rule protects only dependent process criteria; it -does not preserve an independently allocated final-result criterion. A course -rubric may define a narrow exception only by explicitly declaring a conditional -carry-forward rule. Treat a wrong formula, wrong governing relation, or invalid -method as a method criterion, not as a local arithmetic error. Ignore extra -work that is irrelevant to every declared criterion; evaluate extra work only -when it directly contradicts a declared criterion. - -Freeze the grading protocol before student grading starts: - -- page ordering for solutions and each student submission -- rubric question IDs, maximum points, and allowed score increments -- partial-credit rules; do not introduce quarter-point or 0.25-point scores -- treatment of missing pages, blank answers, unreadable work, and alternative correct methods - -Treat transcript or OCR text as an optional aid. The Physics Week 9 pilot does -not prove that transcript workflows are generally better than direct-image -grading, so never claim that a transcript route is automatically more accurate. - -### Benchmark-informed safeguards - -The Physics Week 9 internal benchmark does not prove that transcript workflows -are generally better than the direct-image baseline. Treat transcript or OCR -steps as optional evidence aids, not as an automatic accuracy improvement. - -Before grading, freeze the page ordering, rubric, question IDs, point ranges, -and allowed score increments. During grading, use an evidence-first pass: record the -visible equation, statement, text, or blank-answer marker before assigning -points. Run a second-pass review for low confidence, unreadable regions, blank -or apparently missing answers, total mismatches, and high-impact deductions. -At handoff, report flagged items and which questions they concentrate on; ask -the teacher to spot-check at least 3 students and all flagged items before -publishing grades. - -### Route comparison and calibration - -If a course authorizes a route comparison, evaluate direct multimodal grading -and transcription-assisted grading as separate conditions with the same frozen -rubric, gold, split, prompt packet, and review policy. Transcription is an -evidence aid, not ground truth; never let it silently replace the page image. - -For each representative disagreement, create an evidence card before changing -the prompt, rubric, or skill. Classify it as exactly one primary cause: - -- `clear_model_error`: visible source evidence and frozen rubric support a - different score. -- `representation_loss`: a route lost, mistranscribed, reordered, cropped, or - failed to expose relevant source evidence. -- `rubric_or_gold_conflict`: the frozen scoring rule or reference answer needs - course-owner adjudication. -- `reasonable_severity_difference`: both readings are evidence-supported but - differ within an acceptable strictness range. -- `insufficient_evidence`: source quality or record does not support a reliable - conclusion. - -Do not assume a disagreement is a model error. Preserve the card, route -artifacts, and human decision so future prompt changes are auditable. - -### Step 1 — Discover - -Run `discover.py` on the working directory containing submissions. Parse the JSON: +### Course-package boundary -```bash -python /scripts/discover.py "$PWD" -``` +The current course package decides how calculations, selected responses, +proofs, diagrams, simplification, alternatives, bonus work, partial credit, +and dependent consequences are handled. This skill only enforces that the +chosen score is evidence-based, traceable, and arithmetically valid. OCR or +transcription may assist reading but never replaces the source pages. -If `solutions_error` is non-null, surface it to the user and stop. If the user passed a solutions path explicitly, use that instead of auto-discovery. +### Release safeguards -`discover.py` recurses into subdirectories (so `./submissions/` is picked up automatically) and skips hidden files/dirs. The JSON also includes a `late_students` list — student names whose filename contains `_LATE_` (case-insensitive). Surface the list to the user before grading so they can decide whether to apply a late-submission policy. +Before release, review every row in `review.csv`, inspect every marked page, +and spot-check a representative set of unflagged submissions. A teacher owns +the final score and any course-package correction. -### Step 2 — Load the grading prompt and parse the rubric +### Step 1 - Validate the private batch -Read `/references/grading-prompt.md` and follow it. - -Convert the solutions file to page images: +Validate the roster, then discover the batch: ```bash -python /scripts/to_images.py /tmp/grade-homework/solutions/ +python /scripts/roster.py roster.csv +python /scripts/discover.py . --roster roster.csv ``` -View the solutions images, verify deterministic **page ordering**, parse the -`[N pts]` allocations into a rubric table, and **confirm with the user before -continuing**. Freeze the page list, rubric, question IDs, point ranges, and -allowed score increments before grading. If no `[N pts]` markers are found, stop and -ask for point allocations. - -Partial-credit conventions: -1. Do not use 0.25-point or quarter-point scores. -2. Award full credit when the student's final answer is correct and the process - is roughly correct, including mathematically equivalent alternative methods. -3. Deduct process points only when the final answer is correct but the process - seriously conflicts with the standard solution, required method, or visible - reasoning expectations. -4. When the final answer is wrong, inspect the student's process carefully and - award the appropriate process credit from the frozen rubric. -5. Preserve the frozen point increment; if the rubric is unclear, ask the - teacher before introducing a new increment. -6. If handwriting, page order, or missing work affects the score, add an - explicit flag instead of hiding the uncertainty in the numeric score. +For a new production batch, use `submissions//` directories. +If discovery reports a grouping, roster, missing-scan, or solution ambiguity, +stop and resolve the source data with the TA. Do not silently guess a student +identity or page grouping. The legacy filename-prefix mode remains only for +older workflows without a roster. +### Step 2 - Freeze the course package -### Step 3 — Grade students one at a time +Read `/references/grading-prompt.md`, inspect the assessment, and +confirm the current `course-package.json` with the course owner. It must cover +the leaf hierarchy, point values, increments, visible criteria, alternatives, +partial-credit policy, missing/illegible-work policy, and annotation guidance. +If any needed scoring rule is absent, stop and ask; do not infer it from a +solution format or a previous course. -For each student (in alphabetical order unless the user specifies otherwise): -1. Convert each of that student's files to images: +### Step 3 - Render, grade, write, and mark - ```bash - python /scripts/to_images.py "" /tmp/grade-homework// - ``` +For each submission group, render all scans before scoring: - If any file returns exit code 3 (`docx_unsupported`), include a `needs_manual_review` flag for that student and skip that file — do not block the whole run. - -2. Verify page ordering and question-to-page coverage before reading answers. - Missing, duplicated, rotated, or unreadable pages require an explicit flag. - -3. Use an evidence-first pass. For every question, record the visible equation, - statement, diagram feature, answer text, or blank-answer marker before - assigning points. **Do not guess** missing work or silently repair a - student's reasoning. - -4. Score only against the frozen rubric. Validate each score against its range - and allowed increment, then recompute section and assignment totals. - Attach `high`, `medium`, or `low` confidence plus explicit ambiguity flags. - -5. Run a **second-pass** review for every low-confidence item, unreadable region, - blank or apparently missing answer, total mismatch, and high-impact - deduction. Also check missed semantic equivalents, missed keyword credit, - duplicate credit, keyword misuse, score-band consistency, material-error - caps, local contradictions, indirect constructions, open-ended adequacy, - official-style adequacy, and arithmetic. - The second pass must revisit the source image and evidence, not merely repeat - the first score. +```bash +python /scripts/render_submission.py \ + submissions/ rendered/ \ + --submission-id +``` -6. Produce the JSON record only after those checks pass. - `extracted_evidence` and `evidence` must be plain text strings. Do not output - arrays or objects for these fields. If you use `key_term_evidence`, - `concept_evidence`, or `relation_evidence` internally, summarize those layers - inside the single `extracted_evidence` string or the single `evidence` - string. Every non-full leaf also needs the four-field `deduction_trace` - contract above; full-credit leaves may omit it. +Read the whole rendered page set together. Score only the frozen course leaves, +record visible evidence, attach confidence and flags, and return the JSON +contract in `grading-prompt.md`. Every non-full leaf needs the four-field +deduction trace; every review-needed leaf needs an attention note. -7. Pipe the record into `write_outputs.py`: +Write one private record at a time: - ```bash - echo "$RECORD_JSON" | python /scripts/write_outputs.py "$PRIVATE_GRADES_DIR" - ``` +```bash +echo "$RECORD_JSON" | python /scripts/write_outputs.py grades \ + --roster roster.csv \ + --course-package course-package.json \ + --require-annotations +``` - Set `PRIVATE_GRADES_DIR` to a location outside the repository or to an - already ignored private directory. Never use an unignored project folder for - this command. +Then render the marked pages: -8. After every 3 students, briefly summarize progress to the user so they can course-correct early. +```bash +python /scripts/annotate_submission.py \ + rendered//pages.json \ + grades/annotations/.json \ + grades/marked/ +``` -### Step 4 — Recovery +The writer validates the scores and creates `grades.csv` plus `review.csv`. +The annotation renderer fails closed for an invalid page, box, or label; never +fabricate a marking location. -If `grades/grades.csv` already exists at the start of a run, `write_outputs.py` -will skip any student already present. Preserve immutable benchmark records: -never overwrite a benchmark run, prompt, rubric, or prediction file. For an -ordinary re-grade, archive the prior row and feedback before creating a clearly -identified replacement; do not silently delete grading history. +With `--require-annotations`, every score-bearing leaf needs a `praise` box, +every non-full leaf needs a `deduction` box, and every review-needed leaf needs +a `review` box. A partially correct leaf can therefore need both praise and +deduction boxes. If a real location cannot be established, do not invent one: +flag it for teacher review before release. -### Step 5 — Handoff +### Step 4 - Recovery and release -When all students are graded, list: +The writer skips a `student_id` already present in the same grades CSV and +rejects a header change, preventing a silent mid-batch rubric mix. For a +re-grade, create a new private run directory; do not overwrite an existing +marked submission or silently delete prior grading history. -- Any students skipped (with reason). -- The total number of flagged items and which questions they concentrate on — this is what the teacher should spot-check before publishing grades. +Before release, reconcile all `review.csv` rows, inspect marked pages, and +provide the teacher with the private CSV and marked files. Report skipped scan +groups and their reasons. The teacher reviews flagged work and makes the final +score decision. ## Failure modes -- **No solutions file / multiple candidates** → stop, ask user. -- **No `[N pts]` markers** → stop, ask user for allocations. -- **DOCX submission with no conversion toolchain** → `to_images.py` tries `libreoffice`/`soffice` first, then `pandoc + google-chrome/chromium` (extracts WMF/EMF → PNG via `inkscape` if present, converts HTML→PDF via headless Chrome). If neither path works, the student is flagged `needs_manual_review` and grading continues. -- **CSV header mismatch mid-run** (`write_outputs.py` exit 4) → rubric changed; stop and reconcile with user. +- **Missing or ambiguous course material**: stop until the course owner supplies + a complete package and source solution/rubric. +- **Unknown roster group, missing scan, or duplicate grouping**: stop that batch + and resolve the TA's source data; do not infer an identity. +- **DOCX converter unavailable**: put the affected submission in review; do not + claim pages were rendered. +- **Invalid score, trace, or annotation**: correct the private record against the + frozen course package before release. ## Quality bar -This skill uses evidence-first grading plus targeted second-pass review. It is -**not** a substitute for teacher review: spot-check at least 3 students against -your own grading before publishing, and always review flagged items. - -The Physics Week 9 internal benchmark used one run per condition and a single -primary-rater reference. Its transcript-based GPT condition did not outperform -the historical direct baseline overall, so do not claim a general accuracy -improvement. Treat page ordering, frozen rubrics, evidence, confidence, and -second-pass review as auditability safeguards, with extra attention to the -lowest-agreement Physics Week 9 questions; do not generalize those error -patterns beyond this benchmark. +This skill supports a teacher with traceable, private records and visual marking. +It is not a teacher replacement. Course-specific quality claims require that +course's own approved rubric, human review, and validation process. diff --git a/.agents/skills/grade-homework/references/course-package-template.json b/.agents/skills/grade-homework/references/course-package-template.json new file mode 100644 index 0000000..d3ee0ef --- /dev/null +++ b/.agents/skills/grade-homework/references/course-package-template.json @@ -0,0 +1,30 @@ +{ + "schema_version": 1, + "course_id": "replace-with-course-id", + "assessment_id": "replace-with-assessment-id", + "score_leaves": [ + { + "question_id": "replace-with-leaf-id", + "max_score": 1, + "allowed_increment": 1, + "question_type": "replace-with-local-type-or-omit", + "criteria": [ + { + "criterion": "Replace with a visible, checkable requirement.", + "points": 1, + "evidence_required": "Describe observable work or answer evidence." + } + ], + "full_credit_rule": "State the local full-credit condition.", + "accepted_alternatives": [ + "List valid alternate methods or forms, if any." + ], + "partial_credit_rules": [ + "State local score bands and deduction order, including dependent consequences." + ], + "missing_or_unreadable_policy": "State the local review or score policy.", + "annotation_guidance": "State what a deduction, praise, or review box should locate.", + "bonus": false + } + ] +} diff --git a/.agents/skills/grade-homework/references/grading-prompt.md b/.agents/skills/grade-homework/references/grading-prompt.md index 2e7b9f8..aa57725 100644 --- a/.agents/skills/grade-homework/references/grading-prompt.md +++ b/.agents/skills/grade-homework/references/grading-prompt.md @@ -3,114 +3,36 @@ Use this reference after the solutions/rubric pages are available and before grading any student. -## Rubric freeze - -Create a rubric table with one row per question: - -- `question_id` -- `max_score` -- `allowed_increment` -- `expected_evidence` -- `partial_credit_notes` - -Confirm this table with the teacher before grading. Do not change question IDs, -max scores, or increments in the middle of a run. If the rubric is incomplete, -stop and ask. - -## Candidate evidence-first scoring - -For each student and question, write the evidence before the score: - -- visible equation, statement, diagram feature, answer text, or blank marker -- page number or file reference when available -- any uncertainty about handwriting, cropped pages, missing work, or page order - -Score only against the frozen rubric. Do not infer invisible work. Do not give -0.25-point or quarter-point scores. If the final answer is correct and the -process is roughly correct, award full credit. Deduct process points only when a -correct final answer is supported by a process that seriously conflicts with -the standard solution, required method, or visible reasoning expectations. When -the final answer is wrong, inspect the work carefully and award process credit -for correct terms, concepts, formulas, substitutions, units, and reasoning from -the frozen rubric. For calculation problems, arithmetic mistakes should not -erase a correct method unless the frozen rubric requires the exact result. - -Identify the question type before scoring. For each scoring element, record -`key_term_evidence`, `concept_evidence`, and `relation_evidence`, then use -exactly one state: `absent`, `mentioned_only`, `partial_understanding`, -`demonstrated`, or `misused_or_contradicted`. A correctly used keyword can earn -only the rubric's limited `mentioned_only` credit. An unambiguous semantic -equivalent can demonstrate the matching meaning without standard phrasing. -Do not award duplicate credit: a keyword and its explanation are one element, -and overlapping evidence cannot be credited twice. - -Sum the integer scores for non-overlapping elements. Score bands and a -material-error cap are upper bounds only and cannot raise the subtotal. Award -full credit only when all required essential elements are demonstrated, required -terminology is present when explicitly requested, and no material contradiction -invalidates the answer. - -Apply these Candidate v3.1 calibration rules before finalizing the score: - -- cap-locality: apply a material-error cap only when the cap condition is directly visible and active; - do not trigger a cap merely because an element is - partial, under-detailed, or expressed through a non-standard but viable route. -- contradiction-locality: when a misconception or contradiction is local to one - element, proof direction, or construction step, preserve unrelated element credit - unless the frozen rubric explicitly defines a question-level cap. -- key-term semantics: key terms are evidence signals, not mandatory wording - unless the rubric or full-credit rule explicitly requires that terminology. - Correctly used key terms can earn limited keyword credit, and semantic - equivalents should still be mapped to the matching rubric element. -- indirect-construction: score valid indirect constructions by mapping visible - steps to rubric elements and required output behavior. Do not require the - standard direct construction when an indirect route demonstrates the same - result. - -Apply open-ended adequacy for open-ended short-answer, proof, construction, and -essay questions: score whether the answer satisfies the task requirement. Use -the standard answer as an anchor, not as an exhaustive whitelist. Award credit -for valid, relevant, non-contradictory approaches, examples, or constructions -that answer the prompt, even when they are not listed in the expected answer or semantic equivalents. - -Apply official-style adequacy and avoid being overly harsh. Grade for -official-style adequacy, not ideal-answer completeness. Preserve reasonable -partial credit for demonstrated understanding even when terminology, ordering, -or detail is imperfect. Distinguish missing ideal detail from a visible misconception. -Apply large deductions only for material errors, contradictions, -wrong language/output behavior, or missing required answer behavior. - -This is a cross-course prompt contract. Course-specific calibration overlays -must live in the frozen course rubric and packet; do not import named-question -rules, named examples, or specialized subject policies from another course. - -Classify each question before scoring, then record the type: - -- `objective_selection` (multiple choice, matching, true/false): require a - selected option or unambiguous equivalent. Explanations are not required - unless the prompt explicitly requests proof, explanation, justification, or - visible work. -- `calculation`: evaluate result, valid setup/method, transformations or - substitutions, intermediate calculation, and required reasoning. Retain - evidenced method credit when the result is wrong. A correct result without - required work receives only the frozen answer-only allocation. -- `calculation_short_answer`: score derivation and requested short conclusion - as non-overlapping elements; accept valid alternative methods. -- `short_answer` or `conceptual`: score key-term, concept, and relation - evidence without demanding exact reference wording. -- `algorithm` or `construction`: score a viable method, relevant steps, and - required output behavior; accept valid alternatives. -- `proof` or `explanation`: score each required logical link/direction; preserve - independently demonstrated parts when another part is incomplete. -- `diagram`, `geometry`, or `representation`: score visible required objects, - relations, labels, transformations, and conclusion; never infer invisible - diagram work. -- `essay` or `open_response`: score distinct valid, relevant, non-contradictory - claims; do not require fixed order or standard phrasing. - -For mixed questions, use non-overlapping rubric elements for each required -aspect. The frozen rubric, rather than this prompt, sets point values, allowed -increments, and answer-only credit. +## Course-package freeze + +The current course owner supplies `course-package.json`. It must contain one +row per independently scoreable leaf with `question_id`, `max_score`, +`allowed_increment`, visible criteria, accepted alternatives, and any +course-specific deduction or missing-work policy. Confirm it before grading and +do not change it in the middle of a batch. + +## Evidence-first scoring + +Read the complete submission before scoring. Record concise visible evidence +before assigning each score, including any uncertainty about handwriting, +cropped pages, missing work, or page order. Do not infer invisible work. + +Score only the declared leaves in the frozen course package. The course package +sets the question type, score increments, required evidence, acceptable +alternatives, and all partial-credit or answer-only policy. Do not invent a +universal point rule from another assessment. + +Do not create subparts, transfer points between leaves, or count one fact +twice. Accept an unambiguous valid alternative when it meets a declared +criterion. Ignore extra work unrelated to all declared criteria unless it is +adopted for the graded conclusion or the course package says otherwise. + +The live skill intentionally has no named-course calibration overlays. Every +course-specific scoring detail belongs in the current course package, which is +frozen for the batch and reviewed by the course owner. + +A course package may use question-type labels as local routing aids. Its +declared criteria, leaves, and policies always control the grade. ## Submission-level assembly @@ -158,44 +80,76 @@ entry must contain exactly these fields: - `deduction_type` - `points_deducted` -The trace must be grounded in visible work and the frozen rubric, and its -`points_deducted` values must sum exactly to `max_score - score` for that one -leaf. It is a compact audit statement, not a chain of thought. Deduct from the -first material error; do not deduct again for consequences of that same error. -For selected-response work, do not penalize a missing explanation unless the -question explicitly requires it. When a correct calculation answer lacks -required work, use the frozen answer-only cap. A zero score needs an explicit -missing or incorrect reason. Full-credit leaves omit `deduction_trace`; any -leaf with flags or `low` confidence needs a short `attention_note`. Evaluate a -bonus leaf independently from every base leaf. +The trace must be grounded in visible work and the frozen course package. Its +`points_deducted` values must sum exactly to `max_score - score` for that leaf. +It is a compact audit statement, not a chain of thought. Apply the current +course package's deduction order and no-double-count policy. A zero score needs +a specific visible missing or incorrect reason. Full-credit leaves omit +`deduction_trace`; every flagged, medium-confidence, or low-confidence leaf +needs a short `attention_note` for human review. Never place a name, identifier, email address, private path, or raw private file reference in a trace or attention note. +## Marked-page annotations + +For each visible location that supports a deduction, praise, or review flag, +emit one annotation with exactly these fields: + +- `question_id`: a declared score leaf +- `page_id`: an ID from the private rendered `pages.json` +- `box`: `[x, y, width, height]` normalized to `[0, 1]` +- `kind`: `deduction`, `praise`, or `review` +- `label`: a short learner-facing note without personal data + +For a production record written with `--require-annotations`, add praise for +every leaf awarded more than zero, a deduction annotation for every non-full +leaf, and a review annotation for every flagged or non-high-confidence leaf. +A partially correct leaf can therefore need both praise and deduction boxes. +Do not invent a box: a genuinely uncertain location must be flagged for review +instead. + ## Required JSON record Write one JSON object per student before passing it to `write_outputs.py`: ```json { - "student_id": "anonymous_or_filename_student_id", + "student_id": "opaque-submission-id", "scores": [ { - "question_id": "Q1", + "question_id": "leaf-id", "score": 2.0, "max_score": 3.0, "evidence": "Visible work used to justify the score.", "feedback": "Short English feedback for the student.", - "confidence": "high", + "confidence": "medium", "flags": [], "deduction_trace": [ { - "rubric_criterion": "frozen rubric criterion label", + "rubric_criterion": "course-package criterion label", "observed_evidence_or_missing_or_incorrect_part": "Concise visible missing or incorrect part.", "deduction_type": "material_method_error", "points_deducted": 1.0 } - ] + ], + "attention_note": "Short reason for human review." + } + ], + "annotations": [ + { + "question_id": "leaf-id", + "page_id": "private-page-id", + "box": [0.1, 0.1, 0.2, 0.1], + "kind": "deduction", + "label": "Short marking note." + }, + { + "question_id": "leaf-id", + "page_id": "private-page-id", + "box": [0.1, 0.1, 0.2, 0.1], + "kind": "review", + "label": "Please verify this region." } ], "total": 2.0, @@ -206,47 +160,18 @@ Write one JSON object per student before passing it to `write_outputs.py`: Confidence must be `high`, `medium`, or `low`. Use flags such as `unreadable_region`, `missing_page`, `blank_answer`, `page_order_uncertain`, `rubric_ambiguous`, `high_impact_deduction`, or `needs_manual_review`. -`extracted_evidence` and `evidence` must be plain text strings. Do not output -arrays or objects for these fields. If you use `key_term_evidence`, -`concept_evidence`, or `relation_evidence` internally, summarize those layers -inside the single `extracted_evidence` string or the single `evidence` string. +`evidence`, `feedback`, traces, attention notes, and annotation labels must be +short plain-text fields. Do not output names, student numbers, raw paths, or +private filenames in the JSON record. ## Second-pass triggers -Before writing output, revisit the source page for every item with: - -- `low` confidence -- unreadable or cropped work -- blank or apparently missing answers -- high-impact deductions -- total mismatches -- any score that depends on interpreting handwriting - -Also check missed semantic equivalents, missed keyword credit, duplicate credit, -keyword misuse, score-band consistency, score increments, material-error caps, -local contradictions, indirect constructions, open-ended adequacy, -official-style adequacy, and arithmetic. The -`confidence` field must be exactly `high`, `medium`, or `low`, and the exact -total must be recomputed from itemized scores. - -If the second pass still leaves uncertainty, keep the numeric score conservative -and flag the item for teacher review. - -## Route-comparison evidence card - -When comparing a direct multimodal route with a transcription-assisted route, -use the same frozen rubric, gold, split, scoring packet, and review policy. -For every representative disagreement, write an evidence card before changing -anything. Set one primary category: - -- `clear_model_error`: source evidence and frozen rubric support another score. -- `representation_loss`: relevant source evidence was lost, mistranscribed, - reordered, cropped, or otherwise unavailable to a route. -- `rubric_or_gold_conflict`: a course-owner decision is needed. -- `reasonable_severity_difference`: both evidence-grounded scores lie within an - acceptable strictness range. -- `insufficient_evidence`: the source or record cannot support a reliable - decision. - -Do not label disagreement a model failure by default. Keep the card alongside -the route artifacts and final human disposition. +Before writing output, revisit the source page for any non-full, flagged, +medium-confidence, or low-confidence leaf; for unreadable, cropped, blank, or +apparently missing work; and for any total mismatch. Verify score increments, +leaf coverage, deduction-trace arithmetic, and annotation locations against +the frozen course package. + +If uncertainty remains, preserve it in `attention_note`, `review.csv`, and a +`review` annotation when a real page location is known. The teacher decides the +final resolution; do not disguise uncertainty as a confident score. diff --git a/.agents/skills/grade-homework/scripts/annotate_submission.py b/.agents/skills/grade-homework/scripts/annotate_submission.py new file mode 100644 index 0000000..87714cc --- /dev/null +++ b/.agents/skills/grade-homework/scripts/annotate_submission.py @@ -0,0 +1,221 @@ +from __future__ import annotations + +import argparse +import json +import re +import sys +from collections import defaultdict +from pathlib import Path +from typing import Any + +from PIL import Image, ImageDraw, ImageFont + +from roster import RosterError, SAFE_SUBMISSION_ID, require_private_path + + +ANNOTATION_KINDS = {"deduction", "praise", "review"} +COLORS = { + "deduction": (196, 46, 46), + "praise": (35, 130, 72), + "review": (206, 129, 25), +} +WINDOWS_ABSOLUTE_PATH = re.compile(r"(?:^|\s)[A-Za-z]:[\\/]") +PRIVATE_DATA_PATH = re.compile(r"(?:^|[\\/])Data[\\/]", re.IGNORECASE) +FILE_URI = re.compile(r"\bfile://", re.IGNORECASE) +EMAIL_ADDRESS = re.compile(r"\b[^\s@]+@[^\s@]+\.[^\s@]+\b") +IDENTITY_LABEL = re.compile( + "\\b(?:student[ _-]?(?:id|number|name)|name)\\s*[:=]|(?:\\u59d3\\u540d|\\u5b66\\u53f7)\\s*[:\\uff1a]", + re.IGNORECASE, +) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Render validated praise, deduction, and review annotations." + ) + parser.add_argument("pages_manifest", type=Path) + parser.add_argument("annotations_json", type=Path) + parser.add_argument("output_dir", type=Path) + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + output_dir = namespace.output_dir + if output_dir.exists() and any(output_dir.iterdir()): + print("refusing to overwrite an existing marked submission", file=sys.stderr) + return 4 + try: + require_private_path(output_dir, label="marked submission output") + manifest = _load_object(namespace.pages_manifest, "pages manifest") + annotations_record = _load_object(namespace.annotations_json, "annotation record") + pages, submission_id = _validated_pages(manifest, namespace.pages_manifest.parent) + annotations = _validated_annotations(annotations_record, submission_id, set(pages)) + except (OSError, ValueError, RosterError) as error: + print(f"invalid annotation input: {error}", file=sys.stderr) + return 2 + + by_page: dict[str, list[dict[str, Any]]] = defaultdict(list) + for annotation in annotations: + by_page[annotation["page_id"]].append(annotation) + + output_dir.mkdir(parents=True, exist_ok=True) + font = ImageFont.load_default() + rendered_images: list[Image.Image] = [] + try: + for page_id, page_path in pages.items(): + with Image.open(page_path) as source: + image = source.convert("RGB") + _draw_annotations(image, by_page[page_id], font) + target = output_dir / page_path.name + image.save(target) + rendered_images.append(image) + pdf_path = output_dir / "marked.pdf" + rendered_images[0].save( + pdf_path, + save_all=True, + append_images=rendered_images[1:], + resolution=150.0, + ) + except Exception as error: + print(f"annotation rendering failed: {error}", file=sys.stderr) + return 2 + finally: + for image in rendered_images: + image.close() + + print( + json.dumps( + { + "status": "ok", + "submission_id": submission_id, + "page_count": len(pages), + "marked_pdf": "marked.pdf", + }, + sort_keys=True, + ) + ) + return 0 + + +def _load_object(path: Path, label: str) -> dict[str, Any]: + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, json.JSONDecodeError) as error: + raise ValueError(f"{label} could not be read") from error + if not isinstance(payload, dict): + raise ValueError(f"{label} must be a JSON object") + return payload + + +def _validated_pages( + manifest: dict[str, Any], manifest_dir: Path +) -> tuple[dict[str, Path], str]: + submission_id = str(manifest.get("submission_id", "")).strip() + if not SAFE_SUBMISSION_ID.fullmatch(submission_id): + raise ValueError("pages manifest submission_id is invalid") + raw_pages = manifest.get("pages") + if not isinstance(raw_pages, list) or not raw_pages: + raise ValueError("pages manifest requires non-empty pages") + pages: dict[str, Path] = {} + for page in raw_pages: + if not isinstance(page, dict): + raise ValueError("pages manifest page must be an object") + page_id = str(page.get("page_id", "")).strip() + raw_path = str(page.get("path", "")).strip() + if not SAFE_SUBMISSION_ID.fullmatch(page_id): + raise ValueError("pages manifest page_id is invalid") + if not raw_path or Path(raw_path).name != raw_path: + raise ValueError("pages manifest page path must be a filename") + source = manifest_dir / raw_path + if page_id in pages or not source.is_file(): + raise ValueError("pages manifest has duplicate or missing page") + pages[page_id] = source + return pages, submission_id + + +def _validated_annotations( + record: dict[str, Any], submission_id: str, page_ids: set[str] +) -> list[dict[str, Any]]: + if str(record.get("student_id", "")).strip() != submission_id: + raise ValueError("annotation record does not match the pages manifest") + raw_annotations = record.get("annotations") + if not isinstance(raw_annotations, list): + raise ValueError("annotation record requires annotations") + required = {"question_id", "page_id", "box", "kind", "label"} + result = [] + for annotation in raw_annotations: + if not isinstance(annotation, dict) or set(annotation) != required: + raise ValueError("annotation fields are invalid") + question_id = str(annotation["question_id"]).strip() + if not question_id: + raise ValueError("annotation question_id is invalid") + page_id = str(annotation["page_id"]).strip() + if page_id not in page_ids: + raise ValueError("annotation references an unknown page") + kind = annotation["kind"] + if kind not in ANNOTATION_KINDS: + raise ValueError("annotation kind is invalid") + box = annotation["box"] + if ( + not isinstance(box, list) + or len(box) != 4 + or any(isinstance(part, bool) or not isinstance(part, (int, float)) for part in box) + ): + raise ValueError("annotation box is invalid") + x, y, width, height = (float(part) for part in box) + if x < 0 or y < 0 or width <= 0 or height <= 0 or x + width > 1 or y + height > 1: + raise ValueError("annotation box is outside the page") + result.append( + { + "question_id": question_id, + "page_id": page_id, + "box": [x, y, width, height], + "kind": kind, + "label": _safe_label(annotation["label"], submission_id), + } + ) + return result + + +def _safe_label(value: Any, submission_id: str) -> str: + if not isinstance(value, str) or not value.strip() or len(value) > 500: + raise ValueError("annotation label is invalid") + if ( + WINDOWS_ABSOLUTE_PATH.search(value) + or PRIVATE_DATA_PATH.search(value) + or FILE_URI.search(value) + or EMAIL_ADDRESS.search(value) + or IDENTITY_LABEL.search(value) + or submission_id.casefold() in value.casefold() + ): + raise ValueError("annotation label contains private information") + return value.strip() + + +def _draw_annotations( + image: Image.Image, annotations: list[dict[str, Any]], font: ImageFont.ImageFont +) -> None: + draw = ImageDraw.Draw(image) + width, height = image.size + stroke = max(2, min(width, height) // 400) + for annotation in annotations: + x, y, box_width, box_height = annotation["box"] + left = round(x * width) + top = round(y * height) + right = round((x + box_width) * width) + bottom = round((y + box_height) * height) + color = COLORS[annotation["kind"]] + draw.rectangle((left, top, right, bottom), outline=color, width=stroke) + label = annotation["label"] + text_bbox = draw.textbbox((0, 0), label, font=font) + text_width = text_bbox[2] - text_bbox[0] + 6 + text_height = text_bbox[3] - text_bbox[1] + 4 + label_left = min(max(0, left), max(0, width - text_width)) + label_top = top - text_height if top >= text_height else min(height - text_height, bottom) + draw.rectangle( + (label_left, label_top, label_left + text_width, label_top + text_height), + fill=color, + ) + draw.text((label_left + 3, label_top + 2), label, fill=(255, 255, 255), font=font) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.agents/skills/grade-homework/scripts/discover.py b/.agents/skills/grade-homework/scripts/discover.py index 2489d7d..566ab8a 100644 --- a/.agents/skills/grade-homework/scripts/discover.py +++ b/.agents/skills/grade-homework/scripts/discover.py @@ -1,58 +1,116 @@ from __future__ import annotations +import argparse import json +import re import sys from collections import Counter from pathlib import Path +from roster import RosterError, load_roster -SUPPORTED_SUFFIXES = {".pdf", ".png", ".jpg", ".jpeg", ".docx"} + +SUPPORTED_SUFFIXES = { + ".pdf", + ".png", + ".jpg", + ".jpeg", + ".tif", + ".tiff", + ".webp", + ".heic", + ".docx", +} SOLUTION_MARKERS = ("solution", "solutions", "answer", "answers", "rubric", "key") -SKIP_DIRS = {"grades", "__pycache__"} +SKIP_DIRS = {"grades", "rendered", "marked", "annotations", "__pycache__"} def main(argv: list[str] | None = None) -> int: - args = list(sys.argv[1:] if argv is None else argv) - root = Path(args[0]) if args else Path.cwd() + parser = argparse.ArgumentParser(description="Discover a private grading batch.") + parser.add_argument("root", nargs="?", default=".") + parser.add_argument("--roster", type=Path) + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + root = Path(namespace.root) if not root.is_dir(): - print(json.dumps({"solutions_error": f"not a directory: {root}"})) + print(json.dumps({"status": "error", "solutions_error": "not a directory"})) return 2 files = list(_iter_files(root)) candidates = [path for path in files if _is_supported(path)] - solutions = [path for path in candidates if _looks_like_solution(path)] - submissions = [path for path in candidates if path not in set(solutions)] - late_students = sorted( - { - _student_id(path) - for path in submissions - if "_late_" in path.name.lower() - } + submissions_root = root / "submissions" + grouped_mode = submissions_root.is_dir() + course_candidates = [ + path + for path in candidates + if not grouped_mode or not _is_under(path, submissions_root) + ] + solutions = [path for path in course_candidates if _looks_like_solution(path)] + solutions_error = _solutions_error(solutions) + submission_files = [ + path + for path in candidates + if path not in set(solutions) and (not grouped_mode or _is_under(path, submissions_root)) + ] + grouped, ungrouped_count = _group_submission_files( + root=root, submissions_root=submissions_root, files=submission_files, grouped_mode=grouped_mode ) - solutions_error = None - if not solutions: - solutions_error = "no solutions or rubric file found" - elif len(solutions) > 1: - solutions_error = "multiple solutions or rubric candidates found" + grouping_errors: list[str] = [] + if ungrouped_count: + grouping_errors.append("submission_file_without_submission_id") + roster_used = namespace.roster is not None + roster_error = None + if namespace.roster is not None: + try: + roster = load_roster(namespace.roster) + except RosterError as error: + roster = {} + roster_error = str(error) + if roster_error is None: + unknown = set(grouped) - set(roster) + missing = set(roster) - set(grouped) + if unknown: + grouping_errors.append("scan_group_not_in_roster") + if missing: + grouping_errors.append("roster_entry_without_scan_group") + + submissions = [ + { + "student_id": student_id, + "student": student_id, + "files": [ + { + "source_id": f"source-{index:03d}", + "source_order": index, + "suffix": path.suffix.lower(), + } + for index, path in enumerate(paths, start=1) + ], + "late": any("_late_" in path.name.lower() for path in paths), + } + for student_id, paths in sorted(grouped.items()) + ] + late_students = [ + item["student_id"] for item in submissions if item["late"] + ] payload = { - "root": root.resolve().as_posix(), + "status": "ok" if not solutions_error and not roster_error and not grouping_errors else "review_required", + # A batch manifest must remain portable and must not reveal a local path. + "root": ".", "solutions_error": solutions_error, "solutions_candidates": [_rel(root, path) for path in sorted(solutions)], - "submissions": [ - { - "student": _student_id(path), - "path": _rel(root, path), - "suffix": path.suffix.lower(), - "late": "_late_" in path.name.lower(), - } - for path in sorted(submissions, key=lambda path: (_student_id(path), _rel(root, path))) - ], + "submissions": submissions, "late_students": late_students, - "extension_counts": dict(sorted(Counter(path.suffix.lower() for path in submissions).items())), + "extension_counts": dict( + sorted(Counter(path.suffix.lower() for path in submission_files).items()) + ), + "grouping_mode": "submission_directories" if grouped_mode else "legacy_filename_prefix", + "roster_used": roster_used, + "grouping_errors": grouping_errors, + "roster_error": roster_error, } print(json.dumps(payload, indent=2, sort_keys=True)) - return 0 + return 0 if payload["status"] == "ok" else 3 def _iter_files(root: Path) -> list[Path]: @@ -74,6 +132,51 @@ def _looks_like_solution(path: Path) -> bool: return any(marker in name for marker in SOLUTION_MARKERS) +def _solutions_error(solutions: list[Path]) -> str | None: + if not solutions: + return "no solutions or rubric file found" + if len(solutions) > 1: + return "multiple solutions or rubric candidates found" + return None + + +def _group_submission_files( + *, + root: Path, + submissions_root: Path, + files: list[Path], + grouped_mode: bool, +) -> tuple[dict[str, list[Path]], int]: + grouped: dict[str, list[Path]] = {} + ungrouped_count = 0 + for path in files: + if grouped_mode: + relative = path.relative_to(submissions_root) + if len(relative.parts) < 2: + ungrouped_count += 1 + continue + student_id = relative.parts[0] + else: + student_id = _student_id(path) + grouped.setdefault(student_id, []).append(path) + for paths in grouped.values(): + paths.sort(key=lambda path: _natural_path_key(_rel(root, path))) + return grouped, ungrouped_count + + +def _is_under(path: Path, parent: Path) -> bool: + try: + path.relative_to(parent) + except ValueError: + return False + return True + + +def _natural_path_key(value: str) -> tuple[object, ...]: + parts = re.split(r"(\d+)", value.casefold()) + return tuple(int(part) if part.isdigit() else part for part in parts) + + def _student_id(path: Path) -> str: stem = path.stem return stem.split("_", 1)[0] if "_" in stem else stem diff --git a/.agents/skills/grade-homework/scripts/render_submission.py b/.agents/skills/grade-homework/scripts/render_submission.py new file mode 100644 index 0000000..646ce81 --- /dev/null +++ b/.agents/skills/grade-homework/scripts/render_submission.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +from roster import RosterError, SAFE_SUBMISSION_ID, require_private_path +import to_images + + +SUPPORTED_SUFFIXES = to_images.IMAGE_SUFFIXES | {".pdf", ".docx"} + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Render one complete private submission without page overwrites." + ) + parser.add_argument("submission_dir", type=Path) + parser.add_argument("output_dir", type=Path) + parser.add_argument("--submission-id") + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + source_dir = namespace.submission_dir + output_dir = namespace.output_dir + submission_id = (namespace.submission_id or source_dir.name).strip() + if not SAFE_SUBMISSION_ID.fullmatch(submission_id): + print("invalid submission_id", file=sys.stderr) + return 2 + if not source_dir.is_dir(): + print("submission directory is missing", file=sys.stderr) + return 2 + if output_dir.exists() and any(output_dir.iterdir()): + print("refusing to overwrite an existing rendered submission", file=sys.stderr) + return 4 + try: + require_private_path(source_dir, label="submission directory") + require_private_path(output_dir, label="rendered submission output") + except RosterError as error: + print(f"private-output check failed: {error}", file=sys.stderr) + return 2 + + sources = _source_files(source_dir) + if not sources: + print("submission directory contains no supported scan files", file=sys.stderr) + return 2 + + output_dir.mkdir(parents=True, exist_ok=True) + pages: list[dict[str, object]] = [] + page_order = 0 + try: + for source_order, source in enumerate(sources, start=1): + prefix = f"source-{source_order:03d}" + rendered = to_images.render_file(source, output_dir, prefix=prefix) + for source_page_order, page in enumerate(rendered, start=1): + page_order += 1 + pages.append( + { + "page_id": page.stem, + "path": page.name, + "source_id": f"source-{source_order:03d}", + "source_order": source_order, + "source_page_order": source_page_order, + "page_order": page_order, + } + ) + except Exception as error: + print(f"render failed: {error}", file=sys.stderr) + return 2 + + manifest = { + "schema_version": 1, + "submission_id": submission_id, + "page_count": len(pages), + "pages": pages, + } + manifest_path = output_dir / "pages.json" + manifest_path.write_text( + json.dumps(manifest, ensure_ascii=False, indent=2) + "\n", + encoding="utf-8", + newline="\n", + ) + print( + json.dumps( + { + "status": "ok", + "submission_id": submission_id, + "page_count": len(pages), + "manifest": manifest_path.name, + }, + sort_keys=True, + ) + ) + return 0 + + +def _source_files(source_dir: Path) -> list[Path]: + files = [] + for path in source_dir.rglob("*"): + if not path.is_file(): + continue + relative = path.relative_to(source_dir) + if any(part.startswith(".") or part == "__pycache__" for part in relative.parts): + continue + if path.suffix.lower() in SUPPORTED_SUFFIXES: + files.append(path) + return sorted(files, key=lambda path: _natural_key(path.relative_to(source_dir).as_posix())) + + +def _natural_key(value: str) -> tuple[object, ...]: + return tuple( + int(part) if part.isdigit() else part + for part in re.split(r"(\d+)", value.casefold()) + ) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.agents/skills/grade-homework/scripts/roster.py b/.agents/skills/grade-homework/scripts/roster.py new file mode 100644 index 0000000..14c2edc --- /dev/null +++ b/.agents/skills/grade-homework/scripts/roster.py @@ -0,0 +1,143 @@ +from __future__ import annotations + +import csv +import argparse +import json +import re +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path + + +REQUIRED_COLUMNS = ("submission_id", "student_name", "student_number") +SAFE_SUBMISSION_ID = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") + + +class RosterError(ValueError): + """A private roster does not meet the local delivery contract.""" + + +@dataclass(frozen=True) +class RosterEntry: + submission_id: str + student_name: str + student_number: str + + +def load_roster(path: Path) -> dict[str, RosterEntry]: + """Load a private roster without exposing its contents in diagnostics.""" + + if not path.is_file(): + raise RosterError("roster file is missing") + require_private_path(path, label="roster file") + + try: + with path.open("r", encoding="utf-8-sig", newline="") as handle: + reader = csv.DictReader(handle) + fieldnames = tuple(reader.fieldnames or ()) + if set(fieldnames) != set(REQUIRED_COLUMNS) or len(fieldnames) != len( + REQUIRED_COLUMNS + ): + raise RosterError( + "roster columns must be exactly submission_id,student_name,student_number" + ) + rows = list(reader) + except (OSError, UnicodeError, csv.Error) as error: + raise RosterError("roster could not be read") from error + + if not rows: + raise RosterError("roster must contain at least one student") + + entries: dict[str, RosterEntry] = {} + seen_numbers: set[str] = set() + for row in rows: + submission_id = _required_cell(row, "submission_id") + student_name = _required_cell(row, "student_name") + student_number = _required_cell(row, "student_number") + if not SAFE_SUBMISSION_ID.fullmatch(submission_id): + raise RosterError("roster submission_id contains unsupported characters") + if len(student_name) > 256 or len(student_number) > 128: + raise RosterError("roster field exceeds its supported length") + if submission_id in entries: + raise RosterError("roster contains duplicate submission_id") + if student_number in seen_numbers: + raise RosterError("roster contains duplicate student_number") + entries[submission_id] = RosterEntry( + submission_id=submission_id, + student_name=student_name, + student_number=student_number, + ) + seen_numbers.add(student_number) + return entries + + +def require_private_path(path: Path, *, label: str) -> None: + """Reject private input or output under a tracked Git location.""" + + resolved = path.resolve() + repository_root = next( + ( + parent + for parent in (resolved, *resolved.parents) + if (parent / ".git").exists() + ), + None, + ) + if repository_root is None: + return + try: + relative = resolved.relative_to(repository_root).as_posix() + except ValueError: + return + check = subprocess.run( + [ + "git", + "-c", + f"safe.directory={repository_root.as_posix()}", + "-C", + str(repository_root), + "check-ignore", + "--quiet", + "--no-index", + "--", + relative, + ], + check=False, + capture_output=True, + text=True, + ) + if check.returncode == 0: + return + if check.returncode == 1: + raise RosterError(f"{label} inside a Git worktree must be private and ignored") + raise RosterError(f"could not verify whether {label} is ignored") + + +def _required_cell(row: dict[str, str | None], key: str) -> str: + value = row.get(key) + if value is None: + raise RosterError("roster row is missing a required value") + normalized = value.strip() + if not normalized: + raise RosterError("roster row contains a blank required value") + return normalized + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Validate a private roster without printing its contents." + ) + parser.add_argument("private_roster", type=Path) + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + try: + roster = load_roster(namespace.private_roster) + except RosterError as error: + print(f"invalid roster: {error}", file=sys.stderr) + return 2 + print(json.dumps({"status": "ok", "student_count": len(roster)}, sort_keys=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.agents/skills/grade-homework/scripts/to_images.py b/.agents/skills/grade-homework/scripts/to_images.py index 098046b..a2e399e 100644 --- a/.agents/skills/grade-homework/scripts/to_images.py +++ b/.agents/skills/grade-homework/scripts/to_images.py @@ -1,6 +1,8 @@ from __future__ import annotations +import argparse import json +import re import shutil import subprocess import sys @@ -9,34 +11,68 @@ from PIL import Image, ImageOps +from roster import RosterError, require_private_path -IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg"} +IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg", ".tif", ".tiff", ".webp", ".heic"} + + +def _validated_prefix(value: str) -> str: + normalized = value.strip() + if not normalized: + return "" + if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._-]{0,80}", normalized): + raise ValueError("prefix must contain only letters, digits, dot, underscore, or dash") + return normalized + + +def _page_target(output_dir: Path, prefix: str, index: int) -> Path: + stem = f"{prefix}-page" if prefix else "page" + return output_dir / f"{stem}-{index:03d}.png" + + +def _ensure_new_target(target: Path) -> None: + if target.exists(): + raise FileExistsError(f"refusing to overwrite rendered page: {target.name}") -def main(argv: list[str] | None = None) -> int: - args = list(sys.argv[1:] if argv is None else argv) - if len(args) != 2: - print(json.dumps({"status": "error", "error": "usage: to_images.py "})) - return 2 - source = Path(args[0]) - output_dir = Path(args[1]) - output_dir.mkdir(parents=True, exist_ok=True) +def render_file(source: Path, output_dir: Path, *, prefix: str = "") -> list[Path]: + """Render one supported submission source without overwriting pages.""" + + suffix = source.suffix.lower() + if suffix in IMAGE_SUFFIXES: + return [_convert_image(source, output_dir, prefix=prefix)] + if suffix == ".pdf": + return _convert_pdf(source, output_dir, prefix=prefix) + if suffix == ".docx": + return _convert_docx(source, output_dir, prefix=prefix) + raise ValueError(f"unsupported file type: {suffix}") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Render one source file to PNG pages.") + parser.add_argument("input") + parser.add_argument("output_dir") + parser.add_argument("--prefix", default="") + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + source = Path(namespace.input) + output_dir = Path(namespace.output_dir) + try: + prefix = _validated_prefix(namespace.prefix) + except ValueError as error: + parser.error(str(error)) if not source.is_file(): - print(json.dumps({"status": "error", "error": f"missing file: {source}"})) + print(json.dumps({"status": "error", "error": "missing_input_file"})) return 2 - suffix = source.suffix.lower() try: - if suffix in IMAGE_SUFFIXES: - pages = [_convert_image(source, output_dir)] - return _ok(source, pages) - if suffix == ".pdf": - pages = _convert_pdf(source, output_dir) - return _ok(source, pages) - if suffix == ".docx": - pages = _convert_docx(source, output_dir) - return _ok(source, pages) + require_private_path(output_dir, label="rendered-page output") + output_dir.mkdir(parents=True, exist_ok=True) + pages = render_file(source, output_dir, prefix=prefix) + return _ok(source, pages) + except RosterError as error: + return _error("private_output_required", str(error), code=2) except MissingPdfRenderer as error: return _error("pdf_renderer_missing", str(error), code=2) except MissingDocxConverter as error: @@ -44,17 +80,17 @@ def main(argv: list[str] | None = None) -> int: except Exception as error: return _error("conversion_failed", str(error), code=2) - return _error("unsupported_file_type", suffix, code=2) -def _convert_image(source: Path, output_dir: Path) -> Path: - target = output_dir / "page-001.png" +def _convert_image(source: Path, output_dir: Path, *, prefix: str) -> Path: + target = _page_target(output_dir, prefix, 1) + _ensure_new_target(target) with Image.open(source) as image: ImageOps.exif_transpose(image).convert("RGB").save(target) return target -def _convert_pdf(source: Path, output_dir: Path) -> list[Path]: +def _convert_pdf(source: Path, output_dir: Path, *, prefix: str) -> list[Path]: try: import fitz # type: ignore except ImportError as exc: @@ -66,13 +102,14 @@ def _convert_pdf(source: Path, output_dir: Path) -> list[Path]: document = fitz.open(str(source)) for index, page in enumerate(document, start=1): pixmap = page.get_pixmap(matrix=fitz.Matrix(2, 2), alpha=False) - target = output_dir / f"page-{index:03d}.png" + target = _page_target(output_dir, prefix, index) + _ensure_new_target(target) pixmap.save(target) pages.append(target) return pages -def _convert_docx(source: Path, output_dir: Path) -> list[Path]: +def _convert_docx(source: Path, output_dir: Path, *, prefix: str) -> list[Path]: converter = shutil.which("soffice") or shutil.which("libreoffice") if converter is None: raise MissingDocxConverter("LibreOffice or soffice is required for DOCX files.") @@ -103,7 +140,7 @@ def _convert_docx(source: Path, output_dir: Path) -> list[Path]: if not matches: raise MissingDocxConverter("DOCX converter did not produce a PDF.") pdf = matches[0] - return _convert_pdf(pdf, output_dir) + return _convert_pdf(pdf, output_dir, prefix=prefix) def _ok(source: Path, pages: list[Path]) -> int: @@ -111,8 +148,8 @@ def _ok(source: Path, pages: list[Path]) -> int: json.dumps( { "status": "ok", - "source": source.as_posix(), - "pages": [page.as_posix() for page in pages], + "page_count": len(pages), + "pages": [page.name for page in pages], }, sort_keys=True, ) diff --git a/.agents/skills/grade-homework/scripts/write_outputs.py b/.agents/skills/grade-homework/scripts/write_outputs.py index 895d2b6..4fd1dcc 100644 --- a/.agents/skills/grade-homework/scripts/write_outputs.py +++ b/.agents/skills/grade-homework/scripts/write_outputs.py @@ -1,16 +1,36 @@ from __future__ import annotations +import argparse import csv import json import re -import subprocess import sys from pathlib import Path from typing import Any +from roster import ( + RosterEntry, + RosterError, + SAFE_SUBMISSION_ID, + load_roster, + require_private_path, +) + -BASE_COLUMNS = ["student_id"] -TAIL_COLUMNS = ["total", "flags"] +BASE_COLUMNS = ["student_id", "student_name", "student_number"] +TAIL_COLUMNS = ["total", "uncertainties", "flags"] +REVIEW_COLUMNS = [ + "student_id", + "student_name", + "student_number", + "question_id", + "score", + "max_score", + "confidence", + "uncertainty", + "flags", +] +ANNOTATION_KINDS = {"deduction", "praise", "review"} DEDUCTION_TYPES = { "answer_only_cap", "blank_or_missing_answer", @@ -28,32 +48,51 @@ FILE_URI = re.compile(r"\bfile://", re.IGNORECASE) EMAIL_ADDRESS = re.compile(r"\b[^\s@]+@[^\s@]+\.[^\s@]+\b") IDENTITY_LABEL = re.compile( - r"\b(?:student[ _-]?(?:id|number|name)|name)\s*[:=]|(?:姓名|学号)\s*[::]", + "\\b(?:student[ _-]?(?:id|number|name)|name)\\s*[:=]|(?:\\u59d3\\u540d|\\u5b66\\u53f7)\\s*[:\\uff1a]", re.IGNORECASE, ) def main(argv: list[str] | None = None) -> int: - args = list(sys.argv[1:] if argv is None else argv) - if len(args) != 1: - print("usage: write_outputs.py ", file=sys.stderr) - return 2 - - grades_dir = Path(args[0]) + parser = argparse.ArgumentParser(description="Write private grading outputs.") + parser.add_argument("grades_dir", type=Path) + parser.add_argument("--roster", type=Path) + parser.add_argument("--course-package", type=Path) + parser.add_argument("--require-annotations", action="store_true") + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + grades_dir = namespace.grades_dir try: - _require_private_output_directory(grades_dir) - grades_dir.mkdir(parents=True, exist_ok=True) - feedback_dir = grades_dir / "feedback" - feedback_dir.mkdir(exist_ok=True) + require_private_path(grades_dir, label="grades directory") + roster = load_roster(namespace.roster) if namespace.roster else None + course_leaves = ( + _load_course_package(namespace.course_package) + if namespace.course_package + else None + ) record = json.loads(sys.stdin.read()) - normalized = _normalize_record(record) + normalized = _normalize_record( + record, + course_leaves=course_leaves, + require_annotations=namespace.require_annotations, + ) + roster_entry = _roster_entry(normalized["student_id"], roster) except Exception as error: print(f"invalid record: {error}", file=sys.stderr) return 2 + grades_dir.mkdir(parents=True, exist_ok=True) + feedback_dir = grades_dir / "feedback" + annotation_dir = grades_dir / "annotations" + feedback_dir.mkdir(exist_ok=True) + annotation_dir.mkdir(exist_ok=True) csv_path = grades_dir / "grades.csv" - header = BASE_COLUMNS + [item["question_id"] for item in normalized["scores"]] + TAIL_COLUMNS + question_ids = ( + list(course_leaves) + if course_leaves is not None + else [item["question_id"] for item in normalized["scores"]] + ) + header = BASE_COLUMNS + question_ids + TAIL_COLUMNS existing_rows = _read_existing_rows(csv_path) if existing_rows is not None: existing_header, rows = existing_rows @@ -64,30 +103,47 @@ def main(argv: list[str] | None = None) -> int: print(json.dumps({"status": "skipped", "student_id": normalized["student_id"]})) return 0 + score_by_id = {item["question_id"]: item for item in normalized["scores"]} row = { "student_id": normalized["student_id"], + "student_name": roster_entry.student_name if roster_entry else "", + "student_number": roster_entry.student_number if roster_entry else "", "total": _format_score(normalized["total"]), + "uncertainties": _uncertainty_cell(normalized), "flags": ";".join(normalized["flags"]), } - for item in normalized["scores"]: - row[item["question_id"]] = _format_score(item["score"]) - - write_header = not csv_path.exists() - with csv_path.open("a", encoding="utf-8", newline="") as handle: - writer = csv.DictWriter(handle, fieldnames=header) - if write_header: - writer.writeheader() - writer.writerow(row) + for question_id in question_ids: + row[question_id] = _format_score(score_by_id[question_id]["score"]) + _append_csv(csv_path, header, row) + review_path = grades_dir / "review.csv" + _append_review_rows(review_path, normalized, roster_entry) feedback_path = feedback_dir / f"{_safe_name(normalized['student_id'])}.md" feedback_path.write_text(_feedback_markdown(normalized), encoding="utf-8", newline="\n") + annotation_path = annotation_dir / f"{_safe_name(normalized['student_id'])}.json" + annotation_path.write_text( + json.dumps( + { + "schema_version": 1, + "student_id": normalized["student_id"], + "annotations": normalized["annotations"], + }, + ensure_ascii=False, + indent=2, + ) + + "\n", + encoding="utf-8", + newline="\n", + ) print( json.dumps( { "status": "written", "student_id": normalized["student_id"], - "grades_csv": csv_path.as_posix(), - "feedback": feedback_path.as_posix(), + "grades_csv": "grades.csv", + "review_csv": "review.csv", + "feedback": f"feedback/{feedback_path.name}", + "annotations": f"annotations/{annotation_path.name}", }, sort_keys=True, ) @@ -95,32 +151,53 @@ def main(argv: list[str] | None = None) -> int: return 0 -def _normalize_record(record: dict[str, Any]) -> dict[str, Any]: +def _normalize_record( + record: dict[str, Any], + *, + course_leaves: dict[str, dict[str, float]] | None, + require_annotations: bool, +) -> dict[str, Any]: if not isinstance(record, dict): raise ValueError("record must be a JSON object") - student_id = str(record.get("student_id") or record.get("student") or "").strip() + if "student_name" in record or "student_number" in record: + raise ValueError("student names and numbers must come from the private roster") + student_id = str( + record.get("student_id") or record.get("submission_id") or record.get("student") or "" + ).strip() if not student_id: raise ValueError("student_id is required") + if not SAFE_SUBMISSION_ID.fullmatch(student_id): + raise ValueError("student_id contains unsupported characters") scores = record.get("scores") if not isinstance(scores, list) or not scores: raise ValueError("scores must be a non-empty list") normalized_scores = [] - flags = list(_as_list(record.get("flags", []))) + seen_question_ids: set[str] = set() + flags = _normalized_flags(record.get("flags", []), label="record") for item in scores: if not isinstance(item, dict): raise ValueError("each score item must be an object") question_id = str(item.get("question_id", "")).strip() if not question_id: raise ValueError("question_id is required") + if question_id in seen_question_ids: + raise ValueError(f"duplicate question_id: {question_id}") + seen_question_ids.add(question_id) score = float(item.get("score")) max_score = _positive_score(item.get("max_score"), "max_score", question_id) if score < 0 or score > max_score: raise ValueError(f"score is outside range for {question_id}") + _validate_course_score( + question_id=question_id, + score=score, + max_score=max_score, + course_leaves=course_leaves, + ) confidence = str(item.get("confidence", "")).strip().lower() if confidence not in {"high", "medium", "low"}: raise ValueError(f"invalid confidence for {question_id}: {confidence}") - item_flags = list(_as_list(item.get("flags", []))) + item_flags = _normalized_flags(item.get("flags", []), label=question_id) deduction_trace = _normalize_deduction_trace( item.get("deduction_trace"), question_id=question_id, @@ -131,10 +208,10 @@ def _normalize_record(record: dict[str, Any]) -> dict[str, Any]: attention_note = item.get("attention_note") if attention_note is not None: attention_note = _safe_trace_text(attention_note, "attention_note", student_id) - if item_flags or confidence == "low": + if item_flags or confidence != "high": if attention_note is None: raise ValueError( - f"flags or low confidence require attention_note for {question_id}" + f"flags or non-high confidence require attention_note for {question_id}" ) flags.extend(f"{question_id}:{flag}" for flag in item_flags) normalized_scores.append( @@ -142,20 +219,37 @@ def _normalize_record(record: dict[str, Any]) -> dict[str, Any]: "question_id": question_id, "score": score, "max_score": max_score, - "evidence": str(item.get("evidence", "")).strip(), - "feedback": str(item.get("feedback", "")).strip(), + "evidence": _safe_record_text( + item.get("evidence"), "evidence", student_id, required=True + ), + "feedback": _safe_record_text( + item.get("feedback", ""), "feedback", student_id, required=False + ), "confidence": confidence, "flags": item_flags, "deduction_trace": deduction_trace, "attention_note": attention_note, } ) - total = float(record.get("total", sum(item["score"] for item in normalized_scores))) + if course_leaves is not None and set(seen_question_ids) != set(course_leaves): + raise ValueError("record score leaves do not match the course package") + expected_total = sum(item["score"] for item in normalized_scores) + total = float(record.get("total", expected_total)) + if abs(total - expected_total) > 1e-9: + raise ValueError("total must equal the sum of leaf scores") + annotations = _normalize_annotations( + record.get("annotations", []), + student_id=student_id, + question_ids=seen_question_ids, + score_by_id={item["question_id"]: item for item in normalized_scores}, + require_annotations=require_annotations, + ) return { "student_id": student_id, "scores": normalized_scores, + "annotations": annotations, "total": total, - "flags": sorted(set(str(flag) for flag in flags if str(flag).strip())), + "flags": sorted(set(flags)), } @@ -167,42 +261,220 @@ def _read_existing_rows(path: Path) -> tuple[list[str], list[dict[str, str]]] | return list(reader.fieldnames or []), list(reader) -def _require_private_output_directory(grades_dir: Path) -> None: - """Reject a new per-person grading record in an unignored Git location.""" - - resolved = grades_dir.resolve() - repository_root = next( - (parent for parent in (resolved, *resolved.parents) if (parent / ".git").exists()), - None, - ) - if repository_root is None: - return +def _load_course_package(path: Path) -> dict[str, dict[str, float]]: try: - relative = resolved.relative_to(repository_root).as_posix() - except ValueError: - return - check = subprocess.run( - [ - "git", - "-C", - str(repository_root), - "check-ignore", - "--quiet", - "--no-index", - "--", - relative, - ], - check=False, - capture_output=True, - text=True, - ) - if check.returncode == 0: + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, json.JSONDecodeError) as error: + raise ValueError("course package could not be read") from error + if not isinstance(payload, dict): + raise ValueError("course package must be a JSON object") + leaves = payload.get("score_leaves") + if not isinstance(leaves, list) or not leaves: + raise ValueError("course package requires non-empty score_leaves") + + result: dict[str, dict[str, float]] = {} + for leaf in leaves: + if not isinstance(leaf, dict): + raise ValueError("course package score leaf must be an object") + question_id = str(leaf.get("question_id", "")).strip() + if not question_id: + raise ValueError("course package score leaf requires question_id") + if question_id in result: + raise ValueError("course package has duplicate question_id") + max_score = _positive_score( + leaf.get("max_score"), "course-package max_score", question_id + ) + increment = _positive_score( + leaf.get("allowed_increment"), + "course-package allowed_increment", + question_id, + ) + if not _on_increment(max_score, increment): + raise ValueError( + f"course-package max_score is not on its allowed increment for {question_id}" + ) + result[question_id] = { + "max_score": max_score, + "allowed_increment": increment, + } + return result + + +def _validate_course_score( + *, + question_id: str, + score: float, + max_score: float, + course_leaves: dict[str, dict[str, float]] | None, +) -> None: + if course_leaves is None: return - if check.returncode == 1: - raise ValueError( - "grades directory inside a Git worktree must be private and ignored" + expected = course_leaves.get(question_id) + if expected is None: + raise ValueError(f"question_id is not declared by the course package: {question_id}") + if abs(max_score - expected["max_score"]) > 1e-9: + raise ValueError(f"max_score disagrees with the course package for {question_id}") + if not _on_increment(score, expected["allowed_increment"]): + raise ValueError(f"score is off the allowed increment for {question_id}") + + +def _on_increment(value: float, increment: float) -> bool: + return abs(value / increment - round(value / increment)) <= 1e-9 + + +def _roster_entry( + student_id: str, roster: dict[str, RosterEntry] | None +) -> RosterEntry | None: + if roster is None: + return None + entry = roster.get(student_id) + if entry is None: + raise ValueError("student_id is not present in the private roster") + return entry + + +def _append_csv(path: Path, header: list[str], row: dict[str, str]) -> None: + write_header = not path.exists() + with path.open("a", encoding="utf-8", newline="") as handle: + writer = csv.DictWriter(handle, fieldnames=header) + if write_header: + writer.writeheader() + writer.writerow(row) + + +def _append_review_rows( + path: Path, + record: dict[str, Any], + roster_entry: RosterEntry | None, +) -> None: + rows = [] + for item in record["scores"]: + if item["confidence"] == "high" and not item["flags"]: + continue + rows.append( + { + "student_id": record["student_id"], + "student_name": roster_entry.student_name if roster_entry else "", + "student_number": roster_entry.student_number if roster_entry else "", + "question_id": item["question_id"], + "score": _format_score(item["score"]), + "max_score": _format_score(item["max_score"]), + "confidence": item["confidence"], + "uncertainty": item["attention_note"] or "", + "flags": ";".join(item["flags"]), + } + ) + if record["flags"]: + rows.append( + { + "student_id": record["student_id"], + "student_name": roster_entry.student_name if roster_entry else "", + "student_number": roster_entry.student_number if roster_entry else "", + "question_id": "", + "score": "", + "max_score": "", + "confidence": "", + "uncertainty": "Submission-level review required.", + "flags": ";".join(record["flags"]), + } + ) + write_header = not path.exists() + with path.open("a", encoding="utf-8", newline="") as handle: + writer = csv.DictWriter(handle, fieldnames=REVIEW_COLUMNS) + if write_header: + writer.writeheader() + writer.writerows(rows) + + +def _uncertainty_cell(record: dict[str, Any]) -> str: + parts = [ + f"{item['question_id']}: {item['attention_note']}" + for item in record["scores"] + if item["confidence"] != "high" or item["flags"] + ] + if record["flags"]: + parts.append("submission: " + ", ".join(record["flags"])) + return " | ".join(parts) + + +def _normalize_annotations( + value: Any, + *, + student_id: str, + question_ids: set[str], + score_by_id: dict[str, dict[str, Any]], + require_annotations: bool, +) -> list[dict[str, Any]]: + if value is None: + value = [] + if not isinstance(value, list): + raise ValueError("annotations must be a list") + required = {"question_id", "page_id", "box", "kind", "label"} + normalized = [] + for annotation in value: + if not isinstance(annotation, dict) or set(annotation) != required: + raise ValueError("annotations require exactly question_id,page_id,box,kind,label") + question_id = str(annotation["question_id"]).strip() + if question_id not in question_ids: + raise ValueError("annotation question_id is not a scored leaf") + page_id = str(annotation["page_id"]).strip() + if not SAFE_SUBMISSION_ID.fullmatch(page_id): + raise ValueError("annotation page_id contains unsupported characters") + kind = annotation["kind"] + if kind not in ANNOTATION_KINDS: + raise ValueError("annotation kind is invalid") + box = annotation["box"] + if ( + not isinstance(box, list) + or len(box) != 4 + or any(isinstance(part, bool) or not isinstance(part, (int, float)) for part in box) + ): + raise ValueError("annotation box must contain four numeric normalized values") + x, y, width, height = (float(part) for part in box) + if x < 0 or y < 0 or width <= 0 or height <= 0 or x + width > 1 or y + height > 1: + raise ValueError("annotation box must stay inside the normalized page") + normalized.append( + { + "question_id": question_id, + "page_id": page_id, + "box": [x, y, width, height], + "kind": kind, + "label": _safe_trace_text(annotation["label"], "annotation label", student_id), + } ) - raise ValueError("could not verify whether the grades directory is ignored") + + if require_annotations: + for question_id, item in score_by_id.items(): + matching = [entry for entry in normalized if entry["question_id"] == question_id] + if item["score"] > 0 and not any( + entry["kind"] == "praise" for entry in matching + ): + raise ValueError(f"score-bearing {question_id} requires a praise annotation") + if item["score"] < item["max_score"] and not any( + entry["kind"] == "deduction" for entry in matching + ): + raise ValueError(f"non-full {question_id} requires a deduction annotation") + if (item["confidence"] != "high" or item["flags"]) and not any( + entry["kind"] == "review" for entry in matching + ): + raise ValueError(f"review-needed {question_id} requires a review annotation") + return normalized + + +def _normalized_flags(value: Any, *, label: str) -> list[str]: + flags = _as_list(value) + normalized = [] + for flag in flags: + if not re.fullmatch(r"[a-z][a-z0-9_:-]{0,127}", flag): + raise ValueError(f"invalid flag for {label}") + normalized.append(flag) + return normalized + + +def _require_private_output_directory(grades_dir: Path) -> None: + """Backward-compatible private-output guard.""" + + require_private_path(grades_dir, label="grades directory") def _feedback_markdown(record: dict[str, Any]) -> str: @@ -339,6 +611,20 @@ def _safe_trace_text(value: Any, label: str, student_id: str) -> str: return value.strip() +def _safe_record_text( + value: Any, label: str, student_id: str, *, required: bool +) -> str: + if value is None: + value = "" + if not isinstance(value, str): + raise ValueError(f"{label} must be plain text") + if not value.strip(): + if required: + raise ValueError(f"{label} must be non-blank text") + return "" + return _safe_trace_text(value, label, student_id) + + def _format_score(value: float) -> str: return f"{value:g}" diff --git a/.claude/skills/grade-homework/SKILL.md b/.claude/skills/grade-homework/SKILL.md index 64248fb..663501e 100644 --- a/.claude/skills/grade-homework/SKILL.md +++ b/.claude/skills/grade-homework/SKILL.md @@ -1,68 +1,49 @@ --- name: grade-homework -description: Use when the user wants to grade a folder of student homework, quiz, or exam submissions against a teacher-provided solutions or rubric document. Handles mixed PDF, image, and DOCX inputs, produces a grades CSV and per-student English feedback, and explicitly flags ambiguous, unreadable, missing, or high-impact grading items for teacher review. Triggers on phrases like "grade the homework", "mark HW9", "batch grade submissions", or "批作业". +description: Use when a teacher or TA needs to grade a batch of scanned homework, quiz, or exam submissions against a course-provided rubric. It groups multi-page scans, keeps the roster private, produces grades and review CSVs, and renders checked annotations on each submission. --- # grade-homework -## When to use +## Scope and privacy boundary -- User wants to grade a folder of student submissions. -- A solutions document is present (either auto-discoverable by filename or provided explicitly). -- Student filenames follow `_..._.`. +This is a cross-course delivery skill. It provides evidence-based grading, +private roster handling, scan rendering, structured outputs, annotations, and +human-review handoff. It does not supply subject knowledge, point allocations, +partial-credit bands, canonical forms, required work, or penalties. Those rules +belong only in the frozen course package for the current assessment. -Do NOT use for: single-file grading (just read it inline), plagiarism detection, or rewriting the solutions doc. +Never import a rule, example, point value, or error pattern from a previous +course or data set. If the current course package does not settle a scoring +question, stop and ask the course owner instead of guessing. ## What this skill produces -- `/grades/grades.csv` — one row per student, per-question columns, total, flags. -- `/grades/feedback/.md` — English feedback, per-question breakdown, flags summary. +- `grades/grades.csv` - opaque ID, local name, local student number, item + scores, total, uncertainties, and flags. +- `grades/review.csv` - one row for every flagged or non-high-confidence leaf. +- `grades/feedback/.md` - concise feedback. +- `grades/annotations/.json` - validated annotation data. +- `grades/marked//` - annotated PNG pages plus a marked PDF. -These are private grading records: choose a directory outside any Git worktree, -or an already ignored private directory. They contain per-person grades, -feedback, and answer-derived evidence; never commit, publish, or copy them into -an experiment's public record. The output helper refuses an unignored directory -inside a Git worktree. +These are private per-person records. They never belong in Git, a public report, +a pull request, or a model prompt. The output helpers reject an unignored +directory inside a Git worktree. -## Prerequisites (conditional on submission formats) +## Private input contract -Only `.docx` submissions need an external conversion toolchain. PDF and image -submissions go through Python alone, so most courses won't need anything -beyond `uv`. +Use a private, ignored working directory with `course-package.json`, +`roster.csv`, and `submissions//` directories. `roster.csv` +must have exactly `submission_id,student_name,student_number`; it is local-only +and must never be included in a model-facing record. Start the course package +from `references/course-package-template.json`, replace every placeholder, and +freeze it before grading begins. -Probe at the start of Step 1 (after `discover.py` reports what's present): +PDFs and ordinary image formats render in Python. DOCX files require +LibreOffice or `soffice`. -```bash -# Only if discover.py finds any .docx submissions -if ls "$PWD"/*.docx >/dev/null 2>&1; then - if ! command -v libreoffice >/dev/null 2>&1 && ! command -v soffice >/dev/null 2>&1; then - if ! { command -v pandoc >/dev/null 2>&1 \ - && { command -v google-chrome >/dev/null 2>&1 \ - || command -v chromium >/dev/null 2>&1; }; }; then - echo "MISSING: docx toolchain (need libreoffice OR pandoc+browser)" - fi - fi -fi -``` - -If the toolchain is missing, ask via `AskUserQuestion` whether to install -before grading begins. Without it, `.docx` submissions are silently flagged -`needs_manual_review` and grading proceeds for the rest — call out exactly -how many students that affects so the user can make an informed choice. - -When the user says yes, figure out the right install command for their -environment at runtime (inspect `uname -s` and which of `brew`/`apt`/`dnf`/ -`pacman`/`winget`/etc. is available — see `bootstrap` Step 0 for the -detection pattern), and propose the commands via `AskUserQuestion` before -running them. Notes on package naming: - -- `libreoffice` may be `libreoffice-fresh` on Arch and `--cask libreoffice` - on Homebrew; on Windows use `TheDocumentFoundation.LibreOffice` via winget. -- Headless browser for the fallback path: `google-chrome` or `chromium` — - either works. -- `inkscape` is only needed by the fallback path when a `.docx` embeds WMF/EMF - images. Install on demand if `to_images.py` flags missing inkscape during a - run; otherwise leave it alone. +If DOCX conversion is unavailable, do not claim that the pages were rendered. +Put that submission in review or install the approved converter before grading. ## Workflow @@ -70,107 +51,27 @@ Skill root: the directory containing this `SKILL.md`. Resolve scripts and references relative to that directory; do not assume a Claude- or Codex-specific home path. -### Candidate grading contract - -Grade from visible evidence, not from assumed intent. For every scored question, -first record the visible equation, statement, diagram feature, answer text, or -blank-answer marker. Assign points only after that evidence is written down. - -For every question, identify the question type and extract -`key_term_evidence`, `concept_evidence`, and `relation_evidence`. Map each -non-overlapping rubric scoring element to exactly one state: `absent`, -`mentioned_only`, `partial_understanding`, `demonstrated`, or -`misused_or_contradicted`. A correctly used relevant keyword may receive the -rubric's limited `mentioned_only` credit; a misused keyword receives no -automatic credit. Treat an unambiguous semantic equivalent as evidence for the -matching element without requiring standard wording, notation, or ordering. - -Award the integer credit for that one state only. Do not award duplicate credit: -a keyword and its explanation belong to one element, and overlapping evidence -cannot receive points twice. Sum non-overlapping element credit into a subtotal. -Use the highest satisfied score band and any material-error cap only as upper -bounds; they cannot raise the subtotal. Full credit requires every required -essential element to be demonstrated or expressed by a semantic equivalent, -required terminology when explicitly requested, and no material contradiction. - -When the final answer is wrong, retain justified process credit for correct -terms, concepts, formulas, substitutions, units, and reasoning unless the -frozen rubric makes the conclusion indispensable. For calculation problems, -arithmetic mistakes should not erase a correct method unless the frozen rubric -requires the exact result. When the final answer is correct and the process is -roughly correct, award full credit when the frozen requirements are met. - -Candidate v3.1 adds four calibration rules for concept, proof, and construction -answers: - -- cap-locality: apply a material-error cap only when the cap condition is directly visible and active; - do not trigger a cap merely because an element is - partial, under-detailed, or expressed through a non-standard but viable route. -- contradiction-locality: when a misconception or contradiction is local to one - element, proof direction, or construction step, preserve unrelated element credit - unless the frozen rubric explicitly defines a question-level cap. -- key-term semantics: key terms are evidence signals, not mandatory wording - unless the rubric or full-credit rule explicitly requires that terminology. - Correctly used key terms can earn limited keyword credit, and semantic - equivalents should still be mapped to the matching rubric element. -- indirect-construction: score valid indirect constructions by mapping visible - steps to rubric elements and required output behavior. Do not require the - standard direct construction when an indirect route demonstrates the same - result. - -Candidate v3.1 r2 adds open-ended adequacy: for open-ended short-answer, proof, -construction, and essay questions, score whether the answer satisfies the task requirement. -Use the standard answer as an anchor, not as an exhaustive whitelist. Award -credit for valid, relevant, non-contradictory approaches, examples, or -constructions that answer the prompt, even when they are not listed in the expected answer or semantic equivalents. - -Candidate v3.2 adds official-style adequacy: grade for official-style adequacy, -not ideal-answer completeness, and avoid being overly harsh. Preserve reasonable -partial credit for demonstrated understanding even when terminology, ordering, -or detail is imperfect. Distinguish missing ideal detail from a visible misconception. -Apply large deductions only for material errors, contradictions, -wrong language/output behavior, or missing required answer behavior. - -Candidate v3.2 is a cross-course contract. Course-specific calibration overlays -belong in that course's frozen rubric and packet, never in this reusable skill. -Do not carry rules for named questions, named languages, named theorems, or a -previous course into another course merely because their labels look similar. - -Classify every question from the prompt and frozen rubric *before* scoring. Use -the most specific applicable type, and record it in the grading record: - -- `objective_selection` (including multiple choice, matching, and true/false): - require a selected option or an unambiguous equivalent. Do not require an - explanation for true/false or other selected-response items unless the prompt - explicitly asks to prove, explain, justify, or show work. -- `calculation`: check the final numeric or symbolic result, method/setup, - transformations or substitutions, intermediate calculation, and any - mathematical or domain reasoning required by the rubric. Retain justified - method credit if the final result is wrong. If the result is correct but - required working is absent, award only the course-frozen answer-only credit; - do not invent a universal amount. -- `calculation_short_answer`: score both the visible derivation and the short - conclusion/classification requested by the question. A valid alternative - derivation is acceptable; the reference solution is an anchor, not a required - route. -- `short_answer` or `conceptual`: combine key-term, concept, and relation - evidence; exact standard-answer wording is not required. -- `algorithm` or `construction`: require a viable method plus relevant steps, - relations, and required output behavior; award credit to valid alternatives. -- `proof` or `explanation`: check each required logical direction/link and - preserve credit for independently completed parts. A missing required part - blocks full credit but does not erase unrelated demonstrated work. -- `diagram`, `geometry`, or `representation`: score the observable required - objects, relations, labels, transformations, and conclusion. Do not assume a - missing diagram feature from accompanying prose. -- `essay` or `open_response`: score distinct valid, relevant, non-contradictory - claims against the task requirement; do not require fixed ordering or - standard phrasing. - -When a question genuinely combines types, use non-overlapping rubric elements -for each required aspect rather than forcing it into a narrower legacy label. -The type controls what evidence is relevant; the frozen rubric controls points, -score increments, and any answer-only allocation. +### Generic grading contract + +Score visible evidence from the complete submission, not assumed intent. Record +a concise evidence note before assigning a score. The frozen course package +controls the question type, score leaves, required evidence, alternatives, +increments, and every partial-credit decision. + +Do not invent subparts, transfer points between leaves, or count one visible +fact twice. Accept an unambiguous valid alternative when it satisfies a +course-package criterion. Do not penalize extra work that is irrelevant to all +declared criteria unless the submission adopts it for the graded conclusion or +the course package explicitly says otherwise. + +The course package owns all scoring detail. It may define different policies for +different question types, methods, representations, answer-only work, and +partial credit. This live skill deliberately does not turn a previous course's +calibration into a universal rule. + +The course package may declare question types when that helps its own rubric. +The type is only a routing aid: the package's explicit criteria, score leaves, +and policies always control the grade. ### Submission-level assembly @@ -222,221 +123,122 @@ frozen rubric; their `points_deducted` values must sum exactly to `max_score - score` for that leaf. This is a short audit record, not hidden reasoning or a chain of thought. -Deduct from the first material error and do not split its dependent downstream -consequences into extra deductions. For selected-response questions, assess an -explanation only when the prompt explicitly requires one. Apply the frozen -answer-only cap when a correct calculation answer lacks required work. A zero -score must state the specific missing or incorrect required evidence. Full-credit -leaves omit `deduction_trace`; a leaf with flags or `low` confidence must add a -brief `attention_note`. Treat bonus leaves as independent leaves: never use a -base-leaf deduction to explain, offset, or replace a bonus decision. +Apply the course package's deduction order and no-double-count policy. A zero +score must state specific visible missing or incorrect evidence. Full-credit +leaves omit `deduction_trace`; a flagged, medium-confidence, or low-confidence +leaf must add a brief `attention_note` and appear in review output. Treat any +course-declared bonus leaf independently from base-leaf scoring. Do not put a name, student identifier, email address, private path, or raw private-file reference in a deduction trace or attention note. -### Calculation calibration guardrails - -For each calculation leaf, classify the first score-affecting issue before -withholding points: absent required work, local notation or arithmetic error, -incorrect formula or method, failed required simplification, or incorrect final -result independent of earlier work. Apply only the frozen rubric criterion or -criteria for that first issue; do not convert its downstream result into a -second deduction. - -Check algebraic equivalence before withholding a symbolic-result or -simplification criterion. Reordered, factored, expanded, or otherwise -unambiguous equivalent expressions satisfy an `equivalent_form_accepted` -requirement. A required canonical simplification can be withheld only when the -course rubric explicitly declares it required. - -When a final-result criterion is separately declared, withhold it when the -reported final result is wrong, even when an earlier local error caused that -result. The no-double-count rule protects only dependent process criteria; it -does not preserve an independently allocated final-result criterion. A course -rubric may define a narrow exception only by explicitly declaring a conditional -carry-forward rule. Treat a wrong formula, wrong governing relation, or invalid -method as a method criterion, not as a local arithmetic error. Ignore extra -work that is irrelevant to every declared criterion; evaluate extra work only -when it directly contradicts a declared criterion. - -Freeze the grading protocol before student grading starts: - -- page ordering for solutions and each student submission -- rubric question IDs, maximum points, and allowed score increments -- partial-credit rules; do not introduce quarter-point or 0.25-point scores -- treatment of missing pages, blank answers, unreadable work, and alternative correct methods - -Treat transcript or OCR text as an optional aid. The Physics Week 9 pilot does -not prove that transcript workflows are generally better than direct-image -grading, so never claim that a transcript route is automatically more accurate. - -### Benchmark-informed safeguards - -The Physics Week 9 internal benchmark does not prove that transcript workflows -are generally better than the direct-image baseline. Treat transcript or OCR -steps as optional evidence aids, not as an automatic accuracy improvement. - -Before grading, freeze the page ordering, rubric, question IDs, point ranges, -and allowed score increments. During grading, use an evidence-first pass: record the -visible equation, statement, text, or blank-answer marker before assigning -points. Run a second-pass review for low confidence, unreadable regions, blank -or apparently missing answers, total mismatches, and high-impact deductions. -At handoff, report flagged items and which questions they concentrate on; ask -the teacher to spot-check at least 3 students and all flagged items before -publishing grades. - -### Route comparison and calibration - -If a course authorizes a route comparison, evaluate direct multimodal grading -and transcription-assisted grading as separate conditions with the same frozen -rubric, gold, split, prompt packet, and review policy. Transcription is an -evidence aid, not ground truth; never let it silently replace the page image. - -For each representative disagreement, create an evidence card before changing -the prompt, rubric, or skill. Classify it as exactly one primary cause: - -- `clear_model_error`: visible source evidence and frozen rubric support a - different score. -- `representation_loss`: a route lost, mistranscribed, reordered, cropped, or - failed to expose relevant source evidence. -- `rubric_or_gold_conflict`: the frozen scoring rule or reference answer needs - course-owner adjudication. -- `reasonable_severity_difference`: both readings are evidence-supported but - differ within an acceptable strictness range. -- `insufficient_evidence`: source quality or record does not support a reliable - conclusion. - -Do not assume a disagreement is a model error. Preserve the card, route -artifacts, and human decision so future prompt changes are auditable. - -### Step 1 — Discover - -Run `discover.py` on the working directory containing submissions. Parse the JSON: +### Course-package boundary -```bash -python /scripts/discover.py "$PWD" -``` +The current course package decides how calculations, selected responses, +proofs, diagrams, simplification, alternatives, bonus work, partial credit, +and dependent consequences are handled. This skill only enforces that the +chosen score is evidence-based, traceable, and arithmetically valid. OCR or +transcription may assist reading but never replaces the source pages. -If `solutions_error` is non-null, surface it to the user and stop. If the user passed a solutions path explicitly, use that instead of auto-discovery. +### Release safeguards -`discover.py` recurses into subdirectories (so `./submissions/` is picked up automatically) and skips hidden files/dirs. The JSON also includes a `late_students` list — student names whose filename contains `_LATE_` (case-insensitive). Surface the list to the user before grading so they can decide whether to apply a late-submission policy. +Before release, review every row in `review.csv`, inspect every marked page, +and spot-check a representative set of unflagged submissions. A teacher owns +the final score and any course-package correction. -### Step 2 — Load the grading prompt and parse the rubric +### Step 1 - Validate the private batch -Read `/references/grading-prompt.md` and follow it. - -Convert the solutions file to page images: +Validate the roster, then discover the batch: ```bash -python /scripts/to_images.py /tmp/grade-homework/solutions/ +python /scripts/roster.py roster.csv +python /scripts/discover.py . --roster roster.csv ``` -View the solutions images, verify deterministic **page ordering**, parse the -`[N pts]` allocations into a rubric table, and **confirm with the user before -continuing**. Freeze the page list, rubric, question IDs, point ranges, and -allowed score increments before grading. If no `[N pts]` markers are found, stop and -ask for point allocations. - -Partial-credit conventions: -1. Do not use 0.25-point or quarter-point scores. -2. Award full credit when the student's final answer is correct and the process - is roughly correct, including mathematically equivalent alternative methods. -3. Deduct process points only when the final answer is correct but the process - seriously conflicts with the standard solution, required method, or visible - reasoning expectations. -4. When the final answer is wrong, inspect the student's process carefully and - award the appropriate process credit from the frozen rubric. -5. Preserve the frozen point increment; if the rubric is unclear, ask the - teacher before introducing a new increment. -6. If handwriting, page order, or missing work affects the score, add an - explicit flag instead of hiding the uncertainty in the numeric score. +For a new production batch, use `submissions//` directories. +If discovery reports a grouping, roster, missing-scan, or solution ambiguity, +stop and resolve the source data with the TA. Do not silently guess a student +identity or page grouping. The legacy filename-prefix mode remains only for +older workflows without a roster. +### Step 2 - Freeze the course package -### Step 3 — Grade students one at a time +Read `/references/grading-prompt.md`, inspect the assessment, and +confirm the current `course-package.json` with the course owner. It must cover +the leaf hierarchy, point values, increments, visible criteria, alternatives, +partial-credit policy, missing/illegible-work policy, and annotation guidance. +If any needed scoring rule is absent, stop and ask; do not infer it from a +solution format or a previous course. -For each student (in alphabetical order unless the user specifies otherwise): -1. Convert each of that student's files to images: +### Step 3 - Render, grade, write, and mark - ```bash - python /scripts/to_images.py "" /tmp/grade-homework// - ``` +For each submission group, render all scans before scoring: - If any file returns exit code 3 (`docx_unsupported`), include a `needs_manual_review` flag for that student and skip that file — do not block the whole run. - -2. Verify page ordering and question-to-page coverage before reading answers. - Missing, duplicated, rotated, or unreadable pages require an explicit flag. - -3. Use an evidence-first pass. For every question, record the visible equation, - statement, diagram feature, answer text, or blank-answer marker before - assigning points. **Do not guess** missing work or silently repair a - student's reasoning. - -4. Score only against the frozen rubric. Validate each score against its range - and allowed increment, then recompute section and assignment totals. - Attach `high`, `medium`, or `low` confidence plus explicit ambiguity flags. - -5. Run a **second-pass** review for every low-confidence item, unreadable region, - blank or apparently missing answer, total mismatch, and high-impact - deduction. Also check missed semantic equivalents, missed keyword credit, - duplicate credit, keyword misuse, score-band consistency, material-error - caps, local contradictions, indirect constructions, open-ended adequacy, - official-style adequacy, and arithmetic. - The second pass must revisit the source image and evidence, not merely repeat - the first score. +```bash +python /scripts/render_submission.py \ + submissions/ rendered/ \ + --submission-id +``` -6. Produce the JSON record only after those checks pass. - `extracted_evidence` and `evidence` must be plain text strings. Do not output - arrays or objects for these fields. If you use `key_term_evidence`, - `concept_evidence`, or `relation_evidence` internally, summarize those layers - inside the single `extracted_evidence` string or the single `evidence` - string. Every non-full leaf also needs the four-field `deduction_trace` - contract above; full-credit leaves may omit it. +Read the whole rendered page set together. Score only the frozen course leaves, +record visible evidence, attach confidence and flags, and return the JSON +contract in `grading-prompt.md`. Every non-full leaf needs the four-field +deduction trace; every review-needed leaf needs an attention note. -7. Pipe the record into `write_outputs.py`: +Write one private record at a time: - ```bash - echo "$RECORD_JSON" | python /scripts/write_outputs.py "$PRIVATE_GRADES_DIR" - ``` +```bash +echo "$RECORD_JSON" | python /scripts/write_outputs.py grades \ + --roster roster.csv \ + --course-package course-package.json \ + --require-annotations +``` - Set `PRIVATE_GRADES_DIR` to a location outside the repository or to an - already ignored private directory. Never use an unignored project folder for - this command. +Then render the marked pages: -8. After every 3 students, briefly summarize progress to the user so they can course-correct early. +```bash +python /scripts/annotate_submission.py \ + rendered//pages.json \ + grades/annotations/.json \ + grades/marked/ +``` -### Step 4 — Recovery +The writer validates the scores and creates `grades.csv` plus `review.csv`. +The annotation renderer fails closed for an invalid page, box, or label; never +fabricate a marking location. -If `grades/grades.csv` already exists at the start of a run, `write_outputs.py` -will skip any student already present. Preserve immutable benchmark records: -never overwrite a benchmark run, prompt, rubric, or prediction file. For an -ordinary re-grade, archive the prior row and feedback before creating a clearly -identified replacement; do not silently delete grading history. +With `--require-annotations`, every score-bearing leaf needs a `praise` box, +every non-full leaf needs a `deduction` box, and every review-needed leaf needs +a `review` box. A partially correct leaf can therefore need both praise and +deduction boxes. If a real location cannot be established, do not invent one: +flag it for teacher review before release. -### Step 5 — Handoff +### Step 4 - Recovery and release -When all students are graded, list: +The writer skips a `student_id` already present in the same grades CSV and +rejects a header change, preventing a silent mid-batch rubric mix. For a +re-grade, create a new private run directory; do not overwrite an existing +marked submission or silently delete prior grading history. -- Any students skipped (with reason). -- The total number of flagged items and which questions they concentrate on — this is what the teacher should spot-check before publishing grades. +Before release, reconcile all `review.csv` rows, inspect marked pages, and +provide the teacher with the private CSV and marked files. Report skipped scan +groups and their reasons. The teacher reviews flagged work and makes the final +score decision. ## Failure modes -- **No solutions file / multiple candidates** → stop, ask user. -- **No `[N pts]` markers** → stop, ask user for allocations. -- **DOCX submission with no conversion toolchain** → `to_images.py` tries `libreoffice`/`soffice` first, then `pandoc + google-chrome/chromium` (extracts WMF/EMF → PNG via `inkscape` if present, converts HTML→PDF via headless Chrome). If neither path works, the student is flagged `needs_manual_review` and grading continues. -- **CSV header mismatch mid-run** (`write_outputs.py` exit 4) → rubric changed; stop and reconcile with user. +- **Missing or ambiguous course material**: stop until the course owner supplies + a complete package and source solution/rubric. +- **Unknown roster group, missing scan, or duplicate grouping**: stop that batch + and resolve the TA's source data; do not infer an identity. +- **DOCX converter unavailable**: put the affected submission in review; do not + claim pages were rendered. +- **Invalid score, trace, or annotation**: correct the private record against the + frozen course package before release. ## Quality bar -This skill uses evidence-first grading plus targeted second-pass review. It is -**not** a substitute for teacher review: spot-check at least 3 students against -your own grading before publishing, and always review flagged items. - -The Physics Week 9 internal benchmark used one run per condition and a single -primary-rater reference. Its transcript-based GPT condition did not outperform -the historical direct baseline overall, so do not claim a general accuracy -improvement. Treat page ordering, frozen rubrics, evidence, confidence, and -second-pass review as auditability safeguards, with extra attention to the -lowest-agreement Physics Week 9 questions; do not generalize those error -patterns beyond this benchmark. +This skill supports a teacher with traceable, private records and visual marking. +It is not a teacher replacement. Course-specific quality claims require that +course's own approved rubric, human review, and validation process. diff --git a/.claude/skills/grade-homework/references/course-package-template.json b/.claude/skills/grade-homework/references/course-package-template.json new file mode 100644 index 0000000..d3ee0ef --- /dev/null +++ b/.claude/skills/grade-homework/references/course-package-template.json @@ -0,0 +1,30 @@ +{ + "schema_version": 1, + "course_id": "replace-with-course-id", + "assessment_id": "replace-with-assessment-id", + "score_leaves": [ + { + "question_id": "replace-with-leaf-id", + "max_score": 1, + "allowed_increment": 1, + "question_type": "replace-with-local-type-or-omit", + "criteria": [ + { + "criterion": "Replace with a visible, checkable requirement.", + "points": 1, + "evidence_required": "Describe observable work or answer evidence." + } + ], + "full_credit_rule": "State the local full-credit condition.", + "accepted_alternatives": [ + "List valid alternate methods or forms, if any." + ], + "partial_credit_rules": [ + "State local score bands and deduction order, including dependent consequences." + ], + "missing_or_unreadable_policy": "State the local review or score policy.", + "annotation_guidance": "State what a deduction, praise, or review box should locate.", + "bonus": false + } + ] +} diff --git a/.claude/skills/grade-homework/references/grading-prompt.md b/.claude/skills/grade-homework/references/grading-prompt.md index 2e7b9f8..aa57725 100644 --- a/.claude/skills/grade-homework/references/grading-prompt.md +++ b/.claude/skills/grade-homework/references/grading-prompt.md @@ -3,114 +3,36 @@ Use this reference after the solutions/rubric pages are available and before grading any student. -## Rubric freeze - -Create a rubric table with one row per question: - -- `question_id` -- `max_score` -- `allowed_increment` -- `expected_evidence` -- `partial_credit_notes` - -Confirm this table with the teacher before grading. Do not change question IDs, -max scores, or increments in the middle of a run. If the rubric is incomplete, -stop and ask. - -## Candidate evidence-first scoring - -For each student and question, write the evidence before the score: - -- visible equation, statement, diagram feature, answer text, or blank marker -- page number or file reference when available -- any uncertainty about handwriting, cropped pages, missing work, or page order - -Score only against the frozen rubric. Do not infer invisible work. Do not give -0.25-point or quarter-point scores. If the final answer is correct and the -process is roughly correct, award full credit. Deduct process points only when a -correct final answer is supported by a process that seriously conflicts with -the standard solution, required method, or visible reasoning expectations. When -the final answer is wrong, inspect the work carefully and award process credit -for correct terms, concepts, formulas, substitutions, units, and reasoning from -the frozen rubric. For calculation problems, arithmetic mistakes should not -erase a correct method unless the frozen rubric requires the exact result. - -Identify the question type before scoring. For each scoring element, record -`key_term_evidence`, `concept_evidence`, and `relation_evidence`, then use -exactly one state: `absent`, `mentioned_only`, `partial_understanding`, -`demonstrated`, or `misused_or_contradicted`. A correctly used keyword can earn -only the rubric's limited `mentioned_only` credit. An unambiguous semantic -equivalent can demonstrate the matching meaning without standard phrasing. -Do not award duplicate credit: a keyword and its explanation are one element, -and overlapping evidence cannot be credited twice. - -Sum the integer scores for non-overlapping elements. Score bands and a -material-error cap are upper bounds only and cannot raise the subtotal. Award -full credit only when all required essential elements are demonstrated, required -terminology is present when explicitly requested, and no material contradiction -invalidates the answer. - -Apply these Candidate v3.1 calibration rules before finalizing the score: - -- cap-locality: apply a material-error cap only when the cap condition is directly visible and active; - do not trigger a cap merely because an element is - partial, under-detailed, or expressed through a non-standard but viable route. -- contradiction-locality: when a misconception or contradiction is local to one - element, proof direction, or construction step, preserve unrelated element credit - unless the frozen rubric explicitly defines a question-level cap. -- key-term semantics: key terms are evidence signals, not mandatory wording - unless the rubric or full-credit rule explicitly requires that terminology. - Correctly used key terms can earn limited keyword credit, and semantic - equivalents should still be mapped to the matching rubric element. -- indirect-construction: score valid indirect constructions by mapping visible - steps to rubric elements and required output behavior. Do not require the - standard direct construction when an indirect route demonstrates the same - result. - -Apply open-ended adequacy for open-ended short-answer, proof, construction, and -essay questions: score whether the answer satisfies the task requirement. Use -the standard answer as an anchor, not as an exhaustive whitelist. Award credit -for valid, relevant, non-contradictory approaches, examples, or constructions -that answer the prompt, even when they are not listed in the expected answer or semantic equivalents. - -Apply official-style adequacy and avoid being overly harsh. Grade for -official-style adequacy, not ideal-answer completeness. Preserve reasonable -partial credit for demonstrated understanding even when terminology, ordering, -or detail is imperfect. Distinguish missing ideal detail from a visible misconception. -Apply large deductions only for material errors, contradictions, -wrong language/output behavior, or missing required answer behavior. - -This is a cross-course prompt contract. Course-specific calibration overlays -must live in the frozen course rubric and packet; do not import named-question -rules, named examples, or specialized subject policies from another course. - -Classify each question before scoring, then record the type: - -- `objective_selection` (multiple choice, matching, true/false): require a - selected option or unambiguous equivalent. Explanations are not required - unless the prompt explicitly requests proof, explanation, justification, or - visible work. -- `calculation`: evaluate result, valid setup/method, transformations or - substitutions, intermediate calculation, and required reasoning. Retain - evidenced method credit when the result is wrong. A correct result without - required work receives only the frozen answer-only allocation. -- `calculation_short_answer`: score derivation and requested short conclusion - as non-overlapping elements; accept valid alternative methods. -- `short_answer` or `conceptual`: score key-term, concept, and relation - evidence without demanding exact reference wording. -- `algorithm` or `construction`: score a viable method, relevant steps, and - required output behavior; accept valid alternatives. -- `proof` or `explanation`: score each required logical link/direction; preserve - independently demonstrated parts when another part is incomplete. -- `diagram`, `geometry`, or `representation`: score visible required objects, - relations, labels, transformations, and conclusion; never infer invisible - diagram work. -- `essay` or `open_response`: score distinct valid, relevant, non-contradictory - claims; do not require fixed order or standard phrasing. - -For mixed questions, use non-overlapping rubric elements for each required -aspect. The frozen rubric, rather than this prompt, sets point values, allowed -increments, and answer-only credit. +## Course-package freeze + +The current course owner supplies `course-package.json`. It must contain one +row per independently scoreable leaf with `question_id`, `max_score`, +`allowed_increment`, visible criteria, accepted alternatives, and any +course-specific deduction or missing-work policy. Confirm it before grading and +do not change it in the middle of a batch. + +## Evidence-first scoring + +Read the complete submission before scoring. Record concise visible evidence +before assigning each score, including any uncertainty about handwriting, +cropped pages, missing work, or page order. Do not infer invisible work. + +Score only the declared leaves in the frozen course package. The course package +sets the question type, score increments, required evidence, acceptable +alternatives, and all partial-credit or answer-only policy. Do not invent a +universal point rule from another assessment. + +Do not create subparts, transfer points between leaves, or count one fact +twice. Accept an unambiguous valid alternative when it meets a declared +criterion. Ignore extra work unrelated to all declared criteria unless it is +adopted for the graded conclusion or the course package says otherwise. + +The live skill intentionally has no named-course calibration overlays. Every +course-specific scoring detail belongs in the current course package, which is +frozen for the batch and reviewed by the course owner. + +A course package may use question-type labels as local routing aids. Its +declared criteria, leaves, and policies always control the grade. ## Submission-level assembly @@ -158,44 +80,76 @@ entry must contain exactly these fields: - `deduction_type` - `points_deducted` -The trace must be grounded in visible work and the frozen rubric, and its -`points_deducted` values must sum exactly to `max_score - score` for that one -leaf. It is a compact audit statement, not a chain of thought. Deduct from the -first material error; do not deduct again for consequences of that same error. -For selected-response work, do not penalize a missing explanation unless the -question explicitly requires it. When a correct calculation answer lacks -required work, use the frozen answer-only cap. A zero score needs an explicit -missing or incorrect reason. Full-credit leaves omit `deduction_trace`; any -leaf with flags or `low` confidence needs a short `attention_note`. Evaluate a -bonus leaf independently from every base leaf. +The trace must be grounded in visible work and the frozen course package. Its +`points_deducted` values must sum exactly to `max_score - score` for that leaf. +It is a compact audit statement, not a chain of thought. Apply the current +course package's deduction order and no-double-count policy. A zero score needs +a specific visible missing or incorrect reason. Full-credit leaves omit +`deduction_trace`; every flagged, medium-confidence, or low-confidence leaf +needs a short `attention_note` for human review. Never place a name, identifier, email address, private path, or raw private file reference in a trace or attention note. +## Marked-page annotations + +For each visible location that supports a deduction, praise, or review flag, +emit one annotation with exactly these fields: + +- `question_id`: a declared score leaf +- `page_id`: an ID from the private rendered `pages.json` +- `box`: `[x, y, width, height]` normalized to `[0, 1]` +- `kind`: `deduction`, `praise`, or `review` +- `label`: a short learner-facing note without personal data + +For a production record written with `--require-annotations`, add praise for +every leaf awarded more than zero, a deduction annotation for every non-full +leaf, and a review annotation for every flagged or non-high-confidence leaf. +A partially correct leaf can therefore need both praise and deduction boxes. +Do not invent a box: a genuinely uncertain location must be flagged for review +instead. + ## Required JSON record Write one JSON object per student before passing it to `write_outputs.py`: ```json { - "student_id": "anonymous_or_filename_student_id", + "student_id": "opaque-submission-id", "scores": [ { - "question_id": "Q1", + "question_id": "leaf-id", "score": 2.0, "max_score": 3.0, "evidence": "Visible work used to justify the score.", "feedback": "Short English feedback for the student.", - "confidence": "high", + "confidence": "medium", "flags": [], "deduction_trace": [ { - "rubric_criterion": "frozen rubric criterion label", + "rubric_criterion": "course-package criterion label", "observed_evidence_or_missing_or_incorrect_part": "Concise visible missing or incorrect part.", "deduction_type": "material_method_error", "points_deducted": 1.0 } - ] + ], + "attention_note": "Short reason for human review." + } + ], + "annotations": [ + { + "question_id": "leaf-id", + "page_id": "private-page-id", + "box": [0.1, 0.1, 0.2, 0.1], + "kind": "deduction", + "label": "Short marking note." + }, + { + "question_id": "leaf-id", + "page_id": "private-page-id", + "box": [0.1, 0.1, 0.2, 0.1], + "kind": "review", + "label": "Please verify this region." } ], "total": 2.0, @@ -206,47 +160,18 @@ Write one JSON object per student before passing it to `write_outputs.py`: Confidence must be `high`, `medium`, or `low`. Use flags such as `unreadable_region`, `missing_page`, `blank_answer`, `page_order_uncertain`, `rubric_ambiguous`, `high_impact_deduction`, or `needs_manual_review`. -`extracted_evidence` and `evidence` must be plain text strings. Do not output -arrays or objects for these fields. If you use `key_term_evidence`, -`concept_evidence`, or `relation_evidence` internally, summarize those layers -inside the single `extracted_evidence` string or the single `evidence` string. +`evidence`, `feedback`, traces, attention notes, and annotation labels must be +short plain-text fields. Do not output names, student numbers, raw paths, or +private filenames in the JSON record. ## Second-pass triggers -Before writing output, revisit the source page for every item with: - -- `low` confidence -- unreadable or cropped work -- blank or apparently missing answers -- high-impact deductions -- total mismatches -- any score that depends on interpreting handwriting - -Also check missed semantic equivalents, missed keyword credit, duplicate credit, -keyword misuse, score-band consistency, score increments, material-error caps, -local contradictions, indirect constructions, open-ended adequacy, -official-style adequacy, and arithmetic. The -`confidence` field must be exactly `high`, `medium`, or `low`, and the exact -total must be recomputed from itemized scores. - -If the second pass still leaves uncertainty, keep the numeric score conservative -and flag the item for teacher review. - -## Route-comparison evidence card - -When comparing a direct multimodal route with a transcription-assisted route, -use the same frozen rubric, gold, split, scoring packet, and review policy. -For every representative disagreement, write an evidence card before changing -anything. Set one primary category: - -- `clear_model_error`: source evidence and frozen rubric support another score. -- `representation_loss`: relevant source evidence was lost, mistranscribed, - reordered, cropped, or otherwise unavailable to a route. -- `rubric_or_gold_conflict`: a course-owner decision is needed. -- `reasonable_severity_difference`: both evidence-grounded scores lie within an - acceptable strictness range. -- `insufficient_evidence`: the source or record cannot support a reliable - decision. - -Do not label disagreement a model failure by default. Keep the card alongside -the route artifacts and final human disposition. +Before writing output, revisit the source page for any non-full, flagged, +medium-confidence, or low-confidence leaf; for unreadable, cropped, blank, or +apparently missing work; and for any total mismatch. Verify score increments, +leaf coverage, deduction-trace arithmetic, and annotation locations against +the frozen course package. + +If uncertainty remains, preserve it in `attention_note`, `review.csv`, and a +`review` annotation when a real page location is known. The teacher decides the +final resolution; do not disguise uncertainty as a confident score. diff --git a/.claude/skills/grade-homework/scripts/annotate_submission.py b/.claude/skills/grade-homework/scripts/annotate_submission.py new file mode 100644 index 0000000..87714cc --- /dev/null +++ b/.claude/skills/grade-homework/scripts/annotate_submission.py @@ -0,0 +1,221 @@ +from __future__ import annotations + +import argparse +import json +import re +import sys +from collections import defaultdict +from pathlib import Path +from typing import Any + +from PIL import Image, ImageDraw, ImageFont + +from roster import RosterError, SAFE_SUBMISSION_ID, require_private_path + + +ANNOTATION_KINDS = {"deduction", "praise", "review"} +COLORS = { + "deduction": (196, 46, 46), + "praise": (35, 130, 72), + "review": (206, 129, 25), +} +WINDOWS_ABSOLUTE_PATH = re.compile(r"(?:^|\s)[A-Za-z]:[\\/]") +PRIVATE_DATA_PATH = re.compile(r"(?:^|[\\/])Data[\\/]", re.IGNORECASE) +FILE_URI = re.compile(r"\bfile://", re.IGNORECASE) +EMAIL_ADDRESS = re.compile(r"\b[^\s@]+@[^\s@]+\.[^\s@]+\b") +IDENTITY_LABEL = re.compile( + "\\b(?:student[ _-]?(?:id|number|name)|name)\\s*[:=]|(?:\\u59d3\\u540d|\\u5b66\\u53f7)\\s*[:\\uff1a]", + re.IGNORECASE, +) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Render validated praise, deduction, and review annotations." + ) + parser.add_argument("pages_manifest", type=Path) + parser.add_argument("annotations_json", type=Path) + parser.add_argument("output_dir", type=Path) + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + output_dir = namespace.output_dir + if output_dir.exists() and any(output_dir.iterdir()): + print("refusing to overwrite an existing marked submission", file=sys.stderr) + return 4 + try: + require_private_path(output_dir, label="marked submission output") + manifest = _load_object(namespace.pages_manifest, "pages manifest") + annotations_record = _load_object(namespace.annotations_json, "annotation record") + pages, submission_id = _validated_pages(manifest, namespace.pages_manifest.parent) + annotations = _validated_annotations(annotations_record, submission_id, set(pages)) + except (OSError, ValueError, RosterError) as error: + print(f"invalid annotation input: {error}", file=sys.stderr) + return 2 + + by_page: dict[str, list[dict[str, Any]]] = defaultdict(list) + for annotation in annotations: + by_page[annotation["page_id"]].append(annotation) + + output_dir.mkdir(parents=True, exist_ok=True) + font = ImageFont.load_default() + rendered_images: list[Image.Image] = [] + try: + for page_id, page_path in pages.items(): + with Image.open(page_path) as source: + image = source.convert("RGB") + _draw_annotations(image, by_page[page_id], font) + target = output_dir / page_path.name + image.save(target) + rendered_images.append(image) + pdf_path = output_dir / "marked.pdf" + rendered_images[0].save( + pdf_path, + save_all=True, + append_images=rendered_images[1:], + resolution=150.0, + ) + except Exception as error: + print(f"annotation rendering failed: {error}", file=sys.stderr) + return 2 + finally: + for image in rendered_images: + image.close() + + print( + json.dumps( + { + "status": "ok", + "submission_id": submission_id, + "page_count": len(pages), + "marked_pdf": "marked.pdf", + }, + sort_keys=True, + ) + ) + return 0 + + +def _load_object(path: Path, label: str) -> dict[str, Any]: + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, json.JSONDecodeError) as error: + raise ValueError(f"{label} could not be read") from error + if not isinstance(payload, dict): + raise ValueError(f"{label} must be a JSON object") + return payload + + +def _validated_pages( + manifest: dict[str, Any], manifest_dir: Path +) -> tuple[dict[str, Path], str]: + submission_id = str(manifest.get("submission_id", "")).strip() + if not SAFE_SUBMISSION_ID.fullmatch(submission_id): + raise ValueError("pages manifest submission_id is invalid") + raw_pages = manifest.get("pages") + if not isinstance(raw_pages, list) or not raw_pages: + raise ValueError("pages manifest requires non-empty pages") + pages: dict[str, Path] = {} + for page in raw_pages: + if not isinstance(page, dict): + raise ValueError("pages manifest page must be an object") + page_id = str(page.get("page_id", "")).strip() + raw_path = str(page.get("path", "")).strip() + if not SAFE_SUBMISSION_ID.fullmatch(page_id): + raise ValueError("pages manifest page_id is invalid") + if not raw_path or Path(raw_path).name != raw_path: + raise ValueError("pages manifest page path must be a filename") + source = manifest_dir / raw_path + if page_id in pages or not source.is_file(): + raise ValueError("pages manifest has duplicate or missing page") + pages[page_id] = source + return pages, submission_id + + +def _validated_annotations( + record: dict[str, Any], submission_id: str, page_ids: set[str] +) -> list[dict[str, Any]]: + if str(record.get("student_id", "")).strip() != submission_id: + raise ValueError("annotation record does not match the pages manifest") + raw_annotations = record.get("annotations") + if not isinstance(raw_annotations, list): + raise ValueError("annotation record requires annotations") + required = {"question_id", "page_id", "box", "kind", "label"} + result = [] + for annotation in raw_annotations: + if not isinstance(annotation, dict) or set(annotation) != required: + raise ValueError("annotation fields are invalid") + question_id = str(annotation["question_id"]).strip() + if not question_id: + raise ValueError("annotation question_id is invalid") + page_id = str(annotation["page_id"]).strip() + if page_id not in page_ids: + raise ValueError("annotation references an unknown page") + kind = annotation["kind"] + if kind not in ANNOTATION_KINDS: + raise ValueError("annotation kind is invalid") + box = annotation["box"] + if ( + not isinstance(box, list) + or len(box) != 4 + or any(isinstance(part, bool) or not isinstance(part, (int, float)) for part in box) + ): + raise ValueError("annotation box is invalid") + x, y, width, height = (float(part) for part in box) + if x < 0 or y < 0 or width <= 0 or height <= 0 or x + width > 1 or y + height > 1: + raise ValueError("annotation box is outside the page") + result.append( + { + "question_id": question_id, + "page_id": page_id, + "box": [x, y, width, height], + "kind": kind, + "label": _safe_label(annotation["label"], submission_id), + } + ) + return result + + +def _safe_label(value: Any, submission_id: str) -> str: + if not isinstance(value, str) or not value.strip() or len(value) > 500: + raise ValueError("annotation label is invalid") + if ( + WINDOWS_ABSOLUTE_PATH.search(value) + or PRIVATE_DATA_PATH.search(value) + or FILE_URI.search(value) + or EMAIL_ADDRESS.search(value) + or IDENTITY_LABEL.search(value) + or submission_id.casefold() in value.casefold() + ): + raise ValueError("annotation label contains private information") + return value.strip() + + +def _draw_annotations( + image: Image.Image, annotations: list[dict[str, Any]], font: ImageFont.ImageFont +) -> None: + draw = ImageDraw.Draw(image) + width, height = image.size + stroke = max(2, min(width, height) // 400) + for annotation in annotations: + x, y, box_width, box_height = annotation["box"] + left = round(x * width) + top = round(y * height) + right = round((x + box_width) * width) + bottom = round((y + box_height) * height) + color = COLORS[annotation["kind"]] + draw.rectangle((left, top, right, bottom), outline=color, width=stroke) + label = annotation["label"] + text_bbox = draw.textbbox((0, 0), label, font=font) + text_width = text_bbox[2] - text_bbox[0] + 6 + text_height = text_bbox[3] - text_bbox[1] + 4 + label_left = min(max(0, left), max(0, width - text_width)) + label_top = top - text_height if top >= text_height else min(height - text_height, bottom) + draw.rectangle( + (label_left, label_top, label_left + text_width, label_top + text_height), + fill=color, + ) + draw.text((label_left + 3, label_top + 2), label, fill=(255, 255, 255), font=font) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.claude/skills/grade-homework/scripts/discover.py b/.claude/skills/grade-homework/scripts/discover.py index 2489d7d..566ab8a 100644 --- a/.claude/skills/grade-homework/scripts/discover.py +++ b/.claude/skills/grade-homework/scripts/discover.py @@ -1,58 +1,116 @@ from __future__ import annotations +import argparse import json +import re import sys from collections import Counter from pathlib import Path +from roster import RosterError, load_roster -SUPPORTED_SUFFIXES = {".pdf", ".png", ".jpg", ".jpeg", ".docx"} + +SUPPORTED_SUFFIXES = { + ".pdf", + ".png", + ".jpg", + ".jpeg", + ".tif", + ".tiff", + ".webp", + ".heic", + ".docx", +} SOLUTION_MARKERS = ("solution", "solutions", "answer", "answers", "rubric", "key") -SKIP_DIRS = {"grades", "__pycache__"} +SKIP_DIRS = {"grades", "rendered", "marked", "annotations", "__pycache__"} def main(argv: list[str] | None = None) -> int: - args = list(sys.argv[1:] if argv is None else argv) - root = Path(args[0]) if args else Path.cwd() + parser = argparse.ArgumentParser(description="Discover a private grading batch.") + parser.add_argument("root", nargs="?", default=".") + parser.add_argument("--roster", type=Path) + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + root = Path(namespace.root) if not root.is_dir(): - print(json.dumps({"solutions_error": f"not a directory: {root}"})) + print(json.dumps({"status": "error", "solutions_error": "not a directory"})) return 2 files = list(_iter_files(root)) candidates = [path for path in files if _is_supported(path)] - solutions = [path for path in candidates if _looks_like_solution(path)] - submissions = [path for path in candidates if path not in set(solutions)] - late_students = sorted( - { - _student_id(path) - for path in submissions - if "_late_" in path.name.lower() - } + submissions_root = root / "submissions" + grouped_mode = submissions_root.is_dir() + course_candidates = [ + path + for path in candidates + if not grouped_mode or not _is_under(path, submissions_root) + ] + solutions = [path for path in course_candidates if _looks_like_solution(path)] + solutions_error = _solutions_error(solutions) + submission_files = [ + path + for path in candidates + if path not in set(solutions) and (not grouped_mode or _is_under(path, submissions_root)) + ] + grouped, ungrouped_count = _group_submission_files( + root=root, submissions_root=submissions_root, files=submission_files, grouped_mode=grouped_mode ) - solutions_error = None - if not solutions: - solutions_error = "no solutions or rubric file found" - elif len(solutions) > 1: - solutions_error = "multiple solutions or rubric candidates found" + grouping_errors: list[str] = [] + if ungrouped_count: + grouping_errors.append("submission_file_without_submission_id") + roster_used = namespace.roster is not None + roster_error = None + if namespace.roster is not None: + try: + roster = load_roster(namespace.roster) + except RosterError as error: + roster = {} + roster_error = str(error) + if roster_error is None: + unknown = set(grouped) - set(roster) + missing = set(roster) - set(grouped) + if unknown: + grouping_errors.append("scan_group_not_in_roster") + if missing: + grouping_errors.append("roster_entry_without_scan_group") + + submissions = [ + { + "student_id": student_id, + "student": student_id, + "files": [ + { + "source_id": f"source-{index:03d}", + "source_order": index, + "suffix": path.suffix.lower(), + } + for index, path in enumerate(paths, start=1) + ], + "late": any("_late_" in path.name.lower() for path in paths), + } + for student_id, paths in sorted(grouped.items()) + ] + late_students = [ + item["student_id"] for item in submissions if item["late"] + ] payload = { - "root": root.resolve().as_posix(), + "status": "ok" if not solutions_error and not roster_error and not grouping_errors else "review_required", + # A batch manifest must remain portable and must not reveal a local path. + "root": ".", "solutions_error": solutions_error, "solutions_candidates": [_rel(root, path) for path in sorted(solutions)], - "submissions": [ - { - "student": _student_id(path), - "path": _rel(root, path), - "suffix": path.suffix.lower(), - "late": "_late_" in path.name.lower(), - } - for path in sorted(submissions, key=lambda path: (_student_id(path), _rel(root, path))) - ], + "submissions": submissions, "late_students": late_students, - "extension_counts": dict(sorted(Counter(path.suffix.lower() for path in submissions).items())), + "extension_counts": dict( + sorted(Counter(path.suffix.lower() for path in submission_files).items()) + ), + "grouping_mode": "submission_directories" if grouped_mode else "legacy_filename_prefix", + "roster_used": roster_used, + "grouping_errors": grouping_errors, + "roster_error": roster_error, } print(json.dumps(payload, indent=2, sort_keys=True)) - return 0 + return 0 if payload["status"] == "ok" else 3 def _iter_files(root: Path) -> list[Path]: @@ -74,6 +132,51 @@ def _looks_like_solution(path: Path) -> bool: return any(marker in name for marker in SOLUTION_MARKERS) +def _solutions_error(solutions: list[Path]) -> str | None: + if not solutions: + return "no solutions or rubric file found" + if len(solutions) > 1: + return "multiple solutions or rubric candidates found" + return None + + +def _group_submission_files( + *, + root: Path, + submissions_root: Path, + files: list[Path], + grouped_mode: bool, +) -> tuple[dict[str, list[Path]], int]: + grouped: dict[str, list[Path]] = {} + ungrouped_count = 0 + for path in files: + if grouped_mode: + relative = path.relative_to(submissions_root) + if len(relative.parts) < 2: + ungrouped_count += 1 + continue + student_id = relative.parts[0] + else: + student_id = _student_id(path) + grouped.setdefault(student_id, []).append(path) + for paths in grouped.values(): + paths.sort(key=lambda path: _natural_path_key(_rel(root, path))) + return grouped, ungrouped_count + + +def _is_under(path: Path, parent: Path) -> bool: + try: + path.relative_to(parent) + except ValueError: + return False + return True + + +def _natural_path_key(value: str) -> tuple[object, ...]: + parts = re.split(r"(\d+)", value.casefold()) + return tuple(int(part) if part.isdigit() else part for part in parts) + + def _student_id(path: Path) -> str: stem = path.stem return stem.split("_", 1)[0] if "_" in stem else stem diff --git a/.claude/skills/grade-homework/scripts/render_submission.py b/.claude/skills/grade-homework/scripts/render_submission.py new file mode 100644 index 0000000..646ce81 --- /dev/null +++ b/.claude/skills/grade-homework/scripts/render_submission.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +from roster import RosterError, SAFE_SUBMISSION_ID, require_private_path +import to_images + + +SUPPORTED_SUFFIXES = to_images.IMAGE_SUFFIXES | {".pdf", ".docx"} + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Render one complete private submission without page overwrites." + ) + parser.add_argument("submission_dir", type=Path) + parser.add_argument("output_dir", type=Path) + parser.add_argument("--submission-id") + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + source_dir = namespace.submission_dir + output_dir = namespace.output_dir + submission_id = (namespace.submission_id or source_dir.name).strip() + if not SAFE_SUBMISSION_ID.fullmatch(submission_id): + print("invalid submission_id", file=sys.stderr) + return 2 + if not source_dir.is_dir(): + print("submission directory is missing", file=sys.stderr) + return 2 + if output_dir.exists() and any(output_dir.iterdir()): + print("refusing to overwrite an existing rendered submission", file=sys.stderr) + return 4 + try: + require_private_path(source_dir, label="submission directory") + require_private_path(output_dir, label="rendered submission output") + except RosterError as error: + print(f"private-output check failed: {error}", file=sys.stderr) + return 2 + + sources = _source_files(source_dir) + if not sources: + print("submission directory contains no supported scan files", file=sys.stderr) + return 2 + + output_dir.mkdir(parents=True, exist_ok=True) + pages: list[dict[str, object]] = [] + page_order = 0 + try: + for source_order, source in enumerate(sources, start=1): + prefix = f"source-{source_order:03d}" + rendered = to_images.render_file(source, output_dir, prefix=prefix) + for source_page_order, page in enumerate(rendered, start=1): + page_order += 1 + pages.append( + { + "page_id": page.stem, + "path": page.name, + "source_id": f"source-{source_order:03d}", + "source_order": source_order, + "source_page_order": source_page_order, + "page_order": page_order, + } + ) + except Exception as error: + print(f"render failed: {error}", file=sys.stderr) + return 2 + + manifest = { + "schema_version": 1, + "submission_id": submission_id, + "page_count": len(pages), + "pages": pages, + } + manifest_path = output_dir / "pages.json" + manifest_path.write_text( + json.dumps(manifest, ensure_ascii=False, indent=2) + "\n", + encoding="utf-8", + newline="\n", + ) + print( + json.dumps( + { + "status": "ok", + "submission_id": submission_id, + "page_count": len(pages), + "manifest": manifest_path.name, + }, + sort_keys=True, + ) + ) + return 0 + + +def _source_files(source_dir: Path) -> list[Path]: + files = [] + for path in source_dir.rglob("*"): + if not path.is_file(): + continue + relative = path.relative_to(source_dir) + if any(part.startswith(".") or part == "__pycache__" for part in relative.parts): + continue + if path.suffix.lower() in SUPPORTED_SUFFIXES: + files.append(path) + return sorted(files, key=lambda path: _natural_key(path.relative_to(source_dir).as_posix())) + + +def _natural_key(value: str) -> tuple[object, ...]: + return tuple( + int(part) if part.isdigit() else part + for part in re.split(r"(\d+)", value.casefold()) + ) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.claude/skills/grade-homework/scripts/roster.py b/.claude/skills/grade-homework/scripts/roster.py new file mode 100644 index 0000000..14c2edc --- /dev/null +++ b/.claude/skills/grade-homework/scripts/roster.py @@ -0,0 +1,143 @@ +from __future__ import annotations + +import csv +import argparse +import json +import re +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path + + +REQUIRED_COLUMNS = ("submission_id", "student_name", "student_number") +SAFE_SUBMISSION_ID = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") + + +class RosterError(ValueError): + """A private roster does not meet the local delivery contract.""" + + +@dataclass(frozen=True) +class RosterEntry: + submission_id: str + student_name: str + student_number: str + + +def load_roster(path: Path) -> dict[str, RosterEntry]: + """Load a private roster without exposing its contents in diagnostics.""" + + if not path.is_file(): + raise RosterError("roster file is missing") + require_private_path(path, label="roster file") + + try: + with path.open("r", encoding="utf-8-sig", newline="") as handle: + reader = csv.DictReader(handle) + fieldnames = tuple(reader.fieldnames or ()) + if set(fieldnames) != set(REQUIRED_COLUMNS) or len(fieldnames) != len( + REQUIRED_COLUMNS + ): + raise RosterError( + "roster columns must be exactly submission_id,student_name,student_number" + ) + rows = list(reader) + except (OSError, UnicodeError, csv.Error) as error: + raise RosterError("roster could not be read") from error + + if not rows: + raise RosterError("roster must contain at least one student") + + entries: dict[str, RosterEntry] = {} + seen_numbers: set[str] = set() + for row in rows: + submission_id = _required_cell(row, "submission_id") + student_name = _required_cell(row, "student_name") + student_number = _required_cell(row, "student_number") + if not SAFE_SUBMISSION_ID.fullmatch(submission_id): + raise RosterError("roster submission_id contains unsupported characters") + if len(student_name) > 256 or len(student_number) > 128: + raise RosterError("roster field exceeds its supported length") + if submission_id in entries: + raise RosterError("roster contains duplicate submission_id") + if student_number in seen_numbers: + raise RosterError("roster contains duplicate student_number") + entries[submission_id] = RosterEntry( + submission_id=submission_id, + student_name=student_name, + student_number=student_number, + ) + seen_numbers.add(student_number) + return entries + + +def require_private_path(path: Path, *, label: str) -> None: + """Reject private input or output under a tracked Git location.""" + + resolved = path.resolve() + repository_root = next( + ( + parent + for parent in (resolved, *resolved.parents) + if (parent / ".git").exists() + ), + None, + ) + if repository_root is None: + return + try: + relative = resolved.relative_to(repository_root).as_posix() + except ValueError: + return + check = subprocess.run( + [ + "git", + "-c", + f"safe.directory={repository_root.as_posix()}", + "-C", + str(repository_root), + "check-ignore", + "--quiet", + "--no-index", + "--", + relative, + ], + check=False, + capture_output=True, + text=True, + ) + if check.returncode == 0: + return + if check.returncode == 1: + raise RosterError(f"{label} inside a Git worktree must be private and ignored") + raise RosterError(f"could not verify whether {label} is ignored") + + +def _required_cell(row: dict[str, str | None], key: str) -> str: + value = row.get(key) + if value is None: + raise RosterError("roster row is missing a required value") + normalized = value.strip() + if not normalized: + raise RosterError("roster row contains a blank required value") + return normalized + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Validate a private roster without printing its contents." + ) + parser.add_argument("private_roster", type=Path) + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + try: + roster = load_roster(namespace.private_roster) + except RosterError as error: + print(f"invalid roster: {error}", file=sys.stderr) + return 2 + print(json.dumps({"status": "ok", "student_count": len(roster)}, sort_keys=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.claude/skills/grade-homework/scripts/to_images.py b/.claude/skills/grade-homework/scripts/to_images.py index 098046b..a2e399e 100644 --- a/.claude/skills/grade-homework/scripts/to_images.py +++ b/.claude/skills/grade-homework/scripts/to_images.py @@ -1,6 +1,8 @@ from __future__ import annotations +import argparse import json +import re import shutil import subprocess import sys @@ -9,34 +11,68 @@ from PIL import Image, ImageOps +from roster import RosterError, require_private_path -IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg"} +IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg", ".tif", ".tiff", ".webp", ".heic"} + + +def _validated_prefix(value: str) -> str: + normalized = value.strip() + if not normalized: + return "" + if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._-]{0,80}", normalized): + raise ValueError("prefix must contain only letters, digits, dot, underscore, or dash") + return normalized + + +def _page_target(output_dir: Path, prefix: str, index: int) -> Path: + stem = f"{prefix}-page" if prefix else "page" + return output_dir / f"{stem}-{index:03d}.png" + + +def _ensure_new_target(target: Path) -> None: + if target.exists(): + raise FileExistsError(f"refusing to overwrite rendered page: {target.name}") -def main(argv: list[str] | None = None) -> int: - args = list(sys.argv[1:] if argv is None else argv) - if len(args) != 2: - print(json.dumps({"status": "error", "error": "usage: to_images.py "})) - return 2 - source = Path(args[0]) - output_dir = Path(args[1]) - output_dir.mkdir(parents=True, exist_ok=True) +def render_file(source: Path, output_dir: Path, *, prefix: str = "") -> list[Path]: + """Render one supported submission source without overwriting pages.""" + + suffix = source.suffix.lower() + if suffix in IMAGE_SUFFIXES: + return [_convert_image(source, output_dir, prefix=prefix)] + if suffix == ".pdf": + return _convert_pdf(source, output_dir, prefix=prefix) + if suffix == ".docx": + return _convert_docx(source, output_dir, prefix=prefix) + raise ValueError(f"unsupported file type: {suffix}") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Render one source file to PNG pages.") + parser.add_argument("input") + parser.add_argument("output_dir") + parser.add_argument("--prefix", default="") + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + source = Path(namespace.input) + output_dir = Path(namespace.output_dir) + try: + prefix = _validated_prefix(namespace.prefix) + except ValueError as error: + parser.error(str(error)) if not source.is_file(): - print(json.dumps({"status": "error", "error": f"missing file: {source}"})) + print(json.dumps({"status": "error", "error": "missing_input_file"})) return 2 - suffix = source.suffix.lower() try: - if suffix in IMAGE_SUFFIXES: - pages = [_convert_image(source, output_dir)] - return _ok(source, pages) - if suffix == ".pdf": - pages = _convert_pdf(source, output_dir) - return _ok(source, pages) - if suffix == ".docx": - pages = _convert_docx(source, output_dir) - return _ok(source, pages) + require_private_path(output_dir, label="rendered-page output") + output_dir.mkdir(parents=True, exist_ok=True) + pages = render_file(source, output_dir, prefix=prefix) + return _ok(source, pages) + except RosterError as error: + return _error("private_output_required", str(error), code=2) except MissingPdfRenderer as error: return _error("pdf_renderer_missing", str(error), code=2) except MissingDocxConverter as error: @@ -44,17 +80,17 @@ def main(argv: list[str] | None = None) -> int: except Exception as error: return _error("conversion_failed", str(error), code=2) - return _error("unsupported_file_type", suffix, code=2) -def _convert_image(source: Path, output_dir: Path) -> Path: - target = output_dir / "page-001.png" +def _convert_image(source: Path, output_dir: Path, *, prefix: str) -> Path: + target = _page_target(output_dir, prefix, 1) + _ensure_new_target(target) with Image.open(source) as image: ImageOps.exif_transpose(image).convert("RGB").save(target) return target -def _convert_pdf(source: Path, output_dir: Path) -> list[Path]: +def _convert_pdf(source: Path, output_dir: Path, *, prefix: str) -> list[Path]: try: import fitz # type: ignore except ImportError as exc: @@ -66,13 +102,14 @@ def _convert_pdf(source: Path, output_dir: Path) -> list[Path]: document = fitz.open(str(source)) for index, page in enumerate(document, start=1): pixmap = page.get_pixmap(matrix=fitz.Matrix(2, 2), alpha=False) - target = output_dir / f"page-{index:03d}.png" + target = _page_target(output_dir, prefix, index) + _ensure_new_target(target) pixmap.save(target) pages.append(target) return pages -def _convert_docx(source: Path, output_dir: Path) -> list[Path]: +def _convert_docx(source: Path, output_dir: Path, *, prefix: str) -> list[Path]: converter = shutil.which("soffice") or shutil.which("libreoffice") if converter is None: raise MissingDocxConverter("LibreOffice or soffice is required for DOCX files.") @@ -103,7 +140,7 @@ def _convert_docx(source: Path, output_dir: Path) -> list[Path]: if not matches: raise MissingDocxConverter("DOCX converter did not produce a PDF.") pdf = matches[0] - return _convert_pdf(pdf, output_dir) + return _convert_pdf(pdf, output_dir, prefix=prefix) def _ok(source: Path, pages: list[Path]) -> int: @@ -111,8 +148,8 @@ def _ok(source: Path, pages: list[Path]) -> int: json.dumps( { "status": "ok", - "source": source.as_posix(), - "pages": [page.as_posix() for page in pages], + "page_count": len(pages), + "pages": [page.name for page in pages], }, sort_keys=True, ) diff --git a/.claude/skills/grade-homework/scripts/write_outputs.py b/.claude/skills/grade-homework/scripts/write_outputs.py index 895d2b6..4fd1dcc 100644 --- a/.claude/skills/grade-homework/scripts/write_outputs.py +++ b/.claude/skills/grade-homework/scripts/write_outputs.py @@ -1,16 +1,36 @@ from __future__ import annotations +import argparse import csv import json import re -import subprocess import sys from pathlib import Path from typing import Any +from roster import ( + RosterEntry, + RosterError, + SAFE_SUBMISSION_ID, + load_roster, + require_private_path, +) + -BASE_COLUMNS = ["student_id"] -TAIL_COLUMNS = ["total", "flags"] +BASE_COLUMNS = ["student_id", "student_name", "student_number"] +TAIL_COLUMNS = ["total", "uncertainties", "flags"] +REVIEW_COLUMNS = [ + "student_id", + "student_name", + "student_number", + "question_id", + "score", + "max_score", + "confidence", + "uncertainty", + "flags", +] +ANNOTATION_KINDS = {"deduction", "praise", "review"} DEDUCTION_TYPES = { "answer_only_cap", "blank_or_missing_answer", @@ -28,32 +48,51 @@ FILE_URI = re.compile(r"\bfile://", re.IGNORECASE) EMAIL_ADDRESS = re.compile(r"\b[^\s@]+@[^\s@]+\.[^\s@]+\b") IDENTITY_LABEL = re.compile( - r"\b(?:student[ _-]?(?:id|number|name)|name)\s*[:=]|(?:姓名|学号)\s*[::]", + "\\b(?:student[ _-]?(?:id|number|name)|name)\\s*[:=]|(?:\\u59d3\\u540d|\\u5b66\\u53f7)\\s*[:\\uff1a]", re.IGNORECASE, ) def main(argv: list[str] | None = None) -> int: - args = list(sys.argv[1:] if argv is None else argv) - if len(args) != 1: - print("usage: write_outputs.py ", file=sys.stderr) - return 2 - - grades_dir = Path(args[0]) + parser = argparse.ArgumentParser(description="Write private grading outputs.") + parser.add_argument("grades_dir", type=Path) + parser.add_argument("--roster", type=Path) + parser.add_argument("--course-package", type=Path) + parser.add_argument("--require-annotations", action="store_true") + namespace = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + grades_dir = namespace.grades_dir try: - _require_private_output_directory(grades_dir) - grades_dir.mkdir(parents=True, exist_ok=True) - feedback_dir = grades_dir / "feedback" - feedback_dir.mkdir(exist_ok=True) + require_private_path(grades_dir, label="grades directory") + roster = load_roster(namespace.roster) if namespace.roster else None + course_leaves = ( + _load_course_package(namespace.course_package) + if namespace.course_package + else None + ) record = json.loads(sys.stdin.read()) - normalized = _normalize_record(record) + normalized = _normalize_record( + record, + course_leaves=course_leaves, + require_annotations=namespace.require_annotations, + ) + roster_entry = _roster_entry(normalized["student_id"], roster) except Exception as error: print(f"invalid record: {error}", file=sys.stderr) return 2 + grades_dir.mkdir(parents=True, exist_ok=True) + feedback_dir = grades_dir / "feedback" + annotation_dir = grades_dir / "annotations" + feedback_dir.mkdir(exist_ok=True) + annotation_dir.mkdir(exist_ok=True) csv_path = grades_dir / "grades.csv" - header = BASE_COLUMNS + [item["question_id"] for item in normalized["scores"]] + TAIL_COLUMNS + question_ids = ( + list(course_leaves) + if course_leaves is not None + else [item["question_id"] for item in normalized["scores"]] + ) + header = BASE_COLUMNS + question_ids + TAIL_COLUMNS existing_rows = _read_existing_rows(csv_path) if existing_rows is not None: existing_header, rows = existing_rows @@ -64,30 +103,47 @@ def main(argv: list[str] | None = None) -> int: print(json.dumps({"status": "skipped", "student_id": normalized["student_id"]})) return 0 + score_by_id = {item["question_id"]: item for item in normalized["scores"]} row = { "student_id": normalized["student_id"], + "student_name": roster_entry.student_name if roster_entry else "", + "student_number": roster_entry.student_number if roster_entry else "", "total": _format_score(normalized["total"]), + "uncertainties": _uncertainty_cell(normalized), "flags": ";".join(normalized["flags"]), } - for item in normalized["scores"]: - row[item["question_id"]] = _format_score(item["score"]) - - write_header = not csv_path.exists() - with csv_path.open("a", encoding="utf-8", newline="") as handle: - writer = csv.DictWriter(handle, fieldnames=header) - if write_header: - writer.writeheader() - writer.writerow(row) + for question_id in question_ids: + row[question_id] = _format_score(score_by_id[question_id]["score"]) + _append_csv(csv_path, header, row) + review_path = grades_dir / "review.csv" + _append_review_rows(review_path, normalized, roster_entry) feedback_path = feedback_dir / f"{_safe_name(normalized['student_id'])}.md" feedback_path.write_text(_feedback_markdown(normalized), encoding="utf-8", newline="\n") + annotation_path = annotation_dir / f"{_safe_name(normalized['student_id'])}.json" + annotation_path.write_text( + json.dumps( + { + "schema_version": 1, + "student_id": normalized["student_id"], + "annotations": normalized["annotations"], + }, + ensure_ascii=False, + indent=2, + ) + + "\n", + encoding="utf-8", + newline="\n", + ) print( json.dumps( { "status": "written", "student_id": normalized["student_id"], - "grades_csv": csv_path.as_posix(), - "feedback": feedback_path.as_posix(), + "grades_csv": "grades.csv", + "review_csv": "review.csv", + "feedback": f"feedback/{feedback_path.name}", + "annotations": f"annotations/{annotation_path.name}", }, sort_keys=True, ) @@ -95,32 +151,53 @@ def main(argv: list[str] | None = None) -> int: return 0 -def _normalize_record(record: dict[str, Any]) -> dict[str, Any]: +def _normalize_record( + record: dict[str, Any], + *, + course_leaves: dict[str, dict[str, float]] | None, + require_annotations: bool, +) -> dict[str, Any]: if not isinstance(record, dict): raise ValueError("record must be a JSON object") - student_id = str(record.get("student_id") or record.get("student") or "").strip() + if "student_name" in record or "student_number" in record: + raise ValueError("student names and numbers must come from the private roster") + student_id = str( + record.get("student_id") or record.get("submission_id") or record.get("student") or "" + ).strip() if not student_id: raise ValueError("student_id is required") + if not SAFE_SUBMISSION_ID.fullmatch(student_id): + raise ValueError("student_id contains unsupported characters") scores = record.get("scores") if not isinstance(scores, list) or not scores: raise ValueError("scores must be a non-empty list") normalized_scores = [] - flags = list(_as_list(record.get("flags", []))) + seen_question_ids: set[str] = set() + flags = _normalized_flags(record.get("flags", []), label="record") for item in scores: if not isinstance(item, dict): raise ValueError("each score item must be an object") question_id = str(item.get("question_id", "")).strip() if not question_id: raise ValueError("question_id is required") + if question_id in seen_question_ids: + raise ValueError(f"duplicate question_id: {question_id}") + seen_question_ids.add(question_id) score = float(item.get("score")) max_score = _positive_score(item.get("max_score"), "max_score", question_id) if score < 0 or score > max_score: raise ValueError(f"score is outside range for {question_id}") + _validate_course_score( + question_id=question_id, + score=score, + max_score=max_score, + course_leaves=course_leaves, + ) confidence = str(item.get("confidence", "")).strip().lower() if confidence not in {"high", "medium", "low"}: raise ValueError(f"invalid confidence for {question_id}: {confidence}") - item_flags = list(_as_list(item.get("flags", []))) + item_flags = _normalized_flags(item.get("flags", []), label=question_id) deduction_trace = _normalize_deduction_trace( item.get("deduction_trace"), question_id=question_id, @@ -131,10 +208,10 @@ def _normalize_record(record: dict[str, Any]) -> dict[str, Any]: attention_note = item.get("attention_note") if attention_note is not None: attention_note = _safe_trace_text(attention_note, "attention_note", student_id) - if item_flags or confidence == "low": + if item_flags or confidence != "high": if attention_note is None: raise ValueError( - f"flags or low confidence require attention_note for {question_id}" + f"flags or non-high confidence require attention_note for {question_id}" ) flags.extend(f"{question_id}:{flag}" for flag in item_flags) normalized_scores.append( @@ -142,20 +219,37 @@ def _normalize_record(record: dict[str, Any]) -> dict[str, Any]: "question_id": question_id, "score": score, "max_score": max_score, - "evidence": str(item.get("evidence", "")).strip(), - "feedback": str(item.get("feedback", "")).strip(), + "evidence": _safe_record_text( + item.get("evidence"), "evidence", student_id, required=True + ), + "feedback": _safe_record_text( + item.get("feedback", ""), "feedback", student_id, required=False + ), "confidence": confidence, "flags": item_flags, "deduction_trace": deduction_trace, "attention_note": attention_note, } ) - total = float(record.get("total", sum(item["score"] for item in normalized_scores))) + if course_leaves is not None and set(seen_question_ids) != set(course_leaves): + raise ValueError("record score leaves do not match the course package") + expected_total = sum(item["score"] for item in normalized_scores) + total = float(record.get("total", expected_total)) + if abs(total - expected_total) > 1e-9: + raise ValueError("total must equal the sum of leaf scores") + annotations = _normalize_annotations( + record.get("annotations", []), + student_id=student_id, + question_ids=seen_question_ids, + score_by_id={item["question_id"]: item for item in normalized_scores}, + require_annotations=require_annotations, + ) return { "student_id": student_id, "scores": normalized_scores, + "annotations": annotations, "total": total, - "flags": sorted(set(str(flag) for flag in flags if str(flag).strip())), + "flags": sorted(set(flags)), } @@ -167,42 +261,220 @@ def _read_existing_rows(path: Path) -> tuple[list[str], list[dict[str, str]]] | return list(reader.fieldnames or []), list(reader) -def _require_private_output_directory(grades_dir: Path) -> None: - """Reject a new per-person grading record in an unignored Git location.""" - - resolved = grades_dir.resolve() - repository_root = next( - (parent for parent in (resolved, *resolved.parents) if (parent / ".git").exists()), - None, - ) - if repository_root is None: - return +def _load_course_package(path: Path) -> dict[str, dict[str, float]]: try: - relative = resolved.relative_to(repository_root).as_posix() - except ValueError: - return - check = subprocess.run( - [ - "git", - "-C", - str(repository_root), - "check-ignore", - "--quiet", - "--no-index", - "--", - relative, - ], - check=False, - capture_output=True, - text=True, - ) - if check.returncode == 0: + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, json.JSONDecodeError) as error: + raise ValueError("course package could not be read") from error + if not isinstance(payload, dict): + raise ValueError("course package must be a JSON object") + leaves = payload.get("score_leaves") + if not isinstance(leaves, list) or not leaves: + raise ValueError("course package requires non-empty score_leaves") + + result: dict[str, dict[str, float]] = {} + for leaf in leaves: + if not isinstance(leaf, dict): + raise ValueError("course package score leaf must be an object") + question_id = str(leaf.get("question_id", "")).strip() + if not question_id: + raise ValueError("course package score leaf requires question_id") + if question_id in result: + raise ValueError("course package has duplicate question_id") + max_score = _positive_score( + leaf.get("max_score"), "course-package max_score", question_id + ) + increment = _positive_score( + leaf.get("allowed_increment"), + "course-package allowed_increment", + question_id, + ) + if not _on_increment(max_score, increment): + raise ValueError( + f"course-package max_score is not on its allowed increment for {question_id}" + ) + result[question_id] = { + "max_score": max_score, + "allowed_increment": increment, + } + return result + + +def _validate_course_score( + *, + question_id: str, + score: float, + max_score: float, + course_leaves: dict[str, dict[str, float]] | None, +) -> None: + if course_leaves is None: return - if check.returncode == 1: - raise ValueError( - "grades directory inside a Git worktree must be private and ignored" + expected = course_leaves.get(question_id) + if expected is None: + raise ValueError(f"question_id is not declared by the course package: {question_id}") + if abs(max_score - expected["max_score"]) > 1e-9: + raise ValueError(f"max_score disagrees with the course package for {question_id}") + if not _on_increment(score, expected["allowed_increment"]): + raise ValueError(f"score is off the allowed increment for {question_id}") + + +def _on_increment(value: float, increment: float) -> bool: + return abs(value / increment - round(value / increment)) <= 1e-9 + + +def _roster_entry( + student_id: str, roster: dict[str, RosterEntry] | None +) -> RosterEntry | None: + if roster is None: + return None + entry = roster.get(student_id) + if entry is None: + raise ValueError("student_id is not present in the private roster") + return entry + + +def _append_csv(path: Path, header: list[str], row: dict[str, str]) -> None: + write_header = not path.exists() + with path.open("a", encoding="utf-8", newline="") as handle: + writer = csv.DictWriter(handle, fieldnames=header) + if write_header: + writer.writeheader() + writer.writerow(row) + + +def _append_review_rows( + path: Path, + record: dict[str, Any], + roster_entry: RosterEntry | None, +) -> None: + rows = [] + for item in record["scores"]: + if item["confidence"] == "high" and not item["flags"]: + continue + rows.append( + { + "student_id": record["student_id"], + "student_name": roster_entry.student_name if roster_entry else "", + "student_number": roster_entry.student_number if roster_entry else "", + "question_id": item["question_id"], + "score": _format_score(item["score"]), + "max_score": _format_score(item["max_score"]), + "confidence": item["confidence"], + "uncertainty": item["attention_note"] or "", + "flags": ";".join(item["flags"]), + } + ) + if record["flags"]: + rows.append( + { + "student_id": record["student_id"], + "student_name": roster_entry.student_name if roster_entry else "", + "student_number": roster_entry.student_number if roster_entry else "", + "question_id": "", + "score": "", + "max_score": "", + "confidence": "", + "uncertainty": "Submission-level review required.", + "flags": ";".join(record["flags"]), + } + ) + write_header = not path.exists() + with path.open("a", encoding="utf-8", newline="") as handle: + writer = csv.DictWriter(handle, fieldnames=REVIEW_COLUMNS) + if write_header: + writer.writeheader() + writer.writerows(rows) + + +def _uncertainty_cell(record: dict[str, Any]) -> str: + parts = [ + f"{item['question_id']}: {item['attention_note']}" + for item in record["scores"] + if item["confidence"] != "high" or item["flags"] + ] + if record["flags"]: + parts.append("submission: " + ", ".join(record["flags"])) + return " | ".join(parts) + + +def _normalize_annotations( + value: Any, + *, + student_id: str, + question_ids: set[str], + score_by_id: dict[str, dict[str, Any]], + require_annotations: bool, +) -> list[dict[str, Any]]: + if value is None: + value = [] + if not isinstance(value, list): + raise ValueError("annotations must be a list") + required = {"question_id", "page_id", "box", "kind", "label"} + normalized = [] + for annotation in value: + if not isinstance(annotation, dict) or set(annotation) != required: + raise ValueError("annotations require exactly question_id,page_id,box,kind,label") + question_id = str(annotation["question_id"]).strip() + if question_id not in question_ids: + raise ValueError("annotation question_id is not a scored leaf") + page_id = str(annotation["page_id"]).strip() + if not SAFE_SUBMISSION_ID.fullmatch(page_id): + raise ValueError("annotation page_id contains unsupported characters") + kind = annotation["kind"] + if kind not in ANNOTATION_KINDS: + raise ValueError("annotation kind is invalid") + box = annotation["box"] + if ( + not isinstance(box, list) + or len(box) != 4 + or any(isinstance(part, bool) or not isinstance(part, (int, float)) for part in box) + ): + raise ValueError("annotation box must contain four numeric normalized values") + x, y, width, height = (float(part) for part in box) + if x < 0 or y < 0 or width <= 0 or height <= 0 or x + width > 1 or y + height > 1: + raise ValueError("annotation box must stay inside the normalized page") + normalized.append( + { + "question_id": question_id, + "page_id": page_id, + "box": [x, y, width, height], + "kind": kind, + "label": _safe_trace_text(annotation["label"], "annotation label", student_id), + } ) - raise ValueError("could not verify whether the grades directory is ignored") + + if require_annotations: + for question_id, item in score_by_id.items(): + matching = [entry for entry in normalized if entry["question_id"] == question_id] + if item["score"] > 0 and not any( + entry["kind"] == "praise" for entry in matching + ): + raise ValueError(f"score-bearing {question_id} requires a praise annotation") + if item["score"] < item["max_score"] and not any( + entry["kind"] == "deduction" for entry in matching + ): + raise ValueError(f"non-full {question_id} requires a deduction annotation") + if (item["confidence"] != "high" or item["flags"]) and not any( + entry["kind"] == "review" for entry in matching + ): + raise ValueError(f"review-needed {question_id} requires a review annotation") + return normalized + + +def _normalized_flags(value: Any, *, label: str) -> list[str]: + flags = _as_list(value) + normalized = [] + for flag in flags: + if not re.fullmatch(r"[a-z][a-z0-9_:-]{0,127}", flag): + raise ValueError(f"invalid flag for {label}") + normalized.append(flag) + return normalized + + +def _require_private_output_directory(grades_dir: Path) -> None: + """Backward-compatible private-output guard.""" + + require_private_path(grades_dir, label="grades directory") def _feedback_markdown(record: dict[str, Any]) -> str: @@ -339,6 +611,20 @@ def _safe_trace_text(value: Any, label: str, student_id: str) -> str: return value.strip() +def _safe_record_text( + value: Any, label: str, student_id: str, *, required: bool +) -> str: + if value is None: + value = "" + if not isinstance(value, str): + raise ValueError(f"{label} must be plain text") + if not value.strip(): + if required: + raise ValueError(f"{label} must be non-blank text") + return "" + return _safe_trace_text(value, label, student_id) + + def _format_score(value: float) -> str: return f"{value:g}" diff --git a/.gitignore b/.gitignore index 85cf791..5625ec0 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,12 @@ __pycache__ /Data/ .private-data/ +# Per-person grading runs and rendered scans (private; never commit) +/grades/ +/rendered/ +/marked/ +/annotations/ + # Compiled outputs (PDFs are built artifacts, not sources) *.learning-sheet.pdf *.advanced-learning-sheet.pdf diff --git a/benchmark/core/skill_snapshots.py b/benchmark/core/skill_snapshots.py index e7230af..88da9df 100644 --- a/benchmark/core/skill_snapshots.py +++ b/benchmark/core/skill_snapshots.py @@ -22,7 +22,7 @@ class SkillSnapshot: hash_policy: str = ( "sha256(normalized LF utf-8 text for files; recursive relative path plus " "normalized content in case-folded POSIX relative-path order with an " - "original-path tie-breaker for directories)" + "original-path tie-breaker for directories; excluding __pycache__ runtime caches)" ) schema_version: int = 1 @@ -125,7 +125,12 @@ def _source_hash(path: Path) -> str: def _directory_hash(path: Path) -> str: digest = hashlib.sha256() - files = (item for item in path.rglob("*") if item.is_file()) + files = ( + item + for item in path.rglob("*") + if item.is_file() + and "__pycache__" not in item.relative_to(path).parts + ) for file_path in sorted( files, key=lambda item: ( diff --git a/experiments/records/generic-grading-delivery-v5-4/plan.json b/experiments/records/generic-grading-delivery-v5-4/plan.json new file mode 100644 index 0000000..1fc5568 --- /dev/null +++ b/experiments/records/generic-grading-delivery-v5-4/plan.json @@ -0,0 +1,32 @@ +{ + "schema_version": 1, + "record_type": "generic_grading_delivery_plan", + "skill_version_id": "skill_candidate_v5_4_generic_delivery", + "evaluation_status": "not_run", + "heldout_accessed": false, + "scope": { + "course_specific_scoring_rules_included": false, + "contains_student_level_records": false, + "contains_answers_or_transcripts": false, + "heldout_accessed": false, + "model_run_authorized": false + }, + "delivery_contract": [ + "A private ignored roster maps opaque submission IDs to local names and student numbers.", + "A submission directory groups all scans for one opaque submission ID before scoring.", + "The frozen course package supplies all course-specific score leaves, criteria, increments, alternatives, and partial-credit rules.", + "Private outputs include grades.csv, review.csv, concise feedback, validated annotations, and marked page images/PDF.", + "Production annotation mode requires praise for score-bearing work, deductions for non-full work, and review marks for uncertainty." + ], + "limits": [ + "This is an implementation and synthetic-test plan, not a model accuracy result.", + "It makes no production, heldout, cross-course accuracy, or teacher-replacement claim.", + "A course owner must review and freeze a course package before any real batch is graded.", + "Real scans, roster records, per-person grades, transcripts, and raw model outputs remain private and ignored." + ], + "next_gates": [ + "Run an owner-approved dry run with synthetic or authorized private scans.", + "Review marked pages and review.csv with the course owner.", + "Obtain separate authorization before any model grading run or course-specific evaluation." + ] +} diff --git a/experiments/records/grading-skill-error-book-registry.json b/experiments/records/grading-skill-error-book-registry.json index 4556577..ff00e2e 100644 --- a/experiments/records/grading-skill-error-book-registry.json +++ b/experiments/records/grading-skill-error-book-registry.json @@ -1,6 +1,6 @@ { "schema_version": 1, - "active_skill_version_id": "skill_candidate_v5_3_r3", + "active_skill_version_id": "skill_candidate_v5_4_generic_delivery", "policy": { "required_for_every_skill_update": true, "required_regressions_for_future_skill_updates": true, @@ -160,20 +160,35 @@ { "skill_version_id": "skill_candidate_v5_3_r3", "predecessor_skill_version_id": "skill_candidate_v5_3_r2", - "evaluation_status": "pending", - "pending_reason": "Candidate-v5.3-r3 records course-owner calibration of the Linear Algebra execution rubric, makes wrong independently allocated final answers lose their final-result points, and adds validated scoring gates for explicitly declared fundamental-method reversals. It has no new private gold, packet, model run, score, metric, or version-comparison claim.", + "evaluation_status": "superseded_pending", + "pending_reason": "Candidate-v5.3-r3 was superseded by the generic delivery revision before a production evaluation. Its existing development-only aggregate record remains historical and does not establish a production or cross-course claim.", "pending_gates": [ - "public review of the r3 rubric, prompt, scoring-gate validator, skill snapshot, and tests", - "separate explicit authorization before a new private development-only gold revision", - "separate explicit authorization before a newly frozen r3 packet or model run", - "privacy review confirming deduction traces remain in private run outputs and public artifacts stay aggregate-only", - "development error-book, confidence audit, and iteration-delta review after any authorized run" + "preserve the frozen r3 rubric and its aggregate-only development record", + "review a complete course-specific package before any real batch", + "obtain separate authorization before any model grading run or course-specific evaluation", + "keep deduction traces, individual records, and all private evidence out of public artifacts" ], "skill_snapshot": "experiments/skill_versions/skill_candidate_v5_3_r3.json", "skill_canonical_hash": "3a279867b28eb3f0480db9d350d7863371dbe8233d6db6f8eab5bbbda6635e0f", "public_contract_plan": "experiments/records/linearalgebra-quiz1-v5-3-r3-human-calibrated/plan.json", - "note_en": "Candidate-v5.3-r3 preserves V5.3 deduction traces and records a separately versioned human-calibrated Linear Algebra execution rubric. It remains a development candidate with no run or accuracy claim.", - "note_zh": "Candidate-v5.3-r3 保留 V5.3 的扣分依据合同,并记录单独版本化、经课程负责人校准的 Linear Algebra 执行 rubric。它仍是未运行的开发候选,不含准确率或改进声明。" + "note_en": "Candidate-v5.3-r3 preserves V5.3 deduction traces and records a separately versioned human-calibrated Linear Algebra execution rubric. Its development-only aggregate record is not held-out or production evidence.", + "note_zh": "Candidate-v5.3-r3 保留 V5.3 的扣分依据合同,并记录单独版本化、经课程负责人校准的 Linear Algebra 执行 rubric。其仅开发集聚合记录不是留出集或生产证据。" + }, + { + "skill_version_id": "skill_candidate_v5_4_generic_delivery", + "predecessor_skill_version_id": "skill_candidate_v5_3_r3", + "evaluation_status": "pending", + "pending_reason": "This revision makes the live grading skill a generic private delivery workflow: roster validation, complete-scan grouping, frozen course packages, grades and review CSVs, and validated praise/deduction/review annotations. It has no model run, score, metric, heldout, or production-accuracy claim.", + "pending_gates": [ + "course-owner review and freeze of a complete course-specific package before any real batch", + "owner-approved dry run using synthetic or authorized private scans, with marked-page and review.csv inspection", + "separate explicit authorization before any model grading run or course-specific evaluation", + "development-only error-book, confidence audit, and iteration review before any accuracy claim" + ], + "skill_snapshot": "experiments/skill_versions/skill_candidate_v5_4_generic_delivery.json", + "skill_canonical_hash": "8fc0bffe0b7c72ea1f03706b226c04abf9e101aae83dadc68cf8e6d6cf688e90", + "public_contract_plan": "experiments/records/generic-grading-delivery-v5-4/plan.json", + "note_en": "The active generic delivery skill is registered for reproducibility and synthetic contract testing only. It does not make a course-specific, production, or teacher-replacement claim." } ] } diff --git a/experiments/skill_versions/skill_candidate_v5_4_generic_delivery.json b/experiments/skill_versions/skill_candidate_v5_4_generic_delivery.json new file mode 100644 index 0000000..b069bb7 --- /dev/null +++ b/experiments/skill_versions/skill_candidate_v5_4_generic_delivery.json @@ -0,0 +1,15 @@ +{ + "canonical_hash": "8fc0bffe0b7c72ea1f03706b226c04abf9e101aae83dadc68cf8e6d6cf688e90", + "hash_policy": "sha256(normalized LF utf-8 text for files; recursive relative path plus normalized content in case-folded POSIX relative-path order with an original-path tie-breaker for directories; excluding __pycache__ runtime caches)", + "mirror_synchronized": true, + "schema_version": 1, + "skill_hashes": { + "agents": "8fc0bffe0b7c72ea1f03706b226c04abf9e101aae83dadc68cf8e6d6cf688e90", + "claude": "8fc0bffe0b7c72ea1f03706b226c04abf9e101aae83dadc68cf8e6d6cf688e90" + }, + "skill_source_paths": { + "agents": ".agents/skills/grade-homework", + "claude": ".claude/skills/grade-homework" + }, + "skill_version_id": "skill_candidate_v5_4_generic_delivery" +} diff --git a/tests/benchmark/core/test_candidate_v3_assets.py b/tests/benchmark/core/test_candidate_v3_assets.py index ff77e01..cad0b2f 100644 --- a/tests/benchmark/core/test_candidate_v3_assets.py +++ b/tests/benchmark/core/test_candidate_v3_assets.py @@ -76,8 +76,6 @@ def test_candidate_v3_contract_is_present_in_all_model_facing_assets(self): expected = ( PROMPT, STRICT_SNAPSHOT, - SKILL, - REFERENCE, ) for path in expected: @@ -89,7 +87,7 @@ def test_candidate_v3_contract_is_present_in_all_model_facing_assets(self): def test_candidate_v3_preserves_required_safeguards(self): combined = "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT, STRICT_SNAPSHOT, SKILL, REFERENCE) + for path in (PROMPT, STRICT_SNAPSHOT) ).lower() for phrase in ( @@ -109,18 +107,17 @@ def test_candidate_v3_preserves_required_safeguards(self): with self.subTest(phrase=phrase): self.assertIn(phrase, combined) - def test_current_assets_define_cross_course_question_type_rules(self): - for path in (PROMPT_V5_2, SKILL, REFERENCE): - text = _normalize_whitespace(path.read_text(encoding="utf-8")) - for rule in QUESTION_TYPE_RULES: - with self.subTest(path=path, rule=rule): - self.assertIn(_normalize_whitespace(rule), text) + def test_historical_v5_2_prompt_retains_its_question_type_rules(self): + text = _normalize_whitespace(PROMPT_V5_2.read_text(encoding="utf-8")) + for rule in QUESTION_TYPE_RULES: + with self.subTest(rule=rule): + self.assertIn(_normalize_whitespace(rule), text) def test_calculation_rule_preserves_physics_process_credit(self): combined = _normalize_whitespace( "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT, STRICT_SNAPSHOT, SKILL, REFERENCE) + for path in (PROMPT, STRICT_SNAPSHOT) ) ) @@ -140,7 +137,7 @@ def test_evidence_fields_are_plain_strings_for_schema_compatibility(self): combined = _normalize_whitespace( "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT, STRICT_SNAPSHOT, SKILL, REFERENCE) + for path in (PROMPT, STRICT_SNAPSHOT) ) ) @@ -166,7 +163,7 @@ def test_strict_prompt_preserves_the_generic_grading_algorithm_verbatim(self): def test_candidate_v3_assets_are_generic_and_private(self): combined = "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT, SKILL, REFERENCE) + for path in (PROMPT, STRICT_SNAPSHOT) ) self.assertNotRegex(combined, r"\bS\d{3}\b") @@ -196,8 +193,6 @@ def test_candidate_v31_calibration_rules_are_present(self): expected = ( PROMPT_V31, STRICT_SNAPSHOT_V31, - SKILL, - REFERENCE, ) for path in expected: @@ -217,8 +212,6 @@ def test_candidate_v31_open_ended_adequacy_rule_is_present(self): expected = ( PROMPT_V31, STRICT_SNAPSHOT_V31, - SKILL, - REFERENCE, ) for path in expected: @@ -251,8 +244,6 @@ def test_candidate_v32_official_style_tolerance_rule_is_present(self): expected = ( PROMPT_V32, STRICT_SNAPSHOT_V32, - SKILL, - REFERENCE, ) for path in expected: @@ -358,7 +349,7 @@ def test_candidate_v33_remains_generic_and_private(self): def test_current_assets_do_not_inherit_dsaa_specific_calibration(self): combined = "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT_V5_2, SKILL, REFERENCE) + for path in (SKILL, REFERENCE) ) for phrase in ( @@ -375,7 +366,7 @@ def test_current_assets_score_whole_submissions_not_individual_pages(self): combined = _normalize_whitespace( "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT_V5_2, SKILL, REFERENCE) + for path in (SKILL, REFERENCE) ) ).lower() @@ -393,7 +384,7 @@ def test_current_assets_score_declared_leaf_subparts_separately(self): combined = _normalize_whitespace( "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT_V5_2, SKILL, REFERENCE) + for path in (SKILL, REFERENCE) ) ).lower() @@ -411,7 +402,7 @@ def test_current_assets_treat_page_positions_as_locators_not_question_ids(self): combined = _normalize_whitespace( "\n".join( path.read_text(encoding="utf-8") - for path in (PROMPT_V5_2, SKILL, REFERENCE) + for path in (SKILL, REFERENCE) ) ).lower() diff --git a/tests/benchmark/core/test_deduction_trace_contract.py b/tests/benchmark/core/test_deduction_trace_contract.py index fd535b5..ee20188 100644 --- a/tests/benchmark/core/test_deduction_trace_contract.py +++ b/tests/benchmark/core/test_deduction_trace_contract.py @@ -290,13 +290,13 @@ def test_schema_and_packet_bind_deduction_trace_contract(self): metadata["output_schema_hash"], manifest["output_schema_hash"] ) - def test_public_v5_3_plan_is_privacy_safe_and_registry_tracks_pending_contract(self): + def test_public_generic_delivery_plan_is_privacy_safe_and_registry_tracks_pending_contract(self): repo_root = Path(__file__).parents[3] plan_path = ( repo_root / "experiments" / "records" - / "candidate-v5_3-deduction-trace-plan" + / "generic-grading-delivery-v5-4" / "plan.json" ) registry_path = ( @@ -311,7 +311,10 @@ def test_public_v5_3_plan_is_privacy_safe_and_registry_tracks_pending_contract(s self.assertEqual(audit_public_error_summary(plan), []) self.assertEqual(plan["evaluation_status"], "not_run") self.assertFalse(plan["heldout_accessed"]) - self.assertEqual(registry["active_skill_version_id"], "skill_candidate_v5_3_r3") + self.assertEqual( + registry["active_skill_version_id"], + "skill_candidate_v5_4_generic_delivery", + ) self.assertEqual(registry["entries"][-1]["evaluation_status"], "pending") self.assertEqual( validate_error_book_registry( diff --git a/tests/benchmark/core/test_grade_homework_candidate_skill.py b/tests/benchmark/core/test_grade_homework_candidate_skill.py index 8b25e38..eecde01 100644 --- a/tests/benchmark/core/test_grade_homework_candidate_skill.py +++ b/tests/benchmark/core/test_grade_homework_candidate_skill.py @@ -18,9 +18,13 @@ def test_skill_bundled_resources_are_synchronized(self): relative_files = [ Path("SKILL.md"), Path("references/grading-prompt.md"), + Path("references/course-package-template.json"), + Path("scripts/roster.py"), Path("scripts/discover.py"), Path("scripts/to_images.py"), + Path("scripts/render_submission.py"), Path("scripts/write_outputs.py"), + Path("scripts/annotate_submission.py"), ] for relative in relative_files: @@ -30,46 +34,20 @@ def test_skill_bundled_resources_are_synchronized(self): (CLAUDE_SKILL / relative).read_text(encoding="utf-8"), ) - def test_skill_uses_teacher_partial_credit_policy(self): + def test_skill_keeps_course_specific_policy_outside_the_generic_core(self): text = (AGENT_SKILL / "SKILL.md").read_text(encoding="utf-8").lower() prompt = (AGENT_SKILL / "references" / "grading-prompt.md").read_text( encoding="utf-8" ).lower() - combined = text + "\n" + prompt + combined = " ".join((text + "\n" + prompt).split()) - self.assertIn("do not use 0.25-point", combined) - self.assertIn("final answer is correct and the process", combined) - self.assertIn("roughly correct", combined) - self.assertIn("award full credit", combined) - self.assertIn("when the final answer is wrong", combined) - self.assertIn("process credit", combined) - self.assertNotIn("if the rubric allows quarter points", combined) - self.assertNotIn("quarter-point increment", combined) - - def test_skill_uses_candidate_v3_evidence_states_and_caps(self): - combined = "\n".join( - ( - (AGENT_SKILL / "SKILL.md").read_text(encoding="utf-8"), - (AGENT_SKILL / "references" / "grading-prompt.md").read_text( - encoding="utf-8" - ), - ) - ) - for phrase in ( - "key_term_evidence", - "concept_evidence", - "relation_evidence", - "mentioned_only", - "partial_understanding", - "demonstrated", - "misused_or_contradicted", - "Do not award duplicate credit", - "semantic equivalent", - "question type", - "cannot raise the subtotal", - ): - with self.subTest(phrase=phrase): - self.assertIn(phrase, combined) + self.assertIn("course package", combined) + self.assertIn("does not supply subject knowledge", combined) + self.assertIn("do not invent a universal point rule", combined) + self.assertIn("frozen for the batch", combined) + for historical_overlay in ("physics week", "dsaa", "q7", "q8", "q9"): + with self.subTest(historical_overlay=historical_overlay): + self.assertNotIn(historical_overlay, combined) def test_skill_declares_cross_course_leaf_subpart_scoring(self): combined = "\n".join( @@ -89,7 +67,7 @@ def test_skill_declares_cross_course_leaf_subpart_scoring(self): with self.subTest(phrase=phrase): self.assertIn(phrase, combined) - def test_skill_declares_calculation_locality_and_equivalence_guards(self): + def test_skill_declares_generic_evidence_trace_and_annotation_contract(self): combined = "\n".join( ( (AGENT_SKILL / "SKILL.md").read_text(encoding="utf-8"), @@ -99,12 +77,14 @@ def test_skill_declares_calculation_locality_and_equivalence_guards(self): ) ).lower() for phrase in ( - "first score-affecting issue", - "check algebraic equivalence", - "equivalent_form_accepted", - "do not convert its downstream result", - "wrong formula", - "irrelevant to every declared criterion", + "complete anonymous submission", + "deduction_trace", + "points_deducted", + "attention_note", + "marked-page annotations", + "student_name,student_number", + "private roster", + "a teacher owns", ): with self.subTest(phrase=phrase): self.assertIn(phrase, combined) @@ -149,9 +129,12 @@ def test_discover_script_reports_solution_submissions_and_late_students(self): self.assertEqual(result.returncode, 0) self.assertIsNone(payload["solutions_error"]) + self.assertEqual(payload["root"], ".") self.assertEqual(payload["solutions_candidates"], ["solutions.pdf"]) self.assertEqual(len(payload["submissions"]), 2) self.assertEqual(payload["late_students"], ["S002"]) + self.assertNotIn("S001_page1.jpg", result.stdout) + self.assertNotIn(str(root), result.stdout) def test_to_images_script_converts_image_to_page_png(self): with tempfile.TemporaryDirectory() as tmp: @@ -177,6 +160,8 @@ def test_to_images_script_converts_image_to_page_png(self): self.assertEqual(result.returncode, 0) self.assertEqual(payload["status"], "ok") + self.assertEqual(payload["pages"], ["page-001.png"]) + self.assertNotIn(str(source), result.stdout) self.assertTrue(page_exists) def test_write_outputs_script_writes_csv_and_feedback(self): @@ -235,6 +220,164 @@ def test_write_outputs_script_writes_csv_and_feedback(self): self.assertIn("Shows correct setup", feedback) self.assertIn("Deduction trace", feedback) + def test_grouped_batch_writes_roster_columns_and_renders_marked_pages(self): + record = { + "student_id": "S001", + "scores": [ + { + "question_id": "leaf-a", + "score": 1, + "max_score": 2, + "evidence": "A visible required component is absent.", + "feedback": "Please include the required component.", + "confidence": "medium", + "flags": ["needs_manual_review"], + "deduction_trace": [ + { + "rubric_criterion": "visible requirement", + "observed_evidence_or_missing_or_incorrect_part": "The required component is absent.", + "deduction_type": "missing_required_evidence", + "points_deducted": 1, + } + ], + "attention_note": "Please verify the marked region.", + } + ], + "annotations": [ + { + "question_id": "leaf-a", + "page_id": "source-001-page-001", + "box": [0.1, 0.65, 0.4, 0.15], + "kind": "praise", + "label": "A valid component is clearly shown.", + }, + { + "question_id": "leaf-a", + "page_id": "source-001-page-001", + "box": [0.1, 0.1, 0.4, 0.2], + "kind": "deduction", + "label": "Required component is missing.", + }, + { + "question_id": "leaf-a", + "page_id": "source-001-page-001", + "box": [0.1, 0.4, 0.4, 0.2], + "kind": "review", + "label": "Please verify this region.", + }, + ], + "total": 1, + "flags": [], + } + course_package = { + "schema_version": 1, + "course_id": "synthetic-course", + "assessment_id": "synthetic-assessment", + "score_leaves": [ + {"question_id": "leaf-a", "max_score": 2, "allowed_increment": 1} + ], + } + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + roster = root / "roster.csv" + roster.write_text( + "submission_id,student_name,student_number\nS001,Test Person,900001\n", + encoding="utf-8", + ) + package_path = root / "course-package.json" + package_path.write_text(json.dumps(course_package), encoding="utf-8") + submission = root / "submissions" / "S001" + submission.mkdir(parents=True) + Image.new("RGB", (80, 60), color=(255, 255, 255)).save( + submission / "scan.png" + ) + rendered = root / "rendered" / "S001" + render = subprocess.run( + [ + sys.executable, + str(AGENT_SKILL / "scripts" / "render_submission.py"), + str(submission), + str(rendered), + "--submission-id", + "S001", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=False, + ) + grades = root / "grades" + missing_praise_record = dict(record) + missing_praise_record["annotations"] = [ + annotation + for annotation in record["annotations"] + if annotation["kind"] != "praise" + ] + missing_praise = subprocess.run( + [ + sys.executable, + str(AGENT_SKILL / "scripts" / "write_outputs.py"), + str(root / "missing-praise"), + "--roster", + str(roster), + "--course-package", + str(package_path), + "--require-annotations", + ], + input=json.dumps(missing_praise_record), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=False, + ) + write = subprocess.run( + [ + sys.executable, + str(AGENT_SKILL / "scripts" / "write_outputs.py"), + str(grades), + "--roster", + str(roster), + "--course-package", + str(package_path), + "--require-annotations", + ], + input=json.dumps(record), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=False, + ) + annotate = subprocess.run( + [ + sys.executable, + str(AGENT_SKILL / "scripts" / "annotate_submission.py"), + str(rendered / "pages.json"), + str(grades / "annotations" / "S001.json"), + str(grades / "marked" / "S001"), + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=False, + ) + with (grades / "grades.csv").open(encoding="utf-8", newline="") as handle: + rows = list(csv.DictReader(handle)) + review = (grades / "review.csv").read_text(encoding="utf-8") + manifest = (rendered / "pages.json").read_text(encoding="utf-8") + + self.assertEqual(render.returncode, 0, render.stderr) + self.assertEqual(missing_praise.returncode, 2) + self.assertIn("requires a praise annotation", missing_praise.stderr) + self.assertEqual(write.returncode, 0, write.stderr) + self.assertEqual(annotate.returncode, 0, annotate.stderr) + self.assertEqual(rows[0]["student_name"], "Test Person") + self.assertEqual(rows[0]["student_number"], "900001") + self.assertIn("needs_manual_review", review) + self.assertTrue((grades / "marked" / "S001" / "marked.pdf").is_file()) + self.assertNotIn("scan.png", manifest) + self.assertNotIn(str(root), write.stdout) + self.assertNotIn("Test Person", write.stdout) + def test_write_outputs_refuses_an_unignored_git_directory(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) diff --git a/tests/benchmark/core/test_grade_homework_skill_contract.py b/tests/benchmark/core/test_grade_homework_skill_contract.py index d060ef1..a4efff2 100644 --- a/tests/benchmark/core/test_grade_homework_skill_contract.py +++ b/tests/benchmark/core/test_grade_homework_skill_contract.py @@ -4,7 +4,7 @@ AGENT_SKILL = Path(".agents/skills/grade-homework") CLAUDE_SKILL = Path(".claude/skills/grade-homework") -CURRENT_PROMPT = Path("experiments/prompt_templates/grade_candidate_v5_3.txt") +HISTORICAL_PROMPT = Path("experiments/prompt_templates/grade_candidate_v5_3.txt") class GradeHomeworkSkillContractTests(unittest.TestCase): @@ -12,62 +12,54 @@ def test_agent_and_claude_skill_directories_match(self): agent_files = { path.relative_to(AGENT_SKILL).as_posix(): path.read_bytes() for path in AGENT_SKILL.rglob("*") - if path.is_file() + if path.is_file() and "__pycache__" not in path.relative_to(AGENT_SKILL).parts } claude_files = { path.relative_to(CLAUDE_SKILL).as_posix(): path.read_bytes() for path in CLAUDE_SKILL.rglob("*") - if path.is_file() + if path.is_file() and "__pycache__" not in path.relative_to(CLAUDE_SKILL).parts } self.assertEqual(agent_files, claude_files) - def test_current_candidate_is_cross_course_and_type_first(self): - text = CURRENT_PROMPT.read_text(encoding="utf-8").lower() - expected_types = ( - "objective_selection", - "calculation", - "calculation_short_answer", - "proof", - "diagram", - "essay", - ) - for question_type in expected_types: - with self.subTest(question_type=question_type): - self.assertIn(question_type, text) - - for inherited_rule in ( - "q7 proof-locality", - "q8 enumerator", - "q9 conceptual essay", - "church-turing", - "power-of-two", - ): - with self.subTest(inherited_rule=inherited_rule): - self.assertNotIn(inherited_rule, text) - - def test_current_candidate_preserves_key_calibration_safeguards(self): - text = CURRENT_PROMPT.read_text(encoding="utf-8").lower() + def test_live_skill_is_generic_and_requires_a_current_course_package(self): + combined = "\n".join( + ( + (AGENT_SKILL / "SKILL.md").read_text(encoding="utf-8"), + (AGENT_SKILL / "references" / "grading-prompt.md").read_text( + encoding="utf-8" + ), + (AGENT_SKILL / "references" / "course-package-template.json").read_text( + encoding="utf-8" + ), + ) + ).lower() + combined = " ".join(combined.split()) for safeguard in ( - "evidence before assigning points", - "semantic equivalent", - "official-style adequacy", - "material-error cap", - "local misconception", - "second pass", - "true/false", - "answer-only allocation", - "entire anonymous submission", - "page-level marks", - "page position", - "never question numbers", + "frozen course package", + "complete anonymous submission", "deduction_trace", - "first material error", - "answer-only cap", - "bonus leaves", + "attention_note", + "review.csv", + "marked-page annotations", + "do not invent a universal point rule", + "a teacher owns", ): with self.subTest(safeguard=safeguard): - self.assertIn(safeguard, text) + self.assertIn(safeguard, combined) + for historical_overlay in ("physics week", "dsaa", "church-turing", "q7", "q8", "q9"): + with self.subTest(historical_overlay=historical_overlay): + self.assertNotIn(historical_overlay, combined) + + def test_historical_candidate_prompt_remains_an_explicitly_separate_artifact(self): + text = HISTORICAL_PROMPT.read_text(encoding="utf-8") + self.assertIn("deduction_trace", text) + self.assertNotEqual( + text, + (AGENT_SKILL / "references" / "grading-prompt.md").read_text( + encoding="utf-8" + ), + ) if __name__ == "__main__": diff --git a/tests/benchmark/core/test_skill_snapshots.py b/tests/benchmark/core/test_skill_snapshots.py index 2f4d486..887d7cf 100644 --- a/tests/benchmark/core/test_skill_snapshots.py +++ b/tests/benchmark/core/test_skill_snapshots.py @@ -66,6 +66,26 @@ def test_directory_snapshot_tracks_bundled_resources(self): self.assertTrue(snapshot.mirror_synchronized) self.assertIn("directories", snapshot.hash_policy) + def test_directory_snapshot_ignores_runtime_cache_files(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + first = root / "agent" + second = root / "claude" + for folder in (first, second): + folder.mkdir(parents=True) + (folder / "SKILL.md").write_text("skill\n", encoding="utf-8") + cache = first / "__pycache__" + cache.mkdir() + (cache / "SKILL.cpython-312.pyc").write_bytes(b"runtime cache") + + snapshot = build_skill_snapshot( + skill_version_id="skill_runtime_cache_ignored", + source_paths={"agents": first, "claude": second}, + ) + + self.assertTrue(snapshot.mirror_synchronized) + self.assertIn("excluding __pycache__", snapshot.hash_policy) + def test_directory_snapshot_uses_portable_posix_path_order(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) / "skill" @@ -105,9 +125,9 @@ def test_checked_in_baseline_snapshot_is_synchronized(self): ) self.assertEqual(len(set(snapshot.skill_hashes.values())), 1) - def test_current_skill_directories_match_candidate_v5_3_r3_snapshot(self): + def test_current_skill_directories_match_generic_delivery_snapshot(self): snapshot_path = Path( - "experiments/skill_versions/skill_candidate_v5_3_r3.json" + "experiments/skill_versions/skill_candidate_v5_4_generic_delivery.json" ) snapshot = SkillSnapshot.from_json_path(snapshot_path) rebuilt = build_skill_snapshot( @@ -120,6 +140,7 @@ def test_current_skill_directories_match_candidate_v5_3_r3_snapshot(self): self.assertEqual(rebuilt.skill_hashes, snapshot.skill_hashes) self.assertTrue(snapshot.mirror_synchronized) + self.assertEqual(snapshot.skill_version_id, "skill_candidate_v5_4_generic_delivery") def test_historical_candidate_v3_snapshot_remains_loadable_and_distinct(self): baseline = SkillSnapshot.from_json_path( diff --git a/tests/benchmark/physics/test_skill_sync.py b/tests/benchmark/physics/test_skill_sync.py index f0246c5..36b33d8 100644 --- a/tests/benchmark/physics/test_skill_sync.py +++ b/tests/benchmark/physics/test_skill_sync.py @@ -13,38 +13,39 @@ def test_claude_and_agent_skills_match(self): agent = agent_path.read_text(encoding="utf-8") self.assertEqual(claude, agent) - def test_skill_requires_frozen_evidence_first_workflow(self): + def test_skill_requires_frozen_course_package_and_evidence_first_workflow(self): text = Path(".claude/skills/grade-homework/SKILL.md").read_text( encoding="utf-8" ).lower() for phrase in ( - "page ordering", + "course package", "rubric", "evidence", "confidence", - "second-pass", - "do not guess", + "review.csv", + "do not infer", ): self.assertIn(phrase, text) - def test_skill_includes_benchmark_informed_safeguards(self): + def test_skill_includes_generic_delivery_safeguards_without_course_overlays(self): text = Path(".claude/skills/grade-homework/SKILL.md").read_text( encoding="utf-8" ).lower() normalized = " ".join(text.split()) for phrase in ( - "benchmark-informed safeguards", - "does not prove that transcript workflows are generally better", - "direct-image baseline", - "blank or apparently missing answers", - "high-impact deductions", - "flagged items", + "private roster", + "grades/grades.csv", + "marked pdf", + "deduction trace", + "flagged, medium-confidence, or low-confidence", "spot-check", + "not a teacher replacement", ): self.assertIn(phrase, normalized) for forbidden in ( - "therefore transcript-based grading is generally better", - "transcript grading is generally better than direct image grading", + "benchmark-informed safeguards", + "physics week", + "dsaa3071", ): self.assertNotIn(forbidden, normalized)