From 9a9535112e37b4b8d9496bba522612e2948c0ef0 Mon Sep 17 00:00:00 2001 From: Lasse Benninga Date: Mon, 29 Jun 2026 08:52:22 +0200 Subject: [PATCH] fix(autograder): replace minimal completeness check with level-based grader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five bugs in the original test.sh: 1. file_is_filled() only matched standalone TODO lines, so `-- TODO: GROUP BY ... HAVING COUNT(*) > 1` passed the check AND triggered the HAVING COUNT keyword match → scaffold scored 45/100 and crossed the old passingScore 50 threshold automatically (untouched scaffold = instant pass). 2. Task 1.4 (orphaned-key LEFT JOIN / NOT EXISTS check) not graded at all. 3. Task 2: negative fare filter (WHERE fare_amount >= 0) not verified — the core cleaning step could be omitted with no point loss. 4. Task 2: pickup_datetime::TIMESTAMP cast not verified — students who skip it break their own Task 4 time-pattern queries but still scored full marks. 5. score.example.json had passingScore 50 while test.sh wrote passingScore 60. Changes: - Add grader_lib.sh (shared pass/fail/warn helpers from Week 6 autograder). - Rewrite test.sh: 7 levels, structured pass/fail output per check, rich feedback messages pointing to the exact task and pattern needed. - Fix file_is_filled() to catch any occurrence of TODO (case-insensitive). - Grade all four Task 1 checks, including the relationship orphan check. - Grade Task 2 fare filter and TIMESTAMP cast explicitly. - Align score.example.json passingScore with the 60-point threshold in the grader (was 50, now 60). - Update .hyf/README.md with the grading table. Ladder verified: scaffold → 10/100 (fail), working solution → 100/100 (pass). Co-Authored-By: Claude Sonnet 4.6 --- .hyf/README.md | 21 ++- .hyf/grader_lib.sh | 250 ++++++++++++++++++++++++++++++++++ .hyf/score.example.json | 6 +- .hyf/test.sh | 290 +++++++++++++++++++++++++++++++++------- 4 files changed, 512 insertions(+), 55 deletions(-) create mode 100644 .hyf/grader_lib.sh diff --git a/.hyf/README.md b/.hyf/README.md index 38f1a4d..cdfaea9 100644 --- a/.hyf/README.md +++ b/.hyf/README.md @@ -2,7 +2,7 @@ ## How it works 1. The auto grade tool runs the `test.sh` script located in this directory. -2. `test.sh` should write to a file named `score.json` with following JSON format: +2. `test.sh` writes to `score.json` with the following JSON format: ```json { "score": , @@ -10,6 +10,21 @@ "pass": "" } ``` - All scores are out of 100. It is up to the assignment to determine how to calculate the score. -3. The auto grade runs via a github action on PR creation and updates the PR with the score. + All scores are out of 100. Passing score is 60. +3. The auto grade runs via a GitHub Action on PR creation and updates the PR with the score. +## What is graded + +This is a **static analysis only** grader — it cannot connect to the Azure PostgreSQL database. It verifies: + +| Level | Task | Points | +|-------|------|--------| +| 1 | All 5 required files present | 10 | +| 2 | Task 1: validation_queries.sql — 4 checks (duplicates, NULLs, range, orphans) | 20 | +| 3 | Task 2: schema_setup.sql — views defined, fare filter, TIMESTAMP cast | 30 | +| 4 | Task 3: data_dictionary.md — grain, primary key, measures | 15 | +| 5 | Task 4: verification_results.sql — borough, revenue, time-pattern queries | 15 | +| 6 | Task 4: assets/borough_count.png screenshot present | 5 | +| 7 | Task 5: AI_ASSIST.md — 4 sections filled | 5 | + +The **final grade is teacher review** against the assignment rubric — the teacher runs the SQL and checks that findings match the real NYC taxi data. diff --git a/.hyf/grader_lib.sh b/.hyf/grader_lib.sh new file mode 100644 index 0000000..d383a4a --- /dev/null +++ b/.hyf/grader_lib.sh @@ -0,0 +1,250 @@ +#!/usr/bin/env bash +# grader_lib.sh — shared helpers for HYF Data Track autograders. +# Source this at the top of test.sh: +# source "$(dirname "$0")/grader_lib.sh" +# +# Provides: pass(), fail(), warn(), print_results(), write_score(), +# and a set of common static-analysis checks derived from recurring +# PR review patterns across cohort c55. + +_grader_details=() + +pass() { _grader_details+=("✓ PASS $1"); } +fail() { _grader_details+=("✗ FAIL $1"); } +warn() { _grader_details+=("⚠ WARN $1"); } + +print_results() { + local header="${1:-Autograder Results}" + echo "" + echo "=== $header ===" + for line in "${_grader_details[@]}"; do echo " $line"; done + echo "" +} + +write_score() { + # write_score [] + local score="$1" + local passing="$2" + local outfile="${3:-$(dirname "${BASH_SOURCE[0]}")/score.json}" + local pass_flag="false" + [[ "$score" -ge "$passing" ]] && pass_flag="true" + cat > "$outfile" << JSON +{ + "score": $score, + "pass": $pass_flag, + "passingScore": $passing +} +JSON + echo "Score: $score / 100 (passing: $passing) pass=$pass_flag" +} + +# ── Common static-analysis checks ──────────────────────────────────────────── +# Each function: returns 0 on pass, 1 on fail/warn (for caller logic). +# All feedback goes through pass()/fail()/warn() so it appears in print_results. + +check_no_print_statements() { + # Usage: check_no_print_statements [label] + # Flags bare print() calls that should be logging calls. + local dir="${1:-.}" + local label="${2:-$dir}" + local found + found=$(grep -rn "^[[:space:]]*print(" "$dir" --include="*.py" 2>/dev/null | grep -v "# noqa" || true) + if [[ -n "$found" ]]; then + local count + count=$(echo "$found" | wc -l | tr -d ' ') + warn "$label: $count print() call(s) found — use logging.info/warning/error instead (see Week 1 Ch1)" + return 1 + fi + return 0 +} + +check_no_notimplemented() { + # Usage: check_no_notimplemented [label] + # Flags NotImplementedError stubs left in after implementation. + local dir="${1:-.}" + local label="${2:-$dir}" + local found + found=$(grep -rn "raise NotImplementedError" "$dir" --include="*.py" 2>/dev/null || true) + if [[ -n "$found" ]]; then + fail "$label: raise NotImplementedError still present — remove stubs before submitting" + return 1 + fi + return 0 +} + +check_no_relative_imports() { + # Usage: check_no_relative_imports [label] + # Flags `from .module import x` in scripts not inside a proper package. + # Relative imports break the grader: python3 src/cleaner.py fails with + # "attempted relative import with no known parent package". + local dir="${1:-.}" + local label="${2:-$dir}" + local found + found=$(grep -rn "^from \." "$dir" --include="*.py" 2>/dev/null || true) + if [[ -n "$found" ]]; then + fail "$label: relative import found (from .module) — use absolute: 'from src.module import x'" + return 1 + fi + return 0 +} + +check_no_logging_in_utils() { + # Usage: check_no_logging_in_utils + # utils.py should be pure helpers; logging config belongs in the entry point. + local file="${1:-task-1/src/utils.py}" + if [[ ! -f "$file" ]]; then return 0; fi + if grep -qE "logging\.basicConfig|logging\.getLogger" "$file"; then + warn "$file: logging.basicConfig/getLogger found — logging setup belongs in cleaner.py or the entry-point, not in utils" + return 1 + fi + return 0 +} + +check_gitignore_python() { + # Usage: check_gitignore_python [] + # Warns when Python cache patterns are absent from .gitignore. + local gi="${1:-.gitignore}" + if [[ ! -f "$gi" ]]; then + warn ".gitignore is missing — add one so __pycache__/ and *.pyc are not committed" + return 1 + fi + local ok=true + if ! grep -q "__pycache__" "$gi"; then + warn ".gitignore missing __pycache__/ — Python bytecode cache dirs should not be committed" + ok=false + fi + if ! grep -qE "^\*\.pyc$|^.*\*\.pyc" "$gi"; then + warn ".gitignore missing *.pyc — compiled Python files should not be committed" + ok=false + fi + if ! grep -qE "^\.env$|^\.env\b" "$gi"; then + warn ".gitignore missing .env — secret files should not be committed" + ok=false + fi + if [[ "$ok" = true ]]; then pass ".gitignore correctly excludes __pycache__/, *.pyc, and .env"; fi +} + +check_screenshot_is_png() { + # Usage: check_screenshot_is_png [] + # Awards full credit for .png, warns (and still credits) for .jpg/.jpeg, + # zero for missing. Matches the pattern flagged in c55 PR reviews. + local expected_png="$1" + local dir + dir="$(dirname "$expected_png")" + local base + base="$(basename "$expected_png" .png)" + + if [[ -s "$expected_png" ]]; then + pass "screenshot is $expected_png (.png format ✓)" + return 0 + fi + for ext in jpg jpeg; do + if [[ -s "$dir/$base.$ext" ]]; then + warn "screenshot is .$ext but should be .png — rename to $base.png (partial credit still given)" + return 1 + fi + done + fail "screenshot missing: $expected_png not found" + return 2 +} + +check_silent_zero_in_except() { + # Usage: check_silent_zero_in_except + # Detects the pattern: try: x = compute() / except: x = 0 + # which silently corrupts data instead of skipping or raising. + local file="$1" + if [[ ! -f "$file" ]]; then return 0; fi + local found + found=$(python3 - "$file" 2>/dev/null << 'PY' +import ast, sys +try: + tree = ast.parse(open(sys.argv[1]).read()) +except SyntaxError: + sys.exit(0) +for node in ast.walk(tree): + if isinstance(node, ast.ExceptHandler): + for stmt in node.body: + if isinstance(stmt, ast.Assign): + if isinstance(stmt.value, ast.Constant) and stmt.value.value == 0: + print(f"line {stmt.lineno}: '{ast.unparse(stmt)}' — sets field to 0 in except block (silent data corruption)") +PY +) + if [[ -n "$found" ]]; then + warn "$file: silent 0-assignment in except block — skip the row or raise instead of setting to 0:\n $found" + return 1 + fi + return 0 +} + +check_exception_logged() { + # Usage: check_exception_logged + # Warns when except blocks log/print a message but don't include the + # exception variable (e, err, exc), meaning the error type is lost. + local dir="${1:-.}" + local found + found=$(python3 - "$dir" 2>/dev/null << 'PY' +import ast, os, sys +issues = [] +for root, _, files in os.walk(sys.argv[1]): + for fname in files: + if not fname.endswith(".py"): + continue + path = os.path.join(root, fname) + try: + tree = ast.parse(open(path).read()) + except SyntaxError: + continue + for node in ast.walk(tree): + if not isinstance(node, ast.ExceptHandler): + continue + exc_var = node.name # e.g. "e" in `except ValueError as e` + if not exc_var: + continue + for stmt in node.body: + for call in ast.walk(stmt): + if not isinstance(call, ast.Call): + continue + # Is it a logging.* or print call? + func = call.func + is_log = (isinstance(func, ast.Attribute) and + isinstance(func.value, ast.Name) and + func.value.id == "logging") + is_print = isinstance(func, ast.Name) and func.id == "print" + if not (is_log or is_print): + continue + # Does the call reference the exception variable? + src = ast.unparse(call) + if exc_var not in src: + issues.append(f"{path}:{call.lineno}: log message doesn't include exception variable '{exc_var}' — add it for easier debugging") +if issues: + for i in issues[:3]: # cap at 3 to keep output readable + print(i) +PY +) + if [[ -n "$found" ]]; then + warn "exception variable not included in log message (harder to debug):\n $found" + return 1 + fi + return 0 +} + +check_ruff() { + # Usage: check_ruff [