Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions .github/scripts/assignment1-feedback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Assignment 1 execution feedback

Assignment 1 runs separately in `sql_assignment1_runner.yml`. The existing
`sql_assignment_runner.yml`, `grade_assignments.py`, and Assignment 2 tests are
unchanged. A1 uses `a1-test-results.json` / `a1-grading-results`, so it cannot
replace A2's report or artifact.

## Historical source and scope

The implementation restores the behavior of `tests/test_assignment.py` immediately
before `3fe0023c0af279b275f746948ad08134a631a25f` (October 9, 2026).
The March 24 runner (`cf333a0`) evolved through the March 27 parser changes
(`0349f73`) and April 1 multistatement support (`3445382`). By April 5
(`f7e32c2`, `ae700d8`), the workflow was manual-only with an explicit PR number.
The old workflow selected A2 in preference to A1 when both changed. The restoration
runs A1 independently, so an A2 submission no longer suppresses A1 feedback.

The historical runner only executed SQL and displayed up to three rows. This
restoration likewise does **not** judge answer correctness or award marks.
Feedback explicitly distinguishes executed SQL, missing answers, and SQL errors.
All 12 A1 questions are represented. Instructors still mark answers and written work.

The restored parser accepts `--QUERY 1` and `-- QUERY 1`, and closes blocks with
`--END QUERY` or the historical `--END STATEMENT`. It rejects malformed, duplicate,
unclosed, and out-of-range markers. Multistatement SQL uses SQLite statement
boundaries, preserving semicolons in strings and comments. Queries share a disposable
in-memory copy, preserving temporary tables between blocks; the course database is
opened read-only, external database attachment is denied, and queries have a
10-second execution limit plus a five-minute workflow limit.

## Automatic and manual use

On student repositories (owner other than `UofT-DSI`), A1 runs for PRs targeting
`main` from `assignment-one`, `assignment-1`, `assignment-two`, or `assignment-2`
when an `assignment1.sql` file changes. A mixed A1/A2 PR on an A2 branch runs both
workflows. Each runner selects its own assignment file and rejects multiple cohorts'
files rather than guessing. Existing A2 branch and owner rules remain unchanged:
A2 does not automatically run on an A1 branch.

For the upstream `UofT-DSI` repository, or when automatic PR commenting is restricted
by GitHub token permissions, use the A1 workflow's **Run workflow** action. Choose
a workflow ref containing this restoration, enter the submission branch and target
PR number. The submission branch must also contain the restored runner and tests.
The workflow compares the branch against `origin/main`; exactly one changed
`assignment1.sql` is required. A branch already merged into main has no changed
submission to select. The JSON artifact is retained even if posting feedback fails.

Use **SQL Unit Tests** for A2's existing manual workflow. No privileged
`pull_request_target` trigger is added.

## Local validation

From the repository root, after installing `.github/scripts/requirements.txt`:

```sh
python -m pytest 03_instructional_team/tests/test_assignment1_regressions.py 03_instructional_team/tests/test_grader_regressions.py -q
```

The new tests exercise SQL execution (including an intentionally wrong but executable
answer), missing/invalid submissions, multi-statement state, bounded result previews,
database isolation, query timeout, and CLI selection from mixed A1/A2 changes.
A2's existing private-rubric tests require `A2_SOLUTION_RUBRIC`; without that input
pytest reports them as skipped.
116 changes: 116 additions & 0 deletions .github/scripts/grade_assignment1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
"""Restore A1 execution feedback from tests/test_assignment.py at 3fe0023^.

As in the historical runner, this executes SQL and shows up to three rows;
it does not award marks or establish answer correctness.
"""
import json
from pathlib import Path
import re
import sqlite3
import sys
import time

DATABASE = Path("05_src/sql/farmersmarket.db")


def load_queries(sql_file):
"""Read the historical QUERY / END QUERY (or END STATEMENT) blocks."""
queries = {}
number = None
buffer = []
for line in Path(sql_file).read_text().splitlines():
stripped = line.strip()
start = re.fullmatch(r"--\s*QUERY\s+(\d+)\s*", stripped, re.I)
end = re.fullmatch(r"--\s*END\s+(?:QUERY|STATEMENT)\s*", stripped, re.I)
if start:
if number is not None:
raise ValueError("Nested or unclosed QUERY block.")
number = int(start[1])
if number not in range(1, 13) or number in queries:
raise ValueError(f"Unknown or duplicate QUERY number: {number}.")
buffer = []
elif end:
if number is None:
raise ValueError("END marker has no matching QUERY marker.")
queries[number] = "\n".join(buffer).strip()
number = None
elif re.match(r"--\s*(QUERY|END\s+(?:QUERY|STATEMENT))\b", stripped, re.I):
raise ValueError(f"Invalid answer marker: {stripped}")
elif number is not None:
buffer.append(line)
if number is not None:
raise ValueError(f"QUERY {number} has no END marker.")
return [{"number": n, "query": queries.get(n, "")} for n in range(1, 13)]


def has_sql(query):
return bool(re.sub(r"--[^\n]*|/\*[\s\S]*?\*/|[\s;]+", "", query))


def run_query(conn, query):
"""Keep multistatement support without splitting quoted semicolons."""
deadline = time.monotonic() + 10
conn.set_progress_handler(lambda: int(time.monotonic() > deadline), 1000)
cursor = conn.cursor()
buffer = ""
try:
for character in query:
buffer += character
if character == ";" and sqlite3.complete_statement(buffer):
if has_sql(buffer):
cursor.execute(buffer)
buffer = ""
if has_sql(buffer):
cursor.execute(buffer)
return [dict(row) for row in cursor.fetchmany(3)]
finally:
conn.set_progress_handler(None, 0)


def run_assignment(file_path):
"""Execute on a disposable database, retaining state between A1 blocks."""
queries = load_queries(file_path)
conn = sqlite3.connect(":memory:")
conn.row_factory = sqlite3.Row
try:
with sqlite3.connect(f"file:{DATABASE}?mode=ro", uri=True) as source:
source.backup(conn)
conn.set_authorizer(lambda action, *_: sqlite3.SQLITE_DENY
if action in (sqlite3.SQLITE_ATTACH, sqlite3.SQLITE_DETACH)
else sqlite3.SQLITE_OK)
results = []
for parsed in queries:
result = dict(parsed, result=[], error=None)
if not has_sql(parsed["query"]):
result.update(status="missing", feedback="No SQL answer supplied.")
else:
try:
result["result"] = run_query(conn, parsed["query"])
result.update(status="executed", feedback="SQL executed. Correctness requires instructor review.")
except Exception as error:
result.update(status="error", feedback="SQL could not execute.", error=str(error))
results.append(result)
return results
finally:
conn.close()


def grade_files(files, output):
"""Require one cohort's A1 file and always replace stale reports."""
report = {"file": "Submission selection", "results": [], "runner_error": None}
try:
if len(files) != 1:
raise ValueError("No changed assignment1.sql file was found." if not files else
"More than one assignment1.sql file changed. Submit only your cohort's file.")
report["file"] = files[0]
report["results"] = run_assignment(files[0])
except Exception as error:
report["runner_error"] = str(error)
Path(output).write_text(json.dumps([report], indent=2))
return int(report["runner_error"] is not None)


if __name__ == "__main__":
changed = Path(sys.argv[1]).read_bytes().decode().split("\0")
files = sorted({path for path in changed if Path(path).name == "assignment1.sql"})
sys.exit(grade_files(files, sys.argv[2]))
143 changes: 143 additions & 0 deletions .github/workflows/sql_assignment1_runner.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
name: SQL Assignment 1 Execution Feedback

on:
pull_request:
branches:
- main
types: [opened, reopened, synchronize]
paths:
- '**/assignment1.sql'
workflow_dispatch: # Manual trigger from GitHub UI
inputs:
branch:
description: 'Assignment 1 branch to run, usually assignment-one'
required: true
default: assignment-one
type: string
pr_number:
description: 'Pull request number. Extract a number from the end of pull request url, i.e. https://github.com/khsergvl/sql/pull/17'
required: true
type: string
default: '1'
permissions:
contents: read
pull-requests: write

jobs:
run-assignment-queries:
if: >-
github.event_name == 'workflow_dispatch' ||
((github.head_ref == 'assignment-one' || github.head_ref == 'assignment-1' ||
github.head_ref == 'assignment-two' || github.head_ref == 'assignment-2') &&
github.repository_owner != 'UofT-DSI')
runs-on: ubuntu-latest
timeout-minutes: 5
permissions:
contents: read
pull-requests: write

steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'workflow_dispatch' && inputs.branch || github.event.pull_request.head.sha }}
fetch-depth: 0

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install dependencies
run: |
pip install -r ./.github/scripts/requirements.txt

- name: Verify grader regressions
run: python -m pytest 03_instructional_team/tests/test_assignment1_regressions.py -q

- name: Get changed files
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
run: |
base_ref="${BASE_SHA:-origin/main}"
git diff --name-only -z "$base_ref"...HEAD > changed_files.txt

- name: Grade changed assignments
id: grading
continue-on-error: true
run: python .github/scripts/grade_assignment1.py changed_files.txt a1-test-results.json

- name: Post grading feedback to PR
if: always() && steps.grading.outcome != 'skipped'
uses: actions/github-script@v7
env:
PR_NUMBER: ${{ github.event_name == 'workflow_dispatch' && inputs.pr_number || github.event.pull_request.number }}
with:
script: |
const fs = require('fs');
const labels = {
executed: '⚪ SQL executed; instructor review needed', error: '❌ Error',
missing: '❌ Missing answer', blocked: '⚪ Cannot assess: prerequisite failed',
manual_review: '⚪ Instructor review needed'
};
function fenced(value, language = '') {
const text = String(value);
const runs = text.match(/`+/g) || [];
const fence = '`'.repeat(Math.max(3, ...runs.map(run => run.length + 1)));
return `${fence}${language}\n${text}\n${fence}\n`;
}
let reports;
try {
reports = JSON.parse(fs.readFileSync('a1-test-results.json', 'utf8'));
if (!Array.isArray(reports)) throw new Error('Invalid report format');
} catch (error) {
reports = [{file: 'Grading runner', results: [], runner_error: error.message}];
}
if (!reports.length) return;
let body = '### SQL Assignment 1 execution feedback\n\n';
body += 'Restored A1 execution checks: up to three result rows per query. Successful execution does not establish correctness or award marks; instructor review is required.\n\n';
const truncationNotice = '\nFeedback truncated; inspect the workflow report for remaining results.';
const maxBodyLength = 60000;
let truncated = false;
function appendSection(section) {
if (body.length + section.length + truncationNotice.length > maxBodyLength) {
truncated = true;
return false;
}
body += section;
return true;
}
reportLoop:
for (const report of reports) {
let reportSection = fenced(report.file);
if (report.runner_error) reportSection += `Grading could not finish: ${report.runner_error}\n\n`;
if (!appendSection(reportSection)) break;
for (const result of report.results) {
let resultSection = `**Query ${result.number}: ${labels[result.status] || '⚪ Instructor review needed'}**\n\n`;
resultSection += `${result.feedback}\n\n`;
if (result.error) resultSection += fenced(result.error);
if (result.query) resultSection += fenced(result.query, 'sql');
if (result.result?.length) resultSection += fenced(JSON.stringify(result.result, null, 2), 'json');
resultSection += '\n';
if (!appendSection(resultSection)) break reportLoop;
}
}
if (truncated) body += truncationNotice;
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: Number(process.env.PR_NUMBER),
body
});

- name: Upload grading reports
if: always()
uses: actions/upload-artifact@v4
with:
name: a1-grading-results
path: a1-test-results.json
if-no-files-found: ignore

- name: Preserve runner failure status
if: always() && steps.grading.outcome == 'failure'
run: exit 1
Loading
Loading