Skip to content

fix(autograder): grade comment-stripped SQL so guide comments don't skew scores#6

Merged
lassebenni merged 2 commits into
mainfrom
fix/autograder-todo-guide-comments
Jul 6, 2026
Merged

fix(autograder): grade comment-stripped SQL so guide comments don't skew scores#6
lassebenni merged 2 commits into
mainfrom
fix/autograder-todo-guide-comments

Conversation

@lassebenni

Copy link
Copy Markdown
Collaborator

Problem

The Week 10 autograder (.hyf/test.sh) has the same file_is_filled() blanket-"TODO" gate as Week 9 — but Week 10 needs a deeper fix, because its dbt scaffold ships -- TODO: guide comments inside the starter models, and those comments contain gradeable keywords ("add JOIN", "add GROUP BY", the six output column names).

Two failure modes result:

  1. Correct work scored as a stub. A student who writes the model below the guide comment and leaves the comment in place was failed by file_is_filled (it rejects any file containing "TODO"). Verified: a fully-correct solution that keeps the comments scores 55/FAIL under the current grader (fct_daily_borough_stats.sql: still contains TODO stubs, singular test: still a TODO stub).

  2. Naively relaxing the gate lets the empty scaffold pass. Just fixing file_is_filled (the Week 9 approach) makes the bare scaffold score 62 and pass, because the pattern greps (grep JOIN "$mart") match the JOIN / GROUP BY / column-name keywords sitting inside the TODO comments.

Fix

  • file_is_filled() strips SQL line-comments and only flags a line whose content begins with TODO (an unreplaced markdown/YAML placeholder).
  • New sqlgrep() helper matches patterns against comment-stripped SQL. Every .sql content check now routes through it: staging source(), IS NOT NULL, fare_amount >= 0, tip_pct; mart ref() to both, JOIN, GROUP BY, output columns; and the singular-test ref(). Keywords hidden in comments no longer count as real SQL. YAML/markdown checks are unchanged.

Verified (both sides)

Case before → after
Bare scaffold (nothing filled) 62 pass48 FAIL
Correct solution, guide comments kept 55 FAIL100 pass

So it stops penalizing left-in guide comments and stops the empty scaffold from passing on comment text. There are currently no open student PRs on this repo, so no live submissions are affected; this hardens the template before the cohort forks it.

Note for future weeks: prefer sqlgrep (comment-stripped matching) for any keyword check on a .sql deliverable, and avoid the "fail on any TODO" gate — it both over- and under-counts.

🤖 Generated with Claude Code

Lasse Benninga and others added 2 commits July 6, 2026 17:15
… and hidden keywords don't skew scores

The Week 9 autograder had the same file_is_filled() blanket-TODO bug, but Week 10
needs a deeper fix: the dbt scaffold ships "-- TODO:" guide comments *inside* the
starter models, and those comments contain gradeable keywords ("add JOIN",
"add GROUP BY", the output column names). So two failure modes existed:

1. A correct submission that leaves the guide comment in place was scored as an
   empty stub (file_is_filled failed on the word TODO). Verified: a fully-correct
   solution that keeps the comments scored 55/FAIL under the old grader.

2. Naively relaxing file_is_filled alone let the *bare scaffold* pass at 62,
   because the pattern greps matched the JOIN/GROUP BY/column keywords sitting
   inside the TODO comments.

Fix:
- file_is_filled() strips SQL line-comments and only flags a line whose content
  *begins* with TODO (an unreplaced markdown/YAML placeholder).
- New sqlgrep() helper matches patterns against comment-stripped SQL; every .sql
  content check (staging filters, tip_pct, mart JOIN/GROUP BY, columns, refs,
  singular-test ref) now goes through it, so keywords hidden in comments no
  longer count as real SQL.

Verified: bare scaffold 62->48 (now correctly FAILS), a correct comments-kept
solution 55->100 (now correctly PASSES). YAML/markdown checks unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tartup

Same load-time permissions bug as the other assignment repos: the grade job
forced contents:none, but the reusable auto-grade.yml needs contents:read.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lassebenni lassebenni merged commit 8601a78 into main Jul 6, 2026
1 check passed
@lassebenni lassebenni deleted the fix/autograder-todo-guide-comments branch July 6, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant