From c5d8107ec4edf9755b910b80d21882e9f1a51923 Mon Sep 17 00:00:00 2001 From: Acture Date: Fri, 2 Oct 2026 23:50:42 +0800 Subject: [PATCH 1/2] feat: add teacher-configurable submission matching (P-673) --- README.md | 7 + crates/scriptmark-py/src/lib.rs | 39 +- crates/scriptmark/src/assignment.rs | 3 + crates/scriptmark/src/discovery.rs | 111 +++- crates/scriptmark/src/lib.rs | 1 + crates/scriptmark/src/main.rs | 113 +++- crates/scriptmark/src/matching.py | 117 ++++ crates/scriptmark/src/matching.rs | 585 ++++++++++++++++++ crates/scriptmark/src/models/config.rs | 2 + crates/scriptmark/src/models/result.rs | 7 + crates/scriptmark/src/models/submission.rs | 8 + crates/scriptmark/src/runner/executor.rs | 13 +- crates/scriptmark/src/runner/harness.py | 75 ++- crates/scriptmark/src/runner/judge.rs | 20 + crates/scriptmark/src/runner/oracle.rs | 1 + crates/scriptmark/src/runner/orchestrator.rs | 116 +++- crates/scriptmark/src/runner/prepare.rs | 1 + crates/scriptmark/src/runner/python.rs | 111 +--- crates/scriptmark/tests/harness.rs | 1 + crates/scriptmark/tests/input_equivalence.rs | 1 + crates/scriptmark/tests/matching.rs | 415 +++++++++++++ examples/bundles/matching/assignment.toml | 11 + .../submissions/alice/deep/solution.py | 2 + .../bundles/matching/tests/test_double.toml | 15 + 24 files changed, 1587 insertions(+), 188 deletions(-) create mode 100644 crates/scriptmark/src/matching.py create mode 100644 crates/scriptmark/src/matching.rs create mode 100644 crates/scriptmark/tests/matching.rs create mode 100644 examples/bundles/matching/assignment.toml create mode 100644 examples/bundles/matching/submissions/alice/deep/solution.py create mode 100644 examples/bundles/matching/tests/test_double.toml diff --git a/README.md b/README.md index da28858..9b53610 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,9 @@ scriptmark grade submissions/ -t tests/ -r roster.csv --db grades.db -a archive/ # Run tests only (raw JSON output) scriptmark run submissions/ -t tests/ -o results.json +# Preview student/file/function matching; edit assignment.toml to resolve candidates +scriptmark match submissions/ -t tests/ -o output/matches.json + # Inputs and oracle answers are frozen beside the results (output/results.cases.json); # grade late submissions on the same tests and answers, into results of their own scriptmark grade late/ -t tests/ --replay output/results.cases.json -o output/late.json @@ -172,6 +175,10 @@ print(spec.name, spec.function, spec.num_cases) ## TOML Test Specs +Teachers can configure student ownership, item file patterns, function aliases and +per-student overrides in `assignment.toml`. Preview decisions and candidates with +`scriptmark match`; unresolved conflicts withhold grades. See [matching rules](docs/matching.md). + ```toml [meta] name = "find_max" diff --git a/crates/scriptmark-py/src/lib.rs b/crates/scriptmark-py/src/lib.rs index b988581..0dc1b27 100644 --- a/crates/scriptmark-py/src/lib.rs +++ b/crates/scriptmark-py/src/lib.rs @@ -288,26 +288,32 @@ impl Freezing<'_> { /// `freeze` writes generated inputs and oracle answers; `replay` verifies and reuses /// a frozen bundle without recomputing its answers. #[pyfunction] -#[pyo3(signature = (submissions, tests, *, timeout=10, python="python3", freeze=None, replay=None))] +#[pyo3(signature = (submissions, tests, *, timeout=10, python="python3", assignment=None, freeze=None, replay=None))] fn run( - py: Python<'_>, submissions: Vec, tests: String, timeout: u64, python: &str, + assignment: Option, freeze: Option, replay: Option, ) -> PyResult { let specs = load_specs_from_dir(Path::new(&tests)).map_err(spec_error)?; + let mut declared = + scriptmark::assignment::load(assignment.as_deref().map(Path::new), Path::new(&tests)) + .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string()))?; + scriptmark::assignment::settle(&mut declared.assignment, &declared.grading, &specs) + .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string()))?; let freezing = Freezing { freeze: freeze.as_deref(), replay: replay.as_deref(), }; - let (results, inputs) = run_grading(&submissions, specs, timeout, python, &freezing)?; + let (results, inputs) = + run_grading(&submissions, specs, timeout, python, &freezing, &declared)?; freezing.save(&inputs)?; let json_val = serde_json::to_value(&results) .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string()))?; - json_to_py(py, &json_val) + Python::with_gil(|py| json_to_py(py, &json_val)) } /// Grade all students: run the tests, then score each item under the assignment's @@ -341,7 +347,8 @@ fn grade( freeze: freeze.as_deref(), replay: replay.as_deref(), }; - let (mut reports, inputs) = run_grading(&submissions, specs, timeout, python, &freezing)?; + let (mut reports, inputs) = + run_grading(&submissions, specs, timeout, python, &freezing, &declared)?; grade_all(&mut reports, &declared.assignment.items, &policy).map_err(value_error)?; freezing.save(&inputs)?; @@ -364,7 +371,12 @@ fn run_grading( timeout: u64, python: &str, freezing: &Freezing, + declared: &scriptmark::assignment::Declared, ) -> PyResult<(Vec, Frozen)> { + declared + .matching + .validate(&specs) + .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string()))?; let generation = match freezing.replay { Some(path) => Generation::Replay(Frozen::load(Path::new(path)).map_err(frozen_error)?), None => Generation::fresh(), @@ -381,10 +393,25 @@ fn run_grading( frozen::writable(Path::new(path)) .map_err(|e| pyo3::exceptions::PyOSError::new_err(format!("{path}: {e}")))?; } - let input = local_input(submissions)?; + let paths: Vec<&Path> = submissions.iter().map(|p| Path::new(p.as_str())).collect(); + let input = load_local_input( + &paths, + LocalInputOptions { + assignment: declared.assignment.clone(), + attempt_policy: declared.attempt_policy, + matching: Some(&declared.matching), + ..Default::default() + }, + ) + .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string()))?; + let errors: Vec = input.errors().map(ToString::to_string).collect(); + if !errors.is_empty() { + return Err(pyo3::exceptions::PyValueError::new_err(errors.join("; "))); + } let executor = Arc::new(PythonExecutor::with_python_cmd(python)); let options = RunOptions { + matching: declared.matching.clone(), concurrency: None, python: executor.python_cmd().to_string(), }; diff --git a/crates/scriptmark/src/assignment.rs b/crates/scriptmark/src/assignment.rs index fd6bcd1..b26dd9b 100644 --- a/crates/scriptmark/src/assignment.rs +++ b/crates/scriptmark/src/assignment.rs @@ -15,6 +15,7 @@ pub struct Declared { pub assignment: Assignment, pub attempt_policy: AttemptPolicy, pub grading: GradingConfig, + pub matching: crate::matching::Config, /// Where it was read from; `None` when there was no file. pub path: Option, } @@ -44,6 +45,7 @@ pub fn load(explicit: Option<&Path>, tests_dir: &Path) -> Result { assignment: Assignment::named(name), attempt_policy: AttemptPolicy::default(), grading: GradingConfig::default(), + matching: crate::matching::Config::default(), path: None, }); }; @@ -59,6 +61,7 @@ pub fn load(explicit: Option<&Path>, tests_dir: &Path) -> Result { }, attempt_policy: config.assignment.attempt_policy, grading: config.grading, + matching: config.matching, path: Some(path), }) } diff --git a/crates/scriptmark/src/discovery.rs b/crates/scriptmark/src/discovery.rs index 02fb153..3ffe805 100644 --- a/crates/scriptmark/src/discovery.rs +++ b/crates/scriptmark/src/discovery.rs @@ -40,7 +40,7 @@ pub(crate) fn detect_language(ext: &str) -> Option<&'static str> { /// Extract a student key from a filename. /// /// Convention: `{key}_{rest}.ext` (e.g. `alice_Lab5_1.py` → `alice`). This split is a -/// placeholder — teacher-configurable matching is P-673 — so the key it yields is only +/// source default; the key it yields is only /// ever an unconfirmed [`crate::models::StudentKey::Extracted`] until a roster vouches /// for it. fn extract_sid(filename: &str) -> Option { @@ -112,6 +112,7 @@ pub struct LocalInputOptions<'a> { /// The roster of record. Without one, membership is unknown rather than negative. pub roster: Option<&'a Roster>, pub attempt_policy: AttemptPolicy, + pub matching: Option<&'a crate::matching::Config>, } impl Default for LocalInputOptions<'_> { @@ -120,6 +121,7 @@ impl Default for LocalInputOptions<'_> { assignment: Assignment::default(), roster: None, attempt_policy: AttemptPolicy::Latest, + matching: None, } } } @@ -139,6 +141,11 @@ pub fn load_local_input( let mut by_key: BTreeMap> = BTreeMap::new(); // A student who sent only unusable files still submitted something. let mut seen_keys: std::collections::BTreeSet = Default::default(); + let default_matching = crate::matching::Config::default(); + let matching = options.matching.unwrap_or(&default_matching); + matching + .validate_owners() + .map_err(|e| DiscoveryError::Matching(e.to_string()))?; for path in paths { let path = path.as_ref(); @@ -166,11 +173,54 @@ pub fn load_local_input( } } - let all_paths: Vec = paths + let mut all_paths: Vec = paths .iter() .map(|p| p.as_ref().to_path_buf()) .chain(extra_dirs) .collect(); + // Walk nested submissions; never revisit the extraction cache or directory symlinks. + let mut index = 0; + while index < all_paths.len() { + let dir = &all_paths[index]; + let mut children = Vec::new(); + for entry in std::fs::read_dir(dir).map_err(|e| DiscoveryError::IoError(dir.clone(), e))? { + let entry = entry.map_err(|e| DiscoveryError::IoError(dir.clone(), e))?; + if entry + .file_type() + .map_err(|e| DiscoveryError::IoError(dir.clone(), e))? + .is_dir() && !crate::archive::is_noise(&entry.file_name().to_string_lossy()) + && entry.file_name() != EXTRACT_DIR + { + children.push(entry.path()); + } + } + children.sort(); + for child in children { + if !all_paths.contains(&child) { + if !child + .components() + .any(|part| part.as_os_str() == EXTRACT_DIR) + { + for extracted in extract_archives(&child, &mut diagnostics) { + if let Some(parent) = extracted.out_path.parent() + && !all_paths.contains(&parent.to_path_buf()) + { + all_paths.push(parent.to_path_buf()); + } + origins.insert( + extracted.out_path, + FileOrigin::Archive { + archive: extracted.archive, + entry: extracted.entry, + }, + ); + } + } + all_paths.push(child); + } + } + index += 1; + } for dir_path in &all_paths { let entries = std::fs::read_dir(dir_path) @@ -219,11 +269,39 @@ pub fn load_local_input( .and_then(|e| e.to_str()) .unwrap_or("") .to_lowercase(); - // Archives are inputs to extraction, not submissions in their own right — but - // the upload still happened. Registering the owner here is what stops a - // truncated or empty archive being reported as 缺交. + // Inside an extraction directory the archive stem carries the key; the files + // within it are named by the student. + let default_key = if let Some(FileOrigin::Archive { archive, .. }) = origins.get(&path) + { + archive + .file_name() + .and_then(|n| n.to_str()) + .and_then(extract_sid) + } else { + extract_sid(filename) + }; + let owner_path = match origins.get(&path) { + Some(FileOrigin::Archive { archive, .. }) => archive, + _ => &path, + }; + let relative = paths + .iter() + .filter_map(|root| owner_path.strip_prefix(root.as_ref()).ok()) + .min_by_key(|p| p.components().count()) + .unwrap_or(owner_path); + let owner = matching + .owner(relative, owner_path, default_key) + .map_err(|e| DiscoveryError::Matching(e.to_string()))?; + if owner.state == crate::matching::State::Ambiguous { + diagnostics.push(InputDiagnostic::error(DiagnosticKind::AmbiguousOwner { + path: path.clone(), + decision: owner.clone(), + })); + } + let key = owner.selected.clone(); + // Even an empty/unreadable archive records receipt under the teacher's owner rule. if !is_extracted && crate::archive::format_of(&path).is_some() { - match extract_sid(filename) { + match key { Some(key) => { seen_keys.insert(key); } @@ -235,28 +313,15 @@ pub fn load_local_input( continue; } - // Inside an extraction directory the archive stem carries the key; the files - // within it are named by the student. - let key = if is_extracted { - dir_path - .file_name() - .and_then(|n| n.to_str()) - .and_then(extract_sid) - .or_else(|| extract_sid(filename)) - } else { - extract_sid(filename) - }; - let language = detect_language(&ext); match (key, language) { (Some(key), Some(language)) => { seen_keys.insert(key.clone()); let origin = origins.get(&path).cloned().unwrap_or(FileOrigin::Direct); - by_key - .entry(key) - .or_default() - .push(StudentFile::direct(path.clone(), language).with_origin(origin)); + let mut file = StudentFile::direct(path.clone(), language).with_origin(origin); + file.owner = Some(owner); + by_key.entry(key).or_default().push(file); } (Some(key), None) => { // Owner known, type unusable: the student submitted, just not code. @@ -375,6 +440,8 @@ pub fn load_local_input( #[derive(Debug, thiserror::Error)] pub enum DiscoveryError { + #[error("invalid matching rules: {0}")] + Matching(String), #[error("not a directory: {0}")] NotADirectory(std::path::PathBuf), #[error("IO error reading {0}: {1}")] diff --git a/crates/scriptmark/src/lib.rs b/crates/scriptmark/src/lib.rs index e6f3768..5651d19 100644 --- a/crates/scriptmark/src/lib.rs +++ b/crates/scriptmark/src/lib.rs @@ -6,6 +6,7 @@ pub mod discovery; pub mod export; pub mod grading; pub mod input; +pub mod matching; pub mod roster; pub mod similarity; pub mod spec_loader; diff --git a/crates/scriptmark/src/main.rs b/crates/scriptmark/src/main.rs index 136f9f1..3e30ce1 100644 --- a/crates/scriptmark/src/main.rs +++ b/crates/scriptmark/src/main.rs @@ -37,6 +37,8 @@ enum Commands { Grade(GradeArgs), /// Run tests only, output raw results to JSON Run(RunArgs), + /// Preview student, file and function matching without running student programs + Match(MatchArgs), /// Summarize existing results (re-analyze without re-running) Summarize(SummarizeArgs), /// Canvas LMS: browse courses, and fetch an assignment for grading @@ -175,6 +177,24 @@ struct RunArgs { frozen: FrozenArgs, } +#[derive(Parser)] +struct MatchArgs { + #[arg(required_unless_present = "canvas", conflicts_with = "canvas")] + submissions: Vec, + #[arg(long)] + canvas: Option, + #[arg(short = 't', long = "tests")] + tests_dir: PathBuf, + #[arg(short, long, default_value = "output/matches.json")] + output: PathBuf, + #[arg(short, long)] + roster: Option, + #[arg(long)] + assignment: Option, + #[arg(long, default_value = "python3")] + python: String, +} + #[derive(Parser)] struct SummarizeArgs { /// Path to results JSON file, as `grade` wrote it @@ -360,6 +380,7 @@ fn build_local_input( submissions: &[PathBuf], declared: &Declared, roster_path: Option<&PathBuf>, + preview: bool, ) -> Result { let (assignment, attempt_policy) = (declared.assignment.clone(), declared.attempt_policy); @@ -374,6 +395,7 @@ fn build_local_input( assignment, roster: roster.as_ref(), attempt_policy, + matching: Some(&declared.matching), }, ) .context("Failed to discover submissions")?; @@ -384,7 +406,7 @@ fn build_local_input( // itself about who a 学号 belongs to would attribute somebody's work to the wrong name. // Stop before running anything rather than producing results nobody should act on. let errors: Vec = input.errors().map(|d| d.to_string()).collect(); - if !errors.is_empty() { + if !preview && !errors.is_empty() { anyhow::bail!( "refusing to grade: {} problem(s) with the input\n {}", errors.len(), @@ -478,6 +500,7 @@ async fn main() -> Result<()> { match cli.command { Commands::Grade(args) => cmd_grade(args).await, Commands::Run(args) => cmd_run(args).await, + Commands::Match(args) => cmd_match(args), Commands::Summarize(args) => cmd_summarize(args), Commands::Canvas(cmd) => cmd_canvas(cmd).await, Commands::RosterPull(args) => cmd_roster_pull(args).await, @@ -515,9 +538,8 @@ fn label(value: Option) -> String { async fn run_bundles( students: &[StudentSubmission], specs: Vec, - python: &str, + mut run_options: RunOptions, timeout: u64, - concurrency: Option, output: &Path, options: &FrozenArgs, ) -> Result<(Vec, Frozen)> { @@ -525,7 +547,7 @@ async fn run_bundles( Some(path) => Generation::Replay(Frozen::load(path)?), None => Generation::fresh(), }; - let executor = Arc::new(PythonExecutor::with_python_cmd(python)); + let executor = Arc::new(PythonExecutor::with_python_cmd(&run_options.python)); let bundles = prepare(specs, &generation, executor.clone(), timeout) .await .context("refusing to grade: the test bundle is not ready")?; @@ -552,10 +574,7 @@ async fn run_bundles( } } } - let run_options = RunOptions { - concurrency: concurrency.map(|n| usize::try_from(n).unwrap_or(usize::MAX)), - python: executor.python_cmd().to_string(), - }; + run_options.python = executor.python_cmd().to_string(); // Units run in their own process groups, so the terminal's Ctrl-C reaches only the // grader: take them down with it rather than leave them running to their timeouts. tokio::select! { @@ -591,6 +610,7 @@ struct Batch { input: AssignmentInput, specs: Vec, policy: Policy, + matching: scriptmark::matching::Config, } /// Load the assignment and the specs, settle the items and the policy against each other, @@ -601,12 +621,14 @@ fn prepare_batch( tests_dir: &std::path::Path, assignment_path: Option<&PathBuf>, roster: Option<&PathBuf>, + preview: bool, ) -> Result { let mut declared = assignment::load(assignment_path.map(PathBuf::as_path), tests_dir)?; let specs = load_specs_from_dir(tests_dir).context("Failed to load test specifications")?; println!("Loaded {} test specs", specs.len()); let policy = assignment::settle(&mut declared.assignment, &declared.grading, &specs)?; + declared.matching.validate(&specs)?; if policy.derived_items() { eprintln!( " note: no [[items]] declared; each spec is an item worth 1 point. To weight \ @@ -618,13 +640,14 @@ fn prepare_batch( // Names, roster membership and submission state all come from the model, so there is // no separate roster merge afterwards. let input = match canvas { - Some(bundle) => build_canvas_input(bundle, &declared)?, - None => build_local_input(submissions, &declared, roster)?, + Some(bundle) => build_canvas_input(bundle, &declared, preview)?, + None => build_local_input(submissions, &declared, roster, preview)?, }; Ok(Batch { input, specs, policy, + matching: declared.matching, }) } @@ -633,20 +656,27 @@ async fn cmd_grade(args: GradeArgs) -> Result<()> { input, specs, policy, + matching, } = prepare_batch( &args.submissions, args.canvas.as_ref(), &args.tests_dir, args.assignment.as_ref(), args.roster.as_ref(), + false, )?; let (mut reports, inputs) = run_bundles( &input.students, specs, - &args.python, + RunOptions { + python: args.python, + matching, + concurrency: args + .concurrency + .map(|n| usize::try_from(n).unwrap_or(usize::MAX)), + }, args.timeout, - args.concurrency, &args.output, &args.frozen, ) @@ -811,21 +841,32 @@ async fn cmd_grade(args: GradeArgs) -> Result<()> { async fn cmd_run(args: RunArgs) -> Result<()> { // The policy is settled even though nothing is scored: a run whose results cannot be // graded should say so now, not after the class has run. - let Batch { input, specs, .. } = prepare_batch( + let Batch { + input, + specs, + matching, + .. + } = prepare_batch( &args.submissions, args.canvas.as_ref(), &args.tests_dir, args.assignment.as_ref(), args.roster.as_ref(), + false, )?; // A JSON array, the same shape `grade` writes; unscored until graded. let (results, inputs) = run_bundles( &input.students, specs, - &args.python, + RunOptions { + python: args.python, + matching, + concurrency: args + .concurrency + .map(|n| usize::try_from(n).unwrap_or(usize::MAX)), + }, args.timeout, - args.concurrency, &args.output, &args.frozen, ) @@ -842,6 +883,34 @@ async fn cmd_run(args: RunArgs) -> Result<()> { Ok(()) } +fn cmd_match(args: MatchArgs) -> Result<()> { + let Batch { + input, + specs, + matching, + .. + } = prepare_batch( + &args.submissions, + args.canvas.as_ref(), + &args.tests_dir, + args.assignment.as_ref(), + args.roster.as_ref(), + true, + )?; + let preview = scriptmark::matching::preview(input, &specs, &matching, &args.python)?; + if let Some(parent) = args.output.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(&args.output, serde_json::to_string_pretty(&preview)?)?; + println!( + "Matching preview saved to {}: {} item(s) pending review, {} unattributed file(s)", + args.output.display(), + preview.pending.len(), + preview.input.unmatched.len() + ); + Ok(()) +} + fn cmd_summarize(args: SummarizeArgs) -> Result<()> { let content = std::fs::read_to_string(&args.results).context("Failed to read results file")?; let mut reports = parse_results(&content)?; @@ -880,7 +949,11 @@ fn cmd_summarize(args: SummarizeArgs) -> Result<()> { /// ids only where the toml left them unset, and a genuine disagreement is refused rather /// than resolved — grading one assignment's submissions against another's declaration is /// not something a warning covers. -fn build_canvas_input(bundle: &std::path::Path, declared: &Declared) -> Result { +fn build_canvas_input( + bundle: &std::path::Path, + declared: &Declared, + preview: bool, +) -> Result { use scriptmark::canvas::bundle; let (assignment, attempt_policy) = (declared.assignment.clone(), declared.attempt_policy); @@ -917,7 +990,7 @@ fn build_canvas_input(bundle: &std::path::Path, declared: &Declared) -> Result = input.errors().map(|d| d.to_string()).collect(); - if !errors.is_empty() { + if !preview && !errors.is_empty() { anyhow::bail!( "refusing to grade: {} problem(s) with the input\n {}", errors.len(), @@ -927,8 +1000,10 @@ fn build_canvas_input(bundle: &std::path::Path, declared: &Declared) -> Result MatchDecision: + override: str | None = policy.get("overrides", {}).get(requested) + if override is not None: + found: list[str] = [override] if override in names else [] + rule: str = f"matching.overrides.functions.{requested} = {override!r}" + elif requested in names: + found = [requested] + rule = "source.function_exact" + else: + found = sorted( + set(names).intersection(policy.get("aliases", {}).get(requested, [])) + ) + rule = f"matching.items.functions.{requested} = {policy.get('aliases', {}).get(requested, [])!r}" + if not found and requested not in policy.get("aliases", {}): + found = sorted( + name + for name in set(names) + if not name.startswith("_") + and difflib.SequenceMatcher( + None, requested.lower(), name.lower() + ).ratio() + >= 0.5 + ) + rule = "suggestion.function_similarity" + state: str = ( + "missing" + if not found + else ( + "review" + if rule.startswith("suggestion.") + else "matched" + if len(found) == 1 + else "ambiguous" + ) + ) + return { + "state": state, + "selected": found[0] if state == "matched" else None, + "candidates": [{"value": name, "rules": [rule]} for name in found], + } + + +def static_names(nodes: list[ast.stmt]) -> list[str]: + """Declared module functions/classes, including conditional declarations, never methods.""" + names: list[str] = [] + for node in nodes: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + names.append(node.name) + else: + for child in ast.iter_child_nodes(node): + if isinstance(child, ast.stmt): + names.extend(static_names([child])) + return names + + +class PreviewRequest(TypedDict): + path: str + requested: list[str] + policy: FunctionRules + + +def preview_functions() -> None: + requests: list[PreviewRequest] = json.load(sys.stdin) + results: list[dict[str, MatchDecision]] = [] + for request in requests: + try: + with tokenize.open(request["path"]) as source: + names: list[str] = static_names( + ast.parse(source.read(), filename=request["path"]).body + ) + except (OSError, SyntaxError, UnicodeError, LookupError): + # Syntax/loading faults belong to execution, not a teacher matching mistake. + results.append( + { + name: {"state": "deferred", "selected": None, "candidates": []} + for name in request["requested"] + } + ) + continue + decisions: dict[str, MatchDecision] = {} + for name in request["requested"]: + decision: MatchDecision = function_decision(names, name, request["policy"]) + if decision["state"] == "missing": + decision["state"] = ( + "deferred" # imports, assignments, __getattr__ and decorators + ) + decisions[name] = decision + results.append(decisions) + json.dump(results, sys.stdout, ensure_ascii=False, sort_keys=True) diff --git a/crates/scriptmark/src/matching.rs b/crates/scriptmark/src/matching.rs new file mode 100644 index 0000000..468d7f3 --- /dev/null +++ b/crates/scriptmark/src/matching.rs @@ -0,0 +1,585 @@ +//! Teacher rules and auditable decisions. A fuzzy candidate is a suggestion, never a +//! substitute for a student's declared file or function. + +use std::collections::{BTreeMap, BTreeSet}; +use std::path::{Path, PathBuf}; + +use anyhow::{Result, bail}; +use regex::Regex; +use serde::{Deserialize, Serialize}; + +use crate::models::{FileOrigin, StudentFile, StudentSubmission, Target, TestSpec, normalize_key}; + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct Config { + pub students: Vec, + pub owners: Vec, + pub items: Vec, + pub overrides: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] +pub enum StudentRule { + /// Zero-based component of the directory path relative to the scanned root. + Directory { level: usize }, + /// A filename template with one `{student}` capture; `*` and `?` are wildcards. + Pattern { pattern: String }, + /// Matches the relative path; must capture `(?P...)`. + Regex { regex: String }, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct OwnerOverride { + /// Relative path within a scanned root, or an absolute path. + pub path: PathBuf, + pub student: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ItemRule { + pub id: String, + /// Alternative filename/relative path patterns. All matching files are candidates. + #[serde(default)] + pub files: Vec, + /// Requested function -> acceptable aliases. The exact requested name wins. + #[serde(default)] + pub functions: BTreeMap>, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ItemOverride { + /// A rendered student key (`local:alice`, `canvas:123`, or a confirmed number). + pub student: String, + pub item: String, + /// Exact submitted path or a path suffix. More than one hit remains ambiguous. + #[serde(default)] + pub file: Option, + #[serde(default)] + pub functions: BTreeMap, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum State { + Matched, + Missing, + Ambiguous, + Review, + /// Static inspection cannot see a runtime export (or the module has a syntax error). + Deferred, +} + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +pub struct Candidate { + pub value: String, + pub rules: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +pub struct Decision { + pub state: State, + pub selected: Option, + pub candidates: Vec, +} + +impl Decision { + /// Retain inconsistent runtime lookups as review evidence, in unit declaration order. + pub fn merge(&mut self, other: Self) { + if *self == other { + return; + } + let candidates = self + .candidates + .iter() + .cloned() + .chain(other.candidates) + .collect(); + *self = Self::from_candidates(candidates, true); + } + pub fn exact(value: String, rule: &str) -> Self { + Self::from_candidates( + vec![Candidate { + value, + rules: vec![rule.into()], + }], + false, + ) + } + + fn from_candidates(candidates: Vec, fuzzy: bool) -> Self { + let mut grouped: BTreeMap> = BTreeMap::new(); + for candidate in candidates { + grouped + .entry(candidate.value) + .or_default() + .extend(candidate.rules); + } + let candidates: Vec = grouped + .into_iter() + .map(|(value, rules)| Candidate { + value, + rules: rules.into_iter().collect(), + }) + .collect(); + let state = match candidates.len() { + 0 => State::Missing, + _ if fuzzy => State::Review, + 1 => State::Matched, + _ => State::Ambiguous, + }; + Self { + selected: (state == State::Matched).then(|| candidates[0].value.clone()), + state, + candidates, + } + } +} + +fn pattern_regex(pattern: &str, capture: bool) -> Result { + let mut expression = String::from("^"); + let mut rest = pattern; + while !rest.is_empty() { + if capture && rest.starts_with("{student}") { + expression.push_str("(?P[^/]+?)"); + rest = &rest[9..]; + continue; + } + let ch = rest.chars().next().expect("nonempty pattern"); + match ch { + '*' => expression.push_str(".*"), + '?' => expression.push('.'), + _ => expression.push_str(®ex::escape(&ch.to_string())), + } + rest = &rest[ch.len_utf8()..]; + } + expression.push('$'); + Ok(Regex::new(&expression)?) +} + +impl Config { + pub fn validate_owners(&self) -> Result<()> { + for rule in &self.students { + match rule { + StudentRule::Pattern { pattern } => { + if pattern.matches("{student}").count() != 1 { + bail!("student pattern needs exactly one {{student}}: {pattern}"); + } + pattern_regex(pattern, true)?; + } + StudentRule::Regex { regex } => { + let compiled = Regex::new(regex)?; + if !compiled.capture_names().any(|name| name == Some("student")) { + bail!("student regex needs a named student capture: {regex}"); + } + } + StudentRule::Directory { .. } => {} + } + } + for owner in &self.owners { + if owner.path.as_os_str().is_empty() || normalize_key(&owner.student).is_empty() { + bail!("an owner override needs a path and student"); + } + } + Ok(()) + } + + pub fn validate(&self, specs: &[TestSpec]) -> Result<()> { + self.validate_owners()?; + let mut ids = BTreeSet::new(); + for item in &self.items { + if !ids.insert(&item.id) { + bail!("matching item '{}' is declared twice", item.id); + } + let spec = specs + .iter() + .find(|s| s.meta.name == item.id) + .ok_or_else(|| anyhow::anyhow!("matching item '{}' has no test spec", item.id))?; + for pattern in &item.files { + if pattern.is_empty() { + bail!("empty file pattern for '{}'", item.id); + } + pattern_regex(pattern, false)?; + } + validate_functions(item.functions.keys(), spec)?; + if item + .functions + .values() + .flatten() + .any(|name| name.is_empty()) + { + bail!("empty function alias for '{}'", item.id); + } + } + let mut overrides = BTreeSet::new(); + for entry in &self.overrides { + if entry.student.is_empty() || !overrides.insert((&entry.student, &entry.item)) { + bail!( + "duplicate or empty student override for '{}' / '{}'", + entry.student, + entry.item + ); + } + let spec = specs + .iter() + .find(|s| s.meta.name == entry.item) + .ok_or_else(|| { + anyhow::anyhow!("override item '{}' has no test spec", entry.item) + })?; + validate_functions(entry.functions.keys(), spec)?; + if entry.functions.values().any(|name| name.is_empty()) + || entry + .file + .as_ref() + .is_some_and(|path| path.as_os_str().is_empty()) + { + bail!("empty file/function override for '{}'", entry.item); + } + } + Ok(()) + } + + pub fn owner( + &self, + relative: &Path, + original: &Path, + default: Option, + ) -> Result { + let candidates: Vec = self + .owners + .iter() + .enumerate() + .filter(|(_, entry)| entry.path == relative || entry.path == original) + .map(|(i, entry)| Candidate { + value: normalize_key(&entry.student), + rules: vec![format!( + "matching.owners[{i}]: {} -> {}", + entry.path.display(), + entry.student + )], + }) + .collect(); + if !candidates.is_empty() { + return Ok(Decision::from_candidates(candidates, false)); + } + let mut candidates = Vec::new(); + for (i, rule) in self.students.iter().enumerate() { + let keys: Vec = match rule { + StudentRule::Directory { level } => relative + .parent() + .and_then(|p| p.components().nth(*level)) + .and_then(|c| c.as_os_str().to_str()) + .map(str::to_string) + .into_iter() + .collect(), + StudentRule::Pattern { pattern } => pattern_regex(pattern, true)? + .captures(&relative.file_name().unwrap_or_default().to_string_lossy()) + .and_then(|caps| caps.name("student").map(|m| m.as_str().to_string())) + .into_iter() + .collect(), + StudentRule::Regex { regex } => Regex::new(regex)? + .captures_iter(&relative.to_string_lossy()) + .filter_map(|caps| caps.name("student").map(|m| m.as_str().to_string())) + .collect(), + }; + for key in keys + .into_iter() + .map(|s| normalize_key(&s)) + .filter(|s| !s.is_empty()) + { + candidates.push(Candidate { + value: key, + rules: vec![format!("matching.students[{i}]: {rule:?}")], + }); + } + } + if !candidates.is_empty() { + return Ok(Decision::from_candidates(candidates, false)); + } + Ok(default + .map(|key| Decision::exact(key, "source.filename_prefix")) + .unwrap_or_else(|| Decision::from_candidates(Vec::new(), false))) + } + + fn item_override(&self, student: &str, item: &str) -> Option<&ItemOverride> { + self.overrides + .iter() + .find(|entry| entry.student == student && entry.item == item) + } + + pub fn functions(&self, student: &str, item: &str) -> Functions { + let aliases = self + .items + .iter() + .find(|rule| rule.id == item) + .map(|rule| rule.functions.clone()) + .unwrap_or_default(); + let overrides = self + .item_override(student, item) + .map(|entry| entry.functions.clone()) + .unwrap_or_default(); + Functions { aliases, overrides } + } + + pub fn file(&self, student: &str, files: &[StudentFile], spec: &TestSpec) -> Decision { + let language_files: Vec<&StudentFile> = files + .iter() + .filter(|f| f.language == spec.meta.language) + .collect(); + if let Some(path) = self + .item_override(student, &spec.meta.name) + .and_then(|entry| entry.file.as_ref()) + { + return file_candidates( + &language_files, + |file| file.path == *path || file.path.ends_with(path), + &format!("matching.overrides.file = {}", path.display()), + false, + ); + } + if let Some(rule) = self + .items + .iter() + .find(|rule| rule.id == spec.meta.name && !rule.files.is_empty()) + { + let mut candidates = Vec::new(); + for (i, pattern) in rule.files.iter().enumerate() { + let expression = pattern_regex(pattern, false).expect("validated file pattern"); + for file in &language_files { + let name = file.path.file_name().unwrap_or_default().to_string_lossy(); + let path = file.path.to_string_lossy(); + if expression.is_match(&name) + || path.split('/').enumerate().any(|(start, _)| { + expression.is_match( + &path.split('/').skip(start).collect::>().join("/"), + ) + }) { + candidates.push(Candidate { + value: file.path.to_string_lossy().into_owned(), + rules: vec![format!( + "matching.items[{}].files[{i}] = {pattern:?}", + rule.id + )], + }); + } + } + } + return Decision::from_candidates(candidates, false); + } + let pattern = &spec.meta.file; + let exact = file_candidates( + &language_files, + |file| { + file.path.ends_with(pattern) || file.file_name().ends_with(&format!("_{pattern}")) + }, + "source.file_suffix", + false, + ); + if exact.state != State::Missing { + return exact; + } + let stem = Path::new(pattern) + .file_stem() + .unwrap_or_default() + .to_string_lossy(); + file_candidates( + &language_files, + |file| { + if !stem.is_empty() + && file + .file_name() + .to_lowercase() + .contains(&stem.to_lowercase()) + { + return true; + } + let Ok(text) = std::fs::read_to_string(&file.path) else { + return false; + }; + requested_functions(spec).iter().any(|name| { + Regex::new(&format!( + r"(?m)^(?:async\s+)?(?:def|class)\s+{}\b", + regex::escape(name) + )) + .expect("escaped function") + .is_match(&text) + }) + }, + "suggestion.filename_or_function", + true, + ) + } +} + +fn file_candidates( + files: &[&StudentFile], + predicate: impl Fn(&StudentFile) -> bool, + rule: &str, + fuzzy: bool, +) -> Decision { + Decision::from_candidates( + files + .iter() + .filter(|file| predicate(file)) + .map(|file| Candidate { + value: file.path.to_string_lossy().into_owned(), + rules: vec![rule.into()], + }) + .collect(), + fuzzy, + ) +} + +fn validate_functions<'a>(names: impl Iterator, spec: &TestSpec) -> Result<()> { + let requested = requested_functions(spec); + for name in names { + if !requested.contains(name) { + bail!("function '{name}' is not requested by '{}'", spec.meta.name); + } + } + Ok(()) +} + +pub fn requested_functions(spec: &TestSpec) -> BTreeSet { + let mut names = BTreeSet::new(); + names.extend(spec.meta.function.iter().cloned()); + for case in spec + .cases + .iter() + .chain(spec.scenarios.iter().flat_map(|s| s.steps.iter())) + { + if let Some(Target::Function { name }) = case.target(spec.meta.function.as_deref()) { + names.insert(name); + } + } + for setup in spec.scenarios.iter().flat_map(|s| s.setup.iter()) { + if let Target::Function { name } = setup.target() { + names.insert(name); + } + } + names +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct Functions { + pub aliases: BTreeMap>, + pub overrides: BTreeMap, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ItemMatch { + pub student: String, + pub item: String, + pub file: Decision, + pub origin: Option, + pub owner: Option, + #[serde(default)] + pub functions: BTreeMap, +} + +pub fn item_match(config: &Config, student: &StudentSubmission, spec: &TestSpec) -> ItemMatch { + let file = config.file(&student.key().to_string(), student.files(), spec); + let selected = student + .files() + .iter() + .find(|f| Some(f.path.to_string_lossy().as_ref()) == file.selected.as_deref()); + ItemMatch { + student: student.key().to_string(), + item: spec.meta.name.clone(), + origin: selected.map(|f| f.origin.clone()), + owner: selected.map(|f| { + f.owner + .clone() + .unwrap_or_else(|| Decision::exact(student.key().to_string(), "source.identity")) + }), + file, + functions: BTreeMap::new(), + } +} + +#[derive(Debug, Serialize)] +pub struct Preview { + pub input: crate::models::AssignmentInput, + pub rules: Config, + pub items: Vec, + /// Indices into `items`, plus `input.unmatched` / error diagnostics for ownership. + pub pending: Vec, +} + +/// Parse all selected Python files in one isolated interpreter, without importing them. +pub fn preview( + input: crate::models::AssignmentInput, + specs: &[TestSpec], + config: &Config, + python: &str, +) -> Result { + use std::io::Write; + use std::process::{Command, Stdio}; + config.validate(specs)?; + let mut items = Vec::new(); + let mut requests = Vec::new(); + let mut slots = Vec::new(); + for student in &input.students { + for spec in specs { + let item = item_match(config, student, spec); + if let Some(path) = &item.file.selected { + requests.push(serde_json::json!({ + "path": path, "requested": requested_functions(spec), + "policy": config.functions(&item.student, &item.item), + })); + slots.push(items.len()); + } + items.push(item); + } + } + if !requests.is_empty() { + let code = format!("{}\npreview_functions()", include_str!("matching.py")); + let mut child = Command::new(python) + .args(["-I", "-S", "-c", &code]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + child + .stdin + .take() + .expect("piped stdin") + .write_all(&serde_json::to_vec(&requests)?)?; + let output = child.wait_with_output()?; + if !output.status.success() { + bail!( + "function preview failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + let decisions: Vec> = serde_json::from_slice(&output.stdout)?; + if decisions.len() != slots.len() { + bail!("function preview returned a different item count"); + } + for (slot, functions) in slots.into_iter().zip(decisions) { + items[slot].functions = functions; + } + } + let pending = items + .iter() + .enumerate() + .filter(|(_, item)| { + item.file.state != State::Matched + || item + .functions + .values() + .any(|decision| decision.state != State::Matched) + }) + .map(|(i, _)| i) + .collect(); + Ok(Preview { + input, + rules: config.clone(), + items, + pending, + }) +} diff --git a/crates/scriptmark/src/models/config.rs b/crates/scriptmark/src/models/config.rs index 3b9bb5d..844059e 100644 --- a/crates/scriptmark/src/models/config.rs +++ b/crates/scriptmark/src/models/config.rs @@ -135,6 +135,8 @@ pub enum AttemptPolicy { #[serde(deny_unknown_fields)] pub struct AssignmentConfig { pub assignment: AssignmentInfo, + #[serde(default)] + pub matching: crate::matching::Config, /// Expected student files. #[serde(default)] pub files: Vec, diff --git a/crates/scriptmark/src/models/result.rs b/crates/scriptmark/src/models/result.rs index 25f4222..fb2571c 100644 --- a/crates/scriptmark/src/models/result.rs +++ b/crates/scriptmark/src/models/result.rs @@ -31,6 +31,8 @@ pub enum Fault { #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Cause { + /// A file/function decision needs teacher review; never a student zero. + Matching, /// No file matched the spec. NoFile, /// The function, method or attribute does not exist. @@ -75,6 +77,8 @@ pub enum Cause { /// What the call was given: the evidence behind "明确测试输入". #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct CaseInput { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub matching: Option, /// The name asked for, and — when a lookup may have substituted — the one it found. #[serde(default, skip_serializing_if = "Option::is_none")] pub target: Option, @@ -178,6 +182,8 @@ impl TestResult { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct StudentReport { + #[serde(default)] + pub matches: Vec, /// `StudentKey`'s rendering — a bare 学号, or a `canvas:` / `local:` prefixed form that /// can never be mistaken for one. pub student_id: String, @@ -214,6 +220,7 @@ impl StudentReport { pub fn new(student_id: impl Into, submission_state: SubmissionOutcome) -> Self { Self { student_id: student_id.into(), + matches: Vec::new(), student_name: None, test_results: Vec::new(), backend_name: None, diff --git a/crates/scriptmark/src/models/submission.rs b/crates/scriptmark/src/models/submission.rs index 16d9fab..ddc5edb 100644 --- a/crates/scriptmark/src/models/submission.rs +++ b/crates/scriptmark/src/models/submission.rs @@ -216,6 +216,8 @@ pub enum FileOrigin { pub struct StudentFile { pub path: PathBuf, pub language: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub owner: Option, #[serde(default)] pub origin: FileOrigin, } @@ -226,6 +228,7 @@ impl StudentFile { Self { path: path.into(), language: language.into(), + owner: None, origin: FileOrigin::Direct, } } @@ -533,6 +536,11 @@ pub enum DiagnosticSeverity { )] #[serde(rename_all = "snake_case")] pub enum DiagnosticKind { + #[error("ambiguous owner of '{path}': {decision:?}; add a matching.owners override")] + AmbiguousOwner { + path: PathBuf, + decision: crate::matching::Decision, + }, #[error("roster lists '{key}' in {count} identical rows; merged into one")] DuplicateRosterEntry { key: String, count: usize }, #[error("roster rows disagree about who '{key}' is: {detail}")] diff --git a/crates/scriptmark/src/runner/executor.rs b/crates/scriptmark/src/runner/executor.rs index d478b14..330fc84 100644 --- a/crates/scriptmark/src/runner/executor.rs +++ b/crates/scriptmark/src/runner/executor.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; use serde_json::Value; -use crate::models::{StudentFile, Target, TestSpec}; +use crate::models::{Target, TestSpec}; /// A language backend. pub trait Executor: Send + Sync + 'static { @@ -21,9 +21,6 @@ pub trait Executor: Send + Sync + 'static { Ok(self.language().to_string()) } - /// Pick the student's file for a spec. P-673 owns the rule. - fn locate<'a>(&self, files: &'a [StudentFile], spec: &TestSpec) -> Option<&'a StudentFile>; - /// Import the spec's teacher modules the way a unit would, and report their exports. fn inspect( &self, @@ -48,6 +45,7 @@ pub enum Subject { #[derive(Debug, Clone)] pub struct UnitPlan { pub subject: Subject, + pub functions: crate::matching::Functions, /// Copied into the unit's working directory before running. pub file: PathBuf, /// Run the file as `__main__` with this stdin, instead of calling functions. @@ -180,11 +178,18 @@ pub struct CallObservation { pub struct Resolved { pub requested: String, pub resolved: String, + #[serde(default)] + pub matching: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Outcome { + /// A teacher must choose among candidates or repair an explicit override. + Matching { + message: String, + decision: crate::matching::Decision, + }, Returned { value: Value, #[serde(rename = "type")] diff --git a/crates/scriptmark/src/runner/harness.py b/crates/scriptmark/src/runner/harness.py index 6601942..6910f21 100644 --- a/crates/scriptmark/src/runner/harness.py +++ b/crates/scriptmark/src/runner/harness.py @@ -28,6 +28,10 @@ import runpy # noqa: E402 import signal # noqa: E402 import time # noqa: E402 +from typing import TYPE_CHECKING # noqa: E402 + +if TYPE_CHECKING: + from matching import FunctionRules, MatchDecision, function_decision with open(sys.argv[1], encoding="utf-8") as _fh: PAYLOAD = json.load(_fh) @@ -270,6 +274,12 @@ class Missing(Exception): """The call's target does not exist.""" +class MatchingRequired(Exception): + def __init__(self, decision: MatchDecision) -> None: + self.decision: MatchDecision = decision + super().__init__(f"function matching needs review: {decision}") + + class Unresolved(Exception): """A `$ref` or `object` whose value was never produced in this unit.""" @@ -302,6 +312,8 @@ def call(fn, timeout, stdin=None, guarded=True, serialise=True): outcome = {"timeout": {}} except Missing as exc: outcome = {"missing": {"message": str(exc)}} + except MatchingRequired as exc: + outcome = {"matching": {"message": str(exc), "decision": exc.decision}} except Unresolved as exc: outcome = {"unresolved": {"name": str(exc)}} except Unserialisable as exc: @@ -428,26 +440,35 @@ def resolve(value): return value -def fuzzy_lookup(module, name, argc): - """Find a function by exact name or best fuzzy match. P-673 owns this rule.""" - if hasattr(module, name): - return getattr(module, name), name - from difflib import SequenceMatcher - - scored = [] - for candidate in dir(module): - obj = getattr(module, candidate) - if candidate.startswith("_") or not callable(obj): - continue - score = SequenceMatcher(None, name.lower(), candidate.lower()).ratio() - params = params_of(obj) - positional = [p for p in params or [] if p["name"] != "self" and "var" not in p["kind"]] - if params is not None and len(positional) == argc: - score += 0.2 - scored.append((score, candidate, obj)) - scored.sort(key=lambda s: -s[0]) - if scored and scored[0][0] >= 0.5: - return scored[0][2], scored[0][1] +def matched_function( + module: object, name: str, evidence: list[MatchDecision | None] +) -> tuple[object, str]: + policy: FunctionRules = PAYLOAD.get("functions", {"aliases": {}, "overrides": {}}) + # Read attributes only once: dynamic exports/properties belong to the guarded call. + exports: dict[str, object] = dict(vars(module)) + absent: object = object() + for candidate in { + name, + *policy.get("aliases", {}).get(name, []), + *([policy["overrides"][name]] if name in policy.get("overrides", {}) else []), + }: + if candidate not in exports: + value: object = getattr(module, candidate, absent) + if value is not absent: + exports[candidate] = value + decision: MatchDecision = function_decision( + [candidate for candidate, obj in exports.items() if callable(obj)], name, policy + ) + evidence[0] = decision + selected: str | None = decision["selected"] + if selected is not None: + return exports[selected], selected + if ( + decision["state"] != "missing" + or name in policy.get("overrides", {}) + or name in policy.get("aliases", {}) + ): + raise MatchingRequired(decision) raise Missing(f"function '{name}' not found") @@ -471,7 +492,12 @@ def member(obj, object_id, name, what): raise -def invoke(plan, student, resolved): +def invoke( + plan: dict[str, object], + student: object, + resolved: list[str], + evidence: list[MatchDecision | None], +) -> object: """The call a plan names. Runs inside `call()`: the lookup is student code too.""" kind, spec = next(iter(plan["target"].items())) args = resolve(plan.get("args", [])) @@ -481,7 +507,7 @@ def invoke(plan, student, resolved): raise Missing(f"function '{spec['name']}' not found") fn, resolved[0] = getattr(student, spec["name"]), spec["name"] else: - fn, resolved[0] = fuzzy_lookup(student, spec["name"], len(args)) + fn, resolved[0] = matched_function(student, spec["name"], evidence) return fn(*args) if kind == "method": fn = member(live(spec["object"]), spec["object"], spec["name"], "method") @@ -563,8 +589,9 @@ def run_calls(phase, calls, student): for index, plan in enumerate(calls): requested = next(iter(plan["target"].values()))["name"] resolved = [requested] + evidence: list[MatchDecision | None] = [None] outcome, stdout, truncated, elapsed, value = call( - lambda: invoke(plan, student, resolved), + lambda: invoke(plan, student, resolved, evidence), plan["timeout"], plan.get("stdin"), guarded=STUDENT and "teacher" not in plan["target"], @@ -574,7 +601,7 @@ def run_calls(phase, calls, student): "kind": "call", "phase": phase, "index": index, - "target": {"requested": requested, "resolved": resolved[0]}, + "target": {"requested": requested, "resolved": resolved[0], "matching": evidence[0]}, "outcome": outcome, "stdout": stdout, "stdout_truncated": truncated, diff --git a/crates/scriptmark/src/runner/judge.rs b/crates/scriptmark/src/runner/judge.rs index b4acc18..0b23342 100644 --- a/crates/scriptmark/src/runner/judge.rs +++ b/crates/scriptmark/src/runner/judge.rs @@ -244,6 +244,14 @@ fn unit_failure(plan: &UnitPlan, obs: &UnitObservation) -> Option { match obs.setup.get(index) { Some(record) => match &record.outcome { Outcome::Returned { .. } => {} + Outcome::Matching { message, .. } => { + return blanket( + TestStatus::Error, + Fault::Teacher, + Cause::Matching, + message.clone(), + ); + } outcome => { return blanket( TestStatus::Error, @@ -349,6 +357,7 @@ fn describe(outcome: &Outcome, timeout: u64) -> String { format!("returned a value that cannot be represented: {}", e.message) } Outcome::Missing { message } => message.clone(), + Outcome::Matching { message, .. } => message.clone(), Outcome::Unresolved { name } => format!("needs '{name}', which was never produced"), } } @@ -418,6 +427,7 @@ fn blanket_result(s: &Scored, blanket: &Blanket, target: Option) -> Case fn input_of(case: &TestCase, record: Option<&CallObservation>) -> CaseInput { let resolved = record.and_then(|r| r.target.as_ref()); CaseInput { + matching: resolved.and_then(|r| r.matching.clone()), target: resolved.map(|r| r.requested.clone()), resolved: resolved .filter(|r| r.resolved != r.requested) @@ -484,6 +494,14 @@ fn judge_call( && (case.expected_stdout.is_some() || case.check.is_some()); // 1. The outcome, against `expect_error`. match &record.outcome { + Outcome::Matching { message, .. } => { + return verdict.fail( + TestStatus::Error, + Fault::Teacher, + Cause::Matching, + message.clone(), + ); + } Outcome::Missing { message } => { return verdict.fail( TestStatus::Missing, @@ -771,6 +789,7 @@ mod tests { fn plan(steps: usize) -> UnitPlan { UnitPlan { subject: Subject::Student, + functions: crate::matching::Functions::default(), file: "lab.py".into(), script: None, imports: Vec::new(), @@ -801,6 +820,7 @@ mod tests { CallObservation { index, target: Some(Resolved { + matching: None, requested: "f".into(), resolved: "f".into(), }), diff --git a/crates/scriptmark/src/runner/oracle.rs b/crates/scriptmark/src/runner/oracle.rs index 9995f4e..601296f 100644 --- a/crates/scriptmark/src/runner/oracle.rs +++ b/crates/scriptmark/src/runner/oracle.rs @@ -25,6 +25,7 @@ pub async fn resolve_oracle( let timeout_secs = case.timeout.unwrap_or(timeout_secs); let plan = UnitPlan { subject: Subject::Reference, + functions: crate::matching::Functions::default(), file: reference.into(), script: None, imports: spec.meta.imports.clone(), diff --git a/crates/scriptmark/src/runner/orchestrator.rs b/crates/scriptmark/src/runner/orchestrator.rs index bf9cd72..9850d02 100644 --- a/crates/scriptmark/src/runner/orchestrator.rs +++ b/crates/scriptmark/src/runner/orchestrator.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; use tokio::sync::Semaphore; @@ -15,6 +15,7 @@ use crate::runner::prepare::{Bundle, Unit}; /// How a batch runs. #[derive(Debug, Clone)] pub struct RunOptions { + pub matching: crate::matching::Config, /// Units running at once. Defaults to the number of CPUs; 0 runs one at a time, and a /// value past what a semaphore can count is capped there. pub concurrency: Option, @@ -25,6 +26,7 @@ pub struct RunOptions { impl Default for RunOptions { fn default() -> Self { Self { + matching: crate::matching::Config::default(), concurrency: None, python: "python3".into(), } @@ -58,6 +60,7 @@ pub async fn run_all( .clamp(1, Semaphore::MAX_PERMITS); let semaphore = Arc::new(Semaphore::new(concurrency)); let python: Arc = options.python.as_str().into(); + let matching = Arc::new(options.matching.clone()); let mut handles = Vec::new(); for student in students { @@ -69,6 +72,11 @@ pub async fn run_all( // very output a teacher needs to resolve the mismatch. let runnable = student.state == SubmissionState::Executable; let files = student.files().to_vec(); + let matches: Vec = bundles + .iter() + .map(|bundle| crate::matching::item_match(&matching, student, &bundle.spec)) + .collect(); + let matching = matching.clone(); let (bundles, executor, semaphore, python) = ( bundles.clone(), executor.clone(), @@ -79,8 +87,18 @@ pub async fn run_all( let handle = tokio::spawn(async move { let sid = identity.key.to_string(); let mut report = StudentReport::new(sid, outcome); + report.matches = matches; if runnable { - run_student(&mut report, files, bundles, executor, semaphore, python).await; + run_student( + &mut report, + files, + bundles, + executor, + semaphore, + python, + matching, + ) + .await; } // Nothing to run, but the student still gets a row. report.student_name = identity.name.clone(); @@ -118,25 +136,51 @@ async fn run_student( executor: Arc, semaphore: Arc, python: Arc, + matching: Arc, ) { let mut slots: Vec>>> = bundles.iter().map(|b| vec![None; b.units.len()]).collect(); let mut tasks = JoinSet::new(); let mut where_is = HashMap::new(); let mut graded_files: Vec> = vec![None; bundles.len()]; + let mut lookups: Vec>> = bundles + .iter() + .map(|bundle| vec![BTreeMap::new(); bundle.units.len()]) + .collect(); for (b, bundle) in bundles.iter().enumerate() { // One file per (student, bundle): every unit of a spec runs against the same file. - let Some(file) = executor.locate(&files, &bundle.spec) else { + let decision = &report.matches[b].file; + let Some(file) = files.iter().find(|file| { + Some(file.path.to_string_lossy().as_ref()) == decision.selected.as_deref() + }) else { + let pending = decision.state != crate::matching::State::Missing + || matching.overrides.iter().any(|entry| { + entry.student == report.student_id + && entry.item == bundle.spec.meta.name + && entry.file.is_some() + }); for (u, unit) in bundle.units.iter().enumerate() { slots[b][u] = Some(blanket( unit, - TestStatus::Missing, - Fault::Student, - Cause::NoFile, + if pending { + TestStatus::Error + } else { + TestStatus::Missing + }, + if pending { + Fault::Teacher + } else { + Fault::Student + }, + if pending { + Cause::Matching + } else { + Cause::NoFile + }, &format!( - "No file matching '{}' found in submission", - bundle.spec.meta.file + "File matching '{}' requires {:?}: {:?}", + bundle.spec.meta.file, decision.state, decision.candidates ), )); } @@ -144,7 +188,9 @@ async fn run_student( }; graded_files[b] = Some(file.path.to_string_lossy().into_owned()); let path = std::path::absolute(&file.path).unwrap_or_else(|_| file.path.clone()); + let functions = matching.functions(&report.student_id, &bundle.spec.meta.name); for u in 0..bundle.units.len() { + let functions = functions.clone(); let (bundles, executor, semaphore, python, path) = ( bundles.clone(), executor.clone(), @@ -156,11 +202,30 @@ async fn run_student( let _permit = semaphore.acquire_owned().await.expect("semaphore closed"); let mut plan = bundles[b].units[u].plan.clone(); plan.file = path; + plan.functions = functions; let observation = executor.run(&plan).await; // Judging may run a teacher's checker script: keep it off the async workers. tokio::task::spawn_blocking(move || { let unit = &bundles[b].units[u]; - judge(&plan, &unit.scored(), &observation, &python) + let mut functions: BTreeMap = + BTreeMap::new(); + for target in observation + .setup + .iter() + .chain(&observation.steps) + .filter_map(|record| record.target.as_ref()) + { + if let Some(decision) = &target.matching { + functions + .entry(target.requested.clone()) + .and_modify(|current| current.merge(decision.clone())) + .or_insert_with(|| decision.clone()); + } + } + ( + judge(&plan, &unit.scored(), &observation, &python), + functions, + ) }) .await .expect("judging panicked") @@ -171,9 +236,10 @@ async fn run_student( while let Some(joined) = tasks.join_next_with_id().await { match joined { - Ok((id, results)) => { + Ok((id, (results, functions))) => { let (b, u) = where_is[&id]; slots[b][u] = Some(results); + lookups[b][u] = functions; } // A panicking unit costs that unit, not the student's other results. Err(e) => { @@ -189,6 +255,36 @@ async fn run_student( } } + for (b, units) in lookups.into_iter().enumerate() { + for (name, decision) in units.into_iter().flat_map(BTreeMap::into_iter) { + report.matches[b] + .functions + .entry(name) + .and_modify(|current| current.merge(decision.clone())) + .or_insert(decision); + } + if report.matches[b].functions.values().any(|decision| { + matches!( + decision.state, + crate::matching::State::Review | crate::matching::State::Ambiguous + ) + }) { + for cases in slots[b].iter_mut().flatten() { + for case in cases { + case.status = TestStatus::Error; + case.fault = Some(Fault::Teacher); + case.cause = Some(Cause::Matching); + case.failure = Some(FailureDetail { + message: format!( + "function matching requires review: {:?}", + report.matches[b].functions + ), + details: String::new(), + }); + } + } + } + } report.lint = lint(&bundles, &graded_files, &semaphore).await; report.test_results = bundles .iter() diff --git a/crates/scriptmark/src/runner/prepare.rs b/crates/scriptmark/src/runner/prepare.rs index de285e1..c42c78c 100644 --- a/crates/scriptmark/src/runner/prepare.rs +++ b/crates/scriptmark/src/runner/prepare.rs @@ -544,6 +544,7 @@ fn dry_run_rhai(spec: &TestSpec) -> Vec { fn plan_units(spec: &TestSpec, timeout_secs: u64) -> Vec { let base = UnitPlan { subject: Subject::Student, + functions: crate::matching::Functions::default(), file: PathBuf::new(), script: None, imports: spec.meta.imports.clone(), diff --git a/crates/scriptmark/src/runner/python.rs b/crates/scriptmark/src/runner/python.rs index 7d51232..ed65421 100644 --- a/crates/scriptmark/src/runner/python.rs +++ b/crates/scriptmark/src/runner/python.rs @@ -3,7 +3,7 @@ use std::sync::{Arc, Mutex}; use tokio::process::Command; -use crate::models::{StudentFile, TestSpec}; +use crate::models::TestSpec; use crate::runner::executor::{ Executor, Exit, ProtocolError, Subject, TeacherRuntime, UnitObservation, UnitPlan, }; @@ -47,95 +47,6 @@ impl PythonExecutor { pub fn python_cmd(&self) -> &str { &self.python_cmd } - - /// Find the student file matching the spec's file pattern. - /// - /// Strategy (scored, best wins): - /// 1. Exact suffix match (100) → highest confidence - /// 2. Strip numeric prefixes, compare stems (80-100) - /// 3. Stem-contains (40-60) - /// 4. Function-definition scan (+200 bonus) → if spec has function hint, - /// files containing `def function_name` get a large boost - fn find_student_file_with_hint<'a>( - &self, - student_files: &'a [StudentFile], - pattern: &str, - function_hint: Option<&str>, - ) -> Option<&'a StudentFile> { - let pattern_stem = Path::new(pattern) - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or(pattern); - - // 1. Exact suffix match — highest confidence - if let Some(f) = student_files.iter().find(|f| { - f.path - .file_name() - .and_then(|n| n.to_str()) - .is_some_and(|n| n.ends_with(pattern)) - }) { - return Some(f); - } - - // Helper: strip numeric prefix segments (SID_uploadID_fileID_) - // "21300110043_171469_6012331_Lab3_2-2.py" → "Lab3_2-2.py" - fn extract_actual_name(filename: &str) -> &str { - let mut rest = filename; - loop { - if let Some(idx) = rest.find('_') { - let prefix = &rest[..idx]; - if prefix.chars().all(|c| c.is_ascii_digit()) { - rest = &rest[idx + 1..]; - continue; - } - } - break; - } - rest - } - - // 2. Score all candidates by filename similarity + function content - let mut scored: Vec<(&'a StudentFile, u32)> = student_files - .iter() - .filter_map(|f| { - let filename = f.path.file_name()?.to_str()?; - let actual = extract_actual_name(filename); - let actual_stem = Path::new(actual) - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or(actual); - - let mut score: u32 = 0; - - // --- Filename similarity --- - if actual_stem == pattern_stem { - score += 100; - } else if actual_stem.starts_with(pattern_stem) { - score += 80; - } else if actual.contains(pattern_stem) { - score += 60; - } else if filename.contains(pattern_stem) { - score += 40; - } - - // --- Function definition scan (highest priority tiebreaker) --- - if let Some(func_name) = function_hint - && let Ok(content) = std::fs::read_to_string(&f.path) - { - let needle = format!("def {func_name}"); - if content.contains(&needle) { - score += 200; // trumps filename-only matches - } - } - - if score > 0 { Some((f, score)) } else { None } - }) - .collect(); - - scored.sort_by_key(|a| std::cmp::Reverse(a.1)); - - scored.first().map(|(f, _)| *f) - } } /// The one harness every unit runs under. See `harness.py`. @@ -280,7 +191,11 @@ impl PythonExecutor { // off sys.path. What those variables used to set is spelled as flags. .args(["-I", "-B", "-X", "utf8"]) .arg("-c") - .arg(HARNESS) + .arg(format!( + "import sys; sys.path[:] = [p for p in sys.path if p]\n{}\n{}", + include_str!("../matching.py"), + HARNESS + )) .arg(&staged.payload) .current_dir(&staged.work) .stdin(if stdin.is_some() { @@ -571,18 +486,6 @@ impl Executor for PythonExecutor { )) } - fn locate<'a>(&self, files: &'a [StudentFile], spec: &TestSpec) -> Option<&'a StudentFile> { - // The hint chain mode used: the first function a case names, else [meta] function. - // (Per-case specs named no case function, so this is their hint too.) - let hint = spec - .cases - .iter() - .chain(spec.scenarios.iter().flat_map(|s| s.steps.iter())) - .find_map(|c| c.function.as_deref()) - .or(spec.meta.function.as_deref()); - self.find_student_file_with_hint(files, &spec.meta.file, hint) - } - async fn inspect(&self, spec: &TestSpec, timeout_secs: u64) -> Result { if spec.meta.imports.is_empty() { return Ok(TeacherRuntime::default()); @@ -653,6 +556,7 @@ impl Executor for PythonExecutor { "allowed_imports": plan.allowed_imports, "load_timeout": plan.load_timeout, "subject": plan.subject, + "functions": plan.functions, "lookup": match plan.subject { Subject::Student => "fuzzy", Subject::Reference => "exact", @@ -745,6 +649,7 @@ mod tests { }; let plan = |steps: Vec| UnitPlan { subject: Subject::Student, + functions: crate::matching::Functions::default(), file: PathBuf::from("lab.py"), script: None, imports: Vec::new(), diff --git a/crates/scriptmark/tests/harness.rs b/crates/scriptmark/tests/harness.rs index a1ddb31..094f9de 100644 --- a/crates/scriptmark/tests/harness.rs +++ b/crates/scriptmark/tests/harness.rs @@ -46,6 +46,7 @@ fn call(target: Target, args: Vec) -> CallPlan { fn unit(file: &Path) -> UnitPlan { UnitPlan { subject: Subject::Student, + functions: scriptmark::matching::Functions::default(), file: file.to_path_buf(), script: None, imports: Vec::new(), diff --git a/crates/scriptmark/tests/input_equivalence.rs b/crates/scriptmark/tests/input_equivalence.rs index 344e2ed..26d2073 100644 --- a/crates/scriptmark/tests/input_equivalence.rs +++ b/crates/scriptmark/tests/input_equivalence.rs @@ -52,6 +52,7 @@ fn local_input(dir: &tempfile::TempDir) -> AssignmentInput { assignment: Assignment::named("hw1"), roster: Some(&roster), attempt_policy: AttemptPolicy::Latest, + matching: None, }, ) .unwrap() diff --git a/crates/scriptmark/tests/matching.rs b/crates/scriptmark/tests/matching.rs new file mode 100644 index 0000000..f7724f1 --- /dev/null +++ b/crates/scriptmark/tests/matching.rs @@ -0,0 +1,415 @@ +//! P-673: preview, resolve an ambiguity with teacher configuration, then grade the +//! selected bytes. These exercise the real CLI and Python executor together. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +use scriptmark::discovery::{LocalInputOptions, load_local_input}; +use scriptmark::matching::{Config, OwnerOverride, State}; +use scriptmark::models::{Cause, StudentReport}; + +const SPEC: &str = r#" +[meta] +name = "double" +file = "solve.py" +function = "double" +language = "python" +[[cases]] +name = "basic" +args = [3] +expect = 6 +"#; + +struct Bench { + dir: tempfile::TempDir, +} + +impl Bench { + fn new() -> Self { + let bench = Self { + dir: tempfile::tempdir().unwrap(), + }; + bench.write("tests/test_double.toml", SPEC); + bench + } + + fn path(&self) -> &Path { + self.dir.path() + } + + fn write(&self, name: &str, content: &str) { + let path: PathBuf = self.path().join(name); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, content).unwrap(); + } + + fn config(&self, rules: &str) { + self.write( + "assignment.toml", + &format!("[assignment]\nname = 'hw'\n[grading]\nmissing_file = 'zero'\n{rules}"), + ); + } + + fn run(&self, command: &str) -> Output { + Command::new(env!("CARGO_BIN_EXE_scriptmark")) + .current_dir(self.path()) + .args([command, "subs", "-t", "tests", "-o", "out/result.json"]) + .output() + .unwrap() + } + + fn successful(&self, command: &str) { + let output = self.run(command); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } + + fn preview(&self) -> serde_json::Value { + self.successful("match"); + serde_json::from_slice(&std::fs::read(self.path().join("out/result.json")).unwrap()) + .unwrap() + } + + fn grades(&self) -> Vec { + self.successful("grade"); + serde_json::from_slice(&std::fs::read(self.path().join("out/result.json")).unwrap()) + .unwrap() + } +} + +const DIRECTORY: &str = "[[matching.students]]\nkind = 'directory'\nlevel = 0\n"; + +#[test] +fn nested_files_aliases_and_shared_file_are_traceable_without_mutating_sources() { + let bench = Bench::new(); + bench.write( + "tests/test_triple.toml", + &SPEC + .replace("double", "triple") + .replace("expect = 6", "expect = 9"), + ); + let source: &str = "raise RuntimeError('preview must not execute me')\ndef times_two(x):\n return x * 2\ndef triple(x):\n return x * 3\n"; + bench.write("subs/alice/deep/solve.py", source); + bench.config( + &(DIRECTORY.to_string() + + "[[matching.items]]\nid = 'double'\nfunctions = { double = ['times_two'] }\n"), + ); + let first = bench.preview(); + assert_eq!(first["pending"], serde_json::json!([])); + assert_eq!( + first["items"][0]["functions"]["double"]["selected"], + "times_two" + ); + assert_eq!(first["items"][0]["owner"]["selected"], "alice"); + assert_eq!( + first, + bench.preview(), + "same inputs/rules produce identical preview" + ); + assert_eq!( + std::fs::read_to_string(bench.path().join("subs/alice/deep/solve.py")).unwrap(), + source + ); + assert!(!bench.path().join("subs/alice/deep/__pycache__").exists()); + bench.write( + "subs/alice/deep/solve.py", + &source.replace("raise RuntimeError('preview must not execute me')\n", ""), + ); + let grades = bench.grades(); + assert_eq!(grades[0].final_grade(), Some(100.0)); + assert_eq!(grades[0].matches.len(), 2, "one file can serve both items"); + assert_eq!( + grades[0].matches[0].file.selected, + grades[0].matches[1].file.selected + ); + let input = grades[0].test_results[0].cases[0].input.as_ref().unwrap(); + assert_eq!(input.resolved.as_deref(), Some("times_two")); + assert_eq!(input.matching.as_ref().unwrap().state, State::Matched); +} + +#[test] +fn duplicate_filenames_withhold_grades_until_an_exact_override_selects_one() { + let bench = Bench::new(); + bench.write("subs/alice/old/solve.py", "def double(x):\n return 0\n"); + bench.write( + "subs/alice/new/solve.py", + "def double(x):\n return x * 2\n", + ); + bench.config(DIRECTORY); + let preview = bench.preview(); + assert_eq!(preview["items"][0]["file"]["state"], "ambiguous"); + assert_eq!( + preview["items"][0]["file"]["candidates"] + .as_array() + .unwrap() + .len(), + 2 + ); + let grades = bench.grades(); + assert_eq!( + grades[0].final_grade(), + None, + "missing_file=zero cannot score a conflict" + ); + assert_eq!( + grades[0].test_results[0].cases[0].cause, + Some(Cause::Matching) + ); + bench.config(&(DIRECTORY.to_string() + "[[matching.overrides]]\nstudent = 'local:alice'\nitem = 'double'\nfile = 'alice/new/solve.py'\n")); + assert_eq!(bench.preview()["pending"], serde_json::json!([])); + assert_eq!(bench.grades()[0].final_grade(), Some(100.0)); +} + +#[test] +fn even_one_fuzzy_function_is_reviewed_and_never_silently_called() { + let bench = Bench::new(); + bench.write("subs/alice_solve.py", "def doubled(x):\n return x * 2\n"); + assert_eq!( + bench.preview()["items"][0]["functions"]["double"]["state"], + "review" + ); + let grades = bench.grades(); + assert_eq!(grades[0].final_grade(), None); + assert_eq!( + grades[0].test_results[0].cases[0].cause, + Some(Cause::Matching) + ); + let evidence = grades[0].test_results[0].cases[0] + .input + .as_ref() + .unwrap() + .matching + .as_ref() + .unwrap(); + assert_eq!(evidence.candidates[0].value, "doubled"); + bench.config("[[matching.overrides]]\nstudent = 'local:alice'\nitem = 'double'\nfunctions = { double = 'doubled' }\n"); + assert_eq!(bench.grades()[0].final_grade(), Some(100.0)); +} + +#[test] +fn aliases_are_candidates_and_explicit_function_override_wins_over_exact_name() { + let bench = Bench::new(); + bench.write( + "subs/alice_solve.py", + "def times_two(x):\n return x * 2\ndef multiply_two(x):\n return x * 2\n", + ); + let aliases = "[[matching.items]]\nid = 'double'\nfunctions = { double = ['times_two', 'multiply_two'] }\n"; + bench.config(aliases); + assert_eq!( + bench.preview()["items"][0]["functions"]["double"]["state"], + "ambiguous" + ); + assert_eq!(bench.grades()[0].final_grade(), None); + bench.write( + "subs/alice_solve.py", + "def double(x):\n return 0\ndef times_two(x):\n return x * 2\n", + ); + bench.config(&(aliases.to_string() + "[[matching.overrides]]\nstudent = 'local:alice'\nitem = 'double'\nfunctions = { double = 'times_two' }\n")); + assert_eq!(bench.grades()[0].final_grade(), Some(100.0)); + bench.config(&(aliases.to_string() + "[[matching.overrides]]\nstudent = 'local:alice'\nitem = 'double'\nfunctions = { double = 'absent' }\n")); + assert_eq!( + bench.grades()[0].final_grade(), + None, + "invalid override must not fall back to double" + ); +} + +#[test] +fn fuzzy_files_require_a_teacher_pattern_and_missing_files_stay_visible() { + let bench = Bench::new(); + bench.write("subs/alice_final.py", "def double(x):\n return x * 2\n"); + bench.write("subs/bob_other.py", "pass\n"); + let preview = bench.preview(); + assert_eq!(preview["items"][0]["file"]["state"], "review"); + assert_eq!(preview["items"][1]["file"]["state"], "missing"); + assert_eq!(bench.grades()[0].final_grade(), None); + bench.config("[[matching.items]]\nid = 'double'\nfiles = ['*_final.py']\n"); + let grades = bench.grades(); + assert_eq!(grades[0].final_grade(), Some(100.0)); + assert_eq!( + grades[1].final_grade(), + Some(0.0), + "declared missing_file policy still applies to absence" + ); +} + +#[test] +fn student_rule_conflicts_are_previewable_and_explicit_ownership_resolves_them() { + let bench = Bench::new(); + bench.write( + "subs/alice/bob_solve.py", + "def double(x):\n return x * 2\n", + ); + let conflict = DIRECTORY.to_string() + + "[[matching.students]]\nkind = 'pattern'\npattern = '{student}_*.py'\n"; + bench.config(&conflict); + let preview = bench.preview(); + assert_eq!(preview["input"]["unmatched"].as_array().unwrap().len(), 1); + bench.write("out/result.json", "prior results"); + assert!(!bench.run("grade").status.success()); + assert_eq!( + std::fs::read_to_string(bench.path().join("out/result.json")).unwrap(), + "prior results" + ); + bench.config( + &(conflict + "[[matching.owners]]\npath = 'alice/bob_solve.py'\nstudent = 'alice'\n"), + ); + assert_eq!(bench.grades()[0].student_id, "local:alice"); + assert_eq!(bench.grades()[0].final_grade(), Some(100.0)); +} + +#[test] +fn filename_patterns_regex_and_canvas_download_prefixes_keep_identity_and_rules() { + let bench = Bench::new(); + bench.write( + "subs/00123_777_888_solve.py", + "def double(x):\n return x * 2\n", + ); + let grades = bench.grades(); + assert_eq!(grades[0].student_id, "local:00123"); + assert_eq!(grades[0].final_grade(), Some(100.0)); + bench.config( + "[[matching.students]]\nkind = 'regex'\nregex = '^(?P[0-9]+)_[0-9]+_[0-9]+_.*$'\n", + ); + assert_eq!(bench.grades()[0].student_id, "local:00123"); + bench.config("[[matching.students]]\nkind = 'pattern'\npattern = '{student}_777_888_*.py'\n"); + assert_eq!(bench.grades()[0].student_id, "local:00123"); +} + +#[test] +fn setup_function_matching_withholds_the_entire_scenario() { + let bench = Bench::new(); + bench.write( + "tests/test_double.toml", + r#" +[meta] +name = 'double' +file = 'solve.py' +language = 'python' +[[scenarios]] +name = 'counter' +[[scenarios.setup]] +id = 'c' +function = 'Counter' +[[scenarios.steps]] +name = 'value' +attribute = 'value' +object = 'c' +expect = 6 +"#, + ); + bench.write("subs/alice_solve.py", "class Counters:\n value = 6\n"); + let grades = bench.grades(); + assert_eq!(grades[0].final_grade(), None); + assert_eq!( + grades[0].matches[0].functions["Counter"].candidates[0].value, + "Counters" + ); + bench.config("[[matching.items]]\nid = 'double'\nfunctions = { Counter = ['Counters'] }\n"); + assert_eq!(bench.grades()[0].final_grade(), Some(100.0)); +} + +#[test] +fn an_archive_owner_override_applies_to_archive_contents_and_receipt() { + use std::io::Write; + let bench = Bench::new(); + std::fs::create_dir(bench.path().join("subs")).unwrap(); + let file = std::fs::File::create(bench.path().join("subs/mystery.zip")).unwrap(); + let mut zip = zip::ZipWriter::new(file); + zip.start_file("deep/solve.py", zip::write::SimpleFileOptions::default()) + .unwrap(); + zip.write_all(b"def double(x):\n return x * 2\n") + .unwrap(); + zip.finish().unwrap(); + let config = Config { + owners: vec![OwnerOverride { + path: "mystery.zip".into(), + student: "alice".into(), + }], + ..Default::default() + }; + let input = load_local_input( + &[bench.path().join("subs")], + LocalInputOptions { + matching: Some(&config), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + input.students.len(), + 1, + "no phantom owner from the archive stem" + ); + assert_eq!(input.students[0].key().to_string(), "local:alice"); + assert_eq!(input.students[0].files().len(), 1); +} + +#[test] +fn invalid_rules_are_refused_before_execution() { + let bench = Bench::new(); + bench.write( + "subs/alice_solve.py", + "raise RuntimeError('must not run')\n", + ); + for invalid in [ + "[[matching.students]]\nkind = 'regex'\nregex = '(.*)'\n", + "[[matching.students]]\nkind = 'pattern'\npattern = '*.py'\n", + "[[matching.items]]\nid = 'double'\nfunctions = { typo = ['f'] }\n", + "[[matching.items]]\nid = 'ghost'\nfiles = ['solve.py']\n", + ] { + bench.config(invalid); + assert!(!bench.run("grade").status.success()); + assert!(!bench.path().join("out/result.json").exists()); + } +} + +#[test] +fn canvas_source_identity_and_attachment_origin_survive_teacher_file_rules() { + use scriptmark::input::canvas::{CanvasPayload, DownloadedAttachment, normalize}; + use scriptmark::models::{Assignment, AttemptPolicy, FileOrigin}; + use std::collections::HashMap; + let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/hw1/canvas"); + let payload: CanvasPayload = + serde_json::from_slice(&std::fs::read(fixture.join("assignment.json")).unwrap()).unwrap(); + let downloads = HashMap::from([( + 1001, + Ok(DownloadedAttachment::file( + fixture.join("files/1001/lab1.py"), + )), + )]); + let input = normalize( + &payload, + None, + &downloads, + AttemptPolicy::Latest, + Assignment::default(), + ); + let specs = [scriptmark::spec_loader::load_spec_str( + &SPEC.replace("solve.py", "lab1.py"), + Path::new("."), + ) + .unwrap()]; + let config: Config = toml::from_str("[[students]]\nkind = 'pattern'\npattern = '{student}*.py'\n[[owners]]\npath = 'lab1.py'\nstudent = 'wrong'\n").unwrap(); + let student = input + .students + .iter() + .find(|s| s.identity.canvas_user_id == Some(101)) + .unwrap(); + let matched = scriptmark::matching::item_match(&config, student, &specs[0]); + assert_eq!(matched.student, "2024010001"); + assert_eq!( + matched.owner.as_ref().unwrap().selected.as_deref(), + Some("2024010001") + ); + assert!(matches!( + matched.origin, + Some(FileOrigin::Attachment { + attachment_id: 1001, + .. + }) + )); +} diff --git a/examples/bundles/matching/assignment.toml b/examples/bundles/matching/assignment.toml new file mode 100644 index 0000000..6109279 --- /dev/null +++ b/examples/bundles/matching/assignment.toml @@ -0,0 +1,11 @@ +[assignment] +name = "matching" + +[[matching.students]] +kind = "directory" +level = 0 + +[[matching.items]] +id = "double" +files = ["solution.py"] +functions = { double = ["times_two"] } diff --git a/examples/bundles/matching/submissions/alice/deep/solution.py b/examples/bundles/matching/submissions/alice/deep/solution.py new file mode 100644 index 0000000..b11d85a --- /dev/null +++ b/examples/bundles/matching/submissions/alice/deep/solution.py @@ -0,0 +1,2 @@ +def times_two(value: int) -> int: + return value * 2 diff --git a/examples/bundles/matching/tests/test_double.toml b/examples/bundles/matching/tests/test_double.toml new file mode 100644 index 0000000..a0ace71 --- /dev/null +++ b/examples/bundles/matching/tests/test_double.toml @@ -0,0 +1,15 @@ +[meta] +name = "double" +file = "solve.py" +function = "double" +language = "python" + +[[cases]] +name = "positive" +args = [3] +expect = 6 + +[[cases]] +name = "negative" +args = [-4] +expect = -8 From 19610695547b67f99eb2a38d10a4c3b180fe4206 Mon Sep 17 00:00:00 2001 From: Acture Date: Fri, 2 Oct 2026 23:55:52 +0800 Subject: [PATCH 2/2] docs: link published P-673 matching notes --- README.md | 3 ++- notes | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9b53610..bc68077 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,8 @@ print(spec.name, spec.function, spec.num_cases) Teachers can configure student ownership, item file patterns, function aliases and per-student overrides in `assignment.toml`. Preview decisions and candidates with -`scriptmark match`; unresolved conflicts withhold grades. See [matching rules](docs/matching.md). +`scriptmark match`; unresolved conflicts withhold grades. See +[matching rules](https://github.com/Acture/obsidian-vault/blob/project/scriptmark/scriptmark/docs/matching.md). ```toml [meta] diff --git a/notes b/notes index c22509d..16f98fb 160000 --- a/notes +++ b/notes @@ -1 +1 @@ -Subproject commit c22509d6a3a679c00ad77950f7fcc2b2c0406216 +Subproject commit 16f98fbfc61e8d32c1e381c5d9f5c57089f75387