fix(grader_lib): all check_* functions always return 0 (safe under set -e)

Bare calls to check_no_notimplemented/check_no_relative_imports etc.
were causing the script to exit under set -euo pipefail when they returned 1.
Removed all 'return 1' from check functions — results go through
_grader_details, not exit codes. Verified: scaffold 20/100 fail,
solution 100/100 pass.

Author: Lasse Benninga

.hyf/grader_lib.sh

@@ -3,9 +3,9 @@
 # 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.
+# Design rule: every check_* function always returns 0.
+# Results are communicated via pass()/fail()/warn() into _grader_details,
+# never via exit code. This makes bare calls safe under `set -euo pipefail`.
 
 _grader_details=()
 
@@ -39,8 +39,8 @@ JSON
 }
 
 # ── 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.
+# All check_* functions always return 0. Findings are recorded via
+# pass()/fail()/warn() and printed by print_results().
 
 check_no_print_statements() {
   # Usage: check_no_print_statements <dir> [label]
@@ -53,7 +53,6 @@ check_no_print_statements() {
     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
 }
@@ -67,7 +66,6 @@ check_no_notimplemented() {
   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
 }
@@ -83,7 +81,6 @@ check_no_relative_imports() {
   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
 }
@@ -95,7 +92,6 @@ check_no_logging_in_utils() {
   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
 }
@@ -106,7 +102,7 @@ check_gitignore_python() {
   local gi="${1:-.gitignore}"
   if [[ ! -f "$gi" ]]; then
     warn ".gitignore is missing — add one so __pycache__/ and *.pyc are not committed"
-    return 1
+    return 0
   fi
   local ok=true
   if ! grep -q "__pycache__" "$gi"; then
@@ -122,36 +118,34 @@ check_gitignore_python() {
     ok=false
   fi
   if [[ "$ok" = true ]]; then pass ".gitignore correctly excludes __pycache__/, *.pyc, and .env"; fi
+  return 0
 }
 
 check_screenshot_is_png() {
-  # Usage: check_screenshot_is_png <expected_path> [<wrong_ext_glob>]
-  # Awards full credit for .png, warns (and still credits) for .jpg/.jpeg,
-  # zero for missing. Matches the pattern flagged in c55 PR reviews.
+  # Usage: check_screenshot_is_png <expected_path>
+  # Awards full credit for .png, warns for .jpg/.jpeg, fails if missing.
   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
+      return 0
     fi
   done
   fail "screenshot missing: $expected_png not found"
-  return 2
+  return 0
 }
 
 check_silent_zero_in_except() {
   # Usage: check_silent_zero_in_except <file>
-  # Detects the pattern: try: x = compute() / except: x = 0
-  # which silently corrupts data instead of skipping or raising.
+  # Detects: try: x = compute() / except: x = 0 (silent data corruption).
   local file="$1"
   if [[ ! -f "$file" ]]; then return 0; fi
   local found
@@ -170,16 +164,14 @@ for node in ast.walk(tree):
 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
+    warn "$file: silent 0-assignment in except block — skip the row or raise instead:\n    $found"
   fi
   return 0
 }
 
 check_exception_logged() {
   # Usage: check_exception_logged <dir>
-  # Warns when except blocks log/print a message but don't include the
-  # exception variable (e, err, exc), meaning the error type is lost.
+  # Warns when an except block logs a message but omits the exception variable.
   local dir="${1:-.}"
   local found
   found=$(python3 - "$dir" 2>/dev/null << 'PY'
@@ -197,54 +189,50 @@ for root, _, files in os.walk(sys.argv[1]):
         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`
+            exc_var = node.name
             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")
+                        issues.append(f"{path}:{call.lineno}: log message doesn't include '{exc_var}' — add it for easier debugging")
 if issues:
-    for i in issues[:3]:  # cap at 3 to keep output readable
+    for i in issues[:3]:
         print(i)
 PY
 )
   if [[ -n "$found" ]]; then
-    warn "exception variable not included in log message (harder to debug):\n    $found"
-    return 1
+    warn "exception variable missing from log message:\n    $found"
   fi
   return 0
 }
 
 check_ruff() {
   # Usage: check_ruff <dir> [<select>]
-  # Runs ruff on <dir> if available; warns on violations.
-  # Default select: F401 (unused imports), E302 (missing blank lines).
+  # Runs ruff if available; warns on F401 (unused imports) and E302/E303 (blank lines).
   local dir="${1:-.}"
   local select="${2:-F401,E302,E303}"
   if ! command -v ruff &>/dev/null && ! python3 -m ruff --version &>/dev/null 2>&1; then
-    return 0  # ruff not installed — skip silently
+    return 0
   fi
   local out
   out=$(python3 -m ruff check --select="$select" --output-format=text "$dir" 2>/dev/null || true)
   if [[ -n "$out" ]]; then
     local count
     count=$(echo "$out" | grep -c "\.py:" || true)
     warn "$dir: ruff found $count style issue(s) (unused imports / missing blank lines) — run 'ruff check $dir' to see details"
-    return 1
+  else
+    pass "$dir: no ruff style issues (F401/E302/E303)"
   fi
-  pass "$dir: no ruff style issues (F401/E302/E303)"
   return 0
 }