education/coursebank
1
0
Fork 0
Code Issues Pull Requests 1 Actions Releases 1 Activity

feat: improvement
Pipeline / check (pull_request) Successful in 5m0s
Details
Pipeline / docs (pull_request) Skipped
Details
Pipeline / nightly (pull_request) Skipped
Details
Pipeline / release (pull_request) Skipped
Details

Browse Source
This commit is contained in:
alexm
2026-09-27 01:11:09 -04:00
parent 5ac1e317c0
commit c6d6ee10b6
18 changed files with 2538 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/analysis/calibrate.rs
+105 -41
View File
@@ -28,18 +28,19 @@
use std::collections::{BTreeMap, BTreeSet};
use std::path::PathBuf;
use crate::SCHEMA_VERSION;
use crate::assessment::AssessmentFile;
use crate::bank::BankFile;
use crate::calibration::{CalibrationFile, Measurement, MeasurementFile, MeasurementMeta};
use crate::catalog::Catalog;
use crate::classical::{self, Analysis, ItemAnalysis, Thresholds};
use crate::date::Date;
use crate::error::{Error, Result};
use crate::irt::{self, Fit};
use crate::item::{Calibration, IrtParams, Item, OptionStat, VariantCalibration};
use crate::layout::Layout;
use crate::responses::ResponseSet;
use crate::store::Store;
use crate::taxonomy::Flag;
use crate::yaml;
/// What calibration would change about one item.
#[derive(Debug, Clone)]
@@ -615,60 +616,123 @@ fn diff_calibration(previous: Option<&Calibration>, next: &Calibration) -> Vec<S
out
}
/// Applies a plan, rewriting the affected bank files.
/// Applies a plan, rewriting the calibration store.
///
/// Files are rewritten one at a time and each is re-read before editing, so a plan
/// built against a bank that has since changed on disk fails loudly rather than
/// clobbering the newer version.
/// One file, `analysis/calibration.yaml`, and never a bank. A bank is reviewed
/// for what it asks; its history should be a record of wording decisions, not
/// of every grading run. The store is re-read immediately before editing, so a
/// plan built against a store that has since changed on disk fails loudly
/// rather than clobbering the newer version.
///
/// # Arguments
///
/// * `layout` - the course layout, for where the store lives.
/// * `plan` - the plan to apply.
///
/// # Returns
///
/// The bank files rewritten.
/// The file written.
///
/// # Errors
///
/// Returns [`Error::Unresolved`] when an item in the plan is no longer in its bank,
/// and [`Error::Io`] on a write failure.
pub fn apply(plan: &Plan) -> Result<Vec<PathBuf>> {
// Group by file so each is read and written once.
let mut by_file: BTreeMap<&PathBuf, Vec<&Change>> = BTreeMap::new();
/// Returns [`Error::Io`] on a write failure and [`Error::Yaml`] if the existing
/// store does not parse.
pub fn apply(layout: &Layout, plan: &Plan) -> Result<PathBuf> {
let path = layout.calibration_file();
let mut store = CalibrationFile::load(&path)?;
for change in &plan.changes {
by_file.entry(&change.path).or_default().push(change);
store
.items
.insert(change.uid.clone(), change.calibration.clone());
}
let mut written = Vec::new();
for (path, changes) in by_file {
let mut bank: BankFile = yaml::read(path)?;
for change in changes {
// The uid is `bank::item`; match on the item part.
let item_id = change
.uid
.split_once("::")
.map(|(_, id)| id)
.unwrap_or(&change.uid);
let target = bank.items.iter_mut().find(|i| i.id == item_id);
match target {
Some(item) => item.calibration = Some(change.calibration.clone()),
None => {
return Err(Error::Unresolved {
kind: "item",
id: change.uid.clone(),
context: Some(format!(
"{} — the bank changed since the plan was built; re-run calibration",
path.display()
)),
});
}
}
}
yaml::write(path, &bank)?;
written.push(path.clone());
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent).map_err(|e| Error::io(parent, e))?;
}
Ok(written)
store.save(&path)?;
Ok(path)
}
/// Records what one administration measured, as a file that is never rewritten.
///
/// The audit trail under the pooled store: these are the numbers one exam
/// produced, on a day, under a named model. Keeping them means the history
/// survives losing `data/`, which is ignored by git precisely because every row
/// of it carries a student.
///
/// # Arguments
///
/// * `layout` - the course layout.
/// * `administration` - the administration id, which names the file.
/// * `analysis` - the classical analysis of that administration.
/// * `catalog` - the loaded course, for the digests each item was measured
/// against.
/// * `record` - the assessment record, for the term and date.
///
/// # Returns
///
/// The two files written: one row per question, one row per question and
/// option.
///
/// # Errors
///
/// Returns [`Error::Usage`] when a record for this administration already
/// exists, since an administration happened once.
pub fn record_measurements(
layout: &Layout,
administration: &str,
analysis: &Analysis,
catalog: &Catalog,
record: Option<&AssessmentFile>,
) -> Result<Vec<PathBuf>> {
let mut items = Vec::new();
for item in &analysis.items {
let Some(uid) = &item.item_ref else { continue };
let entry = catalog.get(uid);
items.push(Measurement {
item: uid.clone(),
number: item.number,
variant: item.variant.clone(),
stem_digest: entry.map(|e| e.item.stem_digest()),
n: item.n,
p_value: Some(round4(item.p_value)),
point_biserial: item.point_biserial.map(round4),
discrimination_index: item.discrimination_index.map(round4),
key: item.key.clone(),
option_stats: item
.options
.iter()
.map(|(id, o)| (id.clone(), o.to_option_stat()))
.collect(),
irt: None,
flags: item.flags.clone(),
});
}
let file = MeasurementFile {
schema_version: SCHEMA_VERSION.to_string(),
administration: MeasurementMeta {
id: administration.to_string(),
assessment: record
.map(|r| r.assessment.id.clone())
.unwrap_or_else(|| administration.to_string()),
term: record.and_then(|r| r.assessment.term.clone()),
date: record.and_then(|r| r.assessment.date),
forms: record
.map(|r| r.forms.iter().map(|f| f.id.clone()).collect())
.unwrap_or_default(),
// The cohort, taken as the largest per-item n: a student who
// skipped question 7 still sat the exam.
n_examinees: analysis.items.iter().map(|i| i.n).max().unwrap_or(0),
model: None,
generated: Some(Date::today()),
coursebank: Some(crate::VERSION.to_string()),
},
items,
};
file.write_csv(&layout.measurements())
}
/// Builds a plan for a single administration, from an in-memory analysis.
src/authoring/jsonschema.rs
+127 -10
View File
@@ -37,6 +37,10 @@ pub enum Kind {
Lecture,
/// `objectives/*.yaml`.
Objective,
/// `analysis/calibration.yaml`.
Calibration,
/// `analysis/administrations/*.yaml`.
Measurements,
/// `banks/*.yaml`.
Bank,
/// `assessments/*.yaml`.
@@ -45,13 +49,15 @@ pub enum Kind {
impl Kind {
/// Every kind.
pub const ALL: [Kind; 6] = [
pub const ALL: [Kind; 8] = [
Kind::Course,
Kind::References,
Kind::Lecture,
Kind::Objective,
Kind::Bank,
Kind::Assessment,
Kind::Calibration,
Kind::Measurements,
];
/// The file name a schema is written to.
@@ -61,6 +67,8 @@ impl Kind {
Kind::References => "references.schema.json",
Kind::Lecture => "lecture.schema.json",
Kind::Objective => "objective.schema.json",
Kind::Calibration => "calibration.schema.json",
Kind::Measurements => "measurements.schema.json",
Kind::Bank => "bank.schema.json",
Kind::Assessment => "assessment.schema.json",
}
@@ -99,6 +107,8 @@ pub fn schema(kind: Kind) -> Value {
Kind::References => references_schema(),
Kind::Lecture => lecture_fragment_schema(),
Kind::Objective => objective_fragment_schema(),
Kind::Calibration => calibration_file_schema(),
Kind::Measurements => measurements_file_schema(),
Kind::Bank => bank_schema(),
Kind::Assessment => assessment_schema(),
}
@@ -381,9 +391,12 @@ fn target_schema() -> Value {
"order": {
"type": "integer",
"minimum": 1,
"description": "Position among the other targets of the same objective, low \
first. Ordered within its objective rather than across the \
course, so inserting one renumbers nothing outside its group."
"description": "Ignored since 2.0. An objective's targets are a set of question \
templates, not steps in a sequence — they are not taught in \
order and an exam samples from them — so a position asserts an \
order that does not exist. Where one target depends on another, \
say so with `prerequisites`. `coursebank migrate order` removes \
this."
},
"level_ceiling": level(),
"prerequisites": string_array(
@@ -412,8 +425,10 @@ fn objective_schema() -> Value {
"order": {
"type": "integer",
"minimum": 1,
"description": "Position in teaching order, low first. Without it objectives \
sort by id, which puts one before its own prerequisite."
"description": "Position in teaching order, low first. Derived since 2.0 from \
the position of this objective in a lecture's `teaches` list; \
an authored value still wins, and `coursebank migrate order` \
removes them."
},
"level_ceiling": level(),
"prerequisites": string_array(
@@ -1058,6 +1073,106 @@ fn option_history_schema() -> Value {
})
}
/// The schema for `analysis/calibration.yaml`.
fn calibration_file_schema() -> Value {
json!({
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": format!("{BASE}/calibration.schema.json"),
"title": "coursebank calibration store",
"description": "The pooled statistics, by item id. Kept out of the banks: a bank's diff \
should be a change of intent, not the output of a grading run. Committed \
— every number here is a cohort aggregate, and there is no field for a \
student.",
"type": "object",
"additionalProperties": false,
"properties": {
"schema_version": { "type": ["string", "number"] },
"items": {
"type": "object",
"description": "Keyed by item id, which since 2.0 names the item course-wide and \
carries no file name, so moving a question between banks does \
not orphan its statistics.",
"additionalProperties": calibration_schema()
}
}
})
}
/// The schema for one file under `analysis/administrations/`.
fn measurements_file_schema() -> Value {
json!({
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": format!("{BASE}/measurements.schema.json"),
"title": "coursebank administration record",
"description": "What one administration measured. Written once and not rewritten, like \
a seal: it records a thing that happened on a day. Cohort aggregates \
only — no per-section or per-student breakdown, because a small cell \
crossed with anything else stops being an aggregate.",
"type": "object",
"required": ["administration"],
"additionalProperties": false,
"properties": {
"schema_version": { "type": ["string", "number"] },
"administration": {
"type": "object",
"required": ["id", "assessment", "n_examinees"],
"additionalProperties": false,
"properties": {
"id": { "type": "string" },
"assessment": { "type": "string" },
"term": { "type": "string" },
"date": date("When it was given."),
"forms": string_array("The forms in play."),
"n_examinees": {
"type": "integer",
"minimum": 0,
"description": "The number to read before any of the others: a \
point-biserial on twenty-seven students is a different \
kind of claim than one on three hundred."
},
"model": { "type": "string" },
"generated": date("When the analysis was run."),
"coursebank": { "type": "string" }
}
},
"items": {
"type": "array",
"items": {
"type": "object",
"required": ["item", "number", "n"],
"additionalProperties": false,
"properties": {
"item": { "type": "string" },
"number": { "type": "integer", "minimum": 1 },
"variant": { "type": "string" },
"stem_digest": { "type": "string" },
"n": { "type": "integer", "minimum": 0 },
"p_value": proportion("Proportion correct on this administration."),
"point_biserial": { "type": "number", "minimum": -1.0, "maximum": 1.0 },
"discrimination_index": {
"type": "number", "minimum": -1.0, "maximum": 1.0
},
"option_stats": {
"type": "object",
"additionalProperties": option_stat_schema()
},
"irt": irt_schema(),
"flags": {
"type": "array",
"items": {
"type": "string",
"enum": strings(
&Flag::ALL.iter().map(|f| f.as_str()).collect::<Vec<&str>>()
)
}
}
}
}
}
}
})
}
/// The schema for a retirement record.
fn retirement_schema() -> Value {
json!({
@@ -1080,9 +1195,12 @@ fn item_identity_properties() -> Value {
json!({
"id": {
"type": "string",
"pattern": "^q-[a-z0-9]+(-[a-z0-9]+)*-[0-9]{3}$",
"description": "Item id, e.g. q-glycolysis-014. Stable forever: assessment records \
and stored responses refer to it."
"pattern": "^q-[a-z0-9]+(-[a-z0-9]+)*$",
"description": "Item id, e.g. q-glycolysis-rate-limiting-step. Stable forever: \
assessment records and stored responses refer to it, so renaming one \
is a migration rather than an edit. A trailing counter is no longer \
expected — it recorded when the item was written, which git knows — \
but an id that still has one stays valid."
},
"version": {
"type": "integer",
@@ -1149,7 +1267,6 @@ fn item_content_properties() -> Value {
"prerequisites": string_array("Objective ids a student needs before this item."),
"assets": { "type": "array", "items": asset_schema() },
"design": design_schema(),
"calibration": calibration_schema(),
"review": review_schema(),
"history": {
"type": "array",
src/cli.rs
+39
View File
@@ -137,6 +137,37 @@ pub(crate) enum MigrateCommand {
#[arg(long)]
dry_run: bool,
},
/// Drop the trailing counter from every item id.
///
/// A rename, not a normalization: `q-x-001` and `q-x` are unrelated
/// strings, so banks, records, seals, and the response store are rewritten
/// in one pass or not at all. Two ids that would collide abort it.
Counters {
/// Show the renames, and write nothing.
#[arg(long)]
dry_run: bool,
},
/// Replace the `order:` integers with ordered declarations.
///
/// Objective order comes from each lecture's `teaches` list, target order
/// from a `targets:` list this writes onto each objective. Run
/// `migrate split` first — without `teaches`, objective order has no
/// source.
Order {
/// Show what would change, and write nothing.
#[arg(long)]
dry_run: bool,
},
/// Turn citations written into `note:` fields into real fields.
///
/// Journal, volume, pages, and DOI parsed out of the prose, with whatever
/// the note still says left in it. A field the entry already declares is
/// never overwritten; a disagreement is reported instead.
References {
/// Show what would change, and write nothing.
#[arg(long)]
dry_run: bool,
},
/// Fill in the stored `variant` column from the assessment records.
///
/// Nothing in the tool needs it — a variant is derived from the placement
@@ -724,6 +755,14 @@ pub(crate) enum AnalyzeCommand {
/// Pool every stored administration of this assessment.
#[arg(long)]
pooled: bool,
/// Also write the administration record under analysis/.
///
/// Two CSVs, one row per question and one per question-and-option,
/// written once and never rewritten. Cohort aggregates only, so unlike
/// the response data they are meant to be committed — which is what
/// keeps the history when `data/` rotates.
#[arg(long = "record")]
record: bool,
},
/// Fit an IRT model.
Irt {
src/commands/analysis.rs
+44 -7
View File
@@ -294,12 +294,46 @@ pub(crate) fn analyze(cli: &Cli, sub: &AnalyzeCommand) -> Result<Outcome> {
let store = Store::open(catalog.layout.data())?;
match sub {
AnalyzeCommand::Items { id, pooled } => {
AnalyzeCommand::Items {
id,
pooled,
record: write_record,
} => {
let record = load_record(&catalog, id)?;
let set = responses_for(&store, &catalog, &record, *pooled)?;
let analysis =
classical::analyze(&set, &Thresholds::default(), Some(&record), Some(&catalog));
if *write_record {
// The full administration id, not the assessment id: `e1` is
// given again next year, and a record named after it would
// collide with this cohort's — which `write_csv` would report
// as "already recorded" when it is a different exam entirely.
let administration = coursebank::responses::administration_id(
&catalog.course.course.code,
record
.assessment
.term
.as_deref()
.unwrap_or(&catalog.course.course.term),
&record.assessment.id,
);
let written = calibrate::record_measurements(
&catalog.layout,
&administration,
&analysis,
&catalog,
Some(&record),
)?;
for path in &written {
println!("wrote {}", path.display());
}
println!(
"\nCohort aggregates only, so these are committed. Review with\n git diff \
analysis/\n"
);
}
for w in &analysis.warnings {
println!("! {w}\n");
}
@@ -463,17 +497,20 @@ pub(crate) fn calibrate(cli: &Cli, args: &CalibrateArgs) -> Result<Outcome> {
}
if !args.apply {
println!(
"Nothing written. Re-run with --apply to write these {} change(s) into the bank \
files, then review the git diff.",
"Nothing written. Re-run with --apply to write these {} change(s) into \
analysis/calibration.yaml, then review the git diff.",
plan.changes.len()
);
return Ok(Outcome::Ok);
}
for path in calibrate::apply(&plan)? {
println!("updated {}", path.display());
}
println!("\nReview the diff before committing: git diff banks/");
let path = calibrate::apply(&catalog.layout, &plan)?;
println!("updated {}", path.display());
println!(
"\nReview the diff before committing: git diff analysis/\n\nThe banks are untouched. \
Statistics are cohort aggregates with no student in\n them, which is why analysis/ is \
committed and data/ is not."
);
Ok(Outcome::Ok)
}
src/commands/project.rs
+106 -1
View File
@@ -35,10 +35,15 @@ use crate::helpers::{load, truncate};
/// The `.gitignore` written by `init`.
pub(crate) const GITIGNORE: &str = "\
# Generated output: exports, rendered exams, reports.
# Generated output: exports, rendered exams, reports. A student report carries
# names, so it belongs here rather than in the repository.
build/
reports/
# Response data: every row carries a student. The statistics derived from it are
# cohort aggregates and live in analysis/, which is committed on purpose.
data/
# Typst and PDF artifacts.
*.pdf
@@ -109,9 +114,105 @@ pub(crate) fn migrate(cli: &Cli, sub: &MigrateCommand) -> Result<Outcome> {
MigrateCommand::Options { dry_run } => migrate_options(cli, *dry_run),
MigrateCommand::Stems { dry_run } => migrate_stems(cli, *dry_run),
MigrateCommand::Variants { dry_run } => migrate_variants(cli, *dry_run),
MigrateCommand::References { dry_run } => migrate_references(cli, *dry_run),
MigrateCommand::Order { dry_run } => migrate_order(cli, *dry_run),
MigrateCommand::Counters { dry_run } => migrate_counters(cli, *dry_run),
}
}
/// Drops the trailing counter from every item id.
fn migrate_counters(cli: &Cli, dry_run: bool) -> Result<Outcome> {
let (rename, touched) = migrate::counters(&cli.course, !dry_run)?;
if rename.is_empty() {
println!("nothing to do: no item id ends in a counter");
return Ok(Outcome::Ok);
}
for (old, new) in &rename {
println!(" {old} -> {new}");
}
println!();
for (path, n) in &touched {
println!(" {:<44} {n:>5} reference(s)", path.display());
}
if dry_run {
println!("\nnothing written");
return Ok(Outcome::Ok);
}
let sealed = touched
.iter()
.filter(|(p, _)| p.starts_with("seals"))
.count();
println!(
"\nrenamed {} item(s) across {} file(s)",
rename.len(),
touched.len()
);
if sealed > 0 {
println!(
"\n{sealed} seal(s) were rewritten. The ids are inside the digest, so each one was \
recomputed\n and the previous digest recorded under `superseded_digests`. A seal \
that has been\n rewritten says so rather than looking untouched."
);
}
println!(
"\nNext:\n coursebank validate\n coursebank seal verify\n coursebank analyze items \
--all # the join key moved; check the data still lands"
);
Ok(Outcome::Ok)
}
/// Replaces the order integers with ordered declarations.
fn migrate_order(cli: &Cli, dry_run: bool) -> Result<Outcome> {
let touched = migrate::order(&cli.course, !dry_run)?;
if touched.is_empty() {
println!("nothing to do: no `order:` left to derive");
return Ok(Outcome::Ok);
}
let total: usize = touched.iter().map(|(_, n)| n).sum();
for (path, n) in &touched {
println!(" {:<44} {n:>5} order(s) dropped", path.display());
}
if dry_run {
println!("\nnothing written");
return Ok(Outcome::Ok);
}
println!(
"\nrewrote {} file(s), {total} integer(s) gone\n\nNext:\n coursebank validate\n \
coursebank lecture objectives L1.2 # check the order still reads right",
touched.len()
);
Ok(Outcome::Ok)
}
/// Takes the citations out of the notes and puts them in fields.
fn migrate_references(cli: &Cli, dry_run: bool) -> Result<Outcome> {
let (touched, notes) = migrate::references(&cli.course, !dry_run)?;
for (path, n) in &touched {
println!(" {:<44} {n:>5} note(s) taken apart", path.display());
}
for note in &notes {
println!(" note: {note}");
}
if touched.is_empty() {
println!("nothing to do: no note is carrying a citation");
return Ok(Outcome::Ok);
}
if dry_run {
println!("\nnothing written");
return Ok(Outcome::Ok);
}
println!(
"\nrewrote {} file(s)\n\nNext:\n coursebank validate\n coursebank references list\n\n\
An issue number is never inferred, not even from a DOI that encodes one, so add those \
by hand.",
touched.len()
);
Ok(Outcome::Ok)
}
/// Fills in the stored variant column.
fn migrate_variants(cli: &Cli, dry_run: bool) -> Result<Outcome> {
let touched = migrate::store_variants(&cli.course, !dry_run)?;
@@ -590,6 +691,10 @@ pub(crate) fn validate(cli: &Cli) -> Result<Outcome> {
let seals = coursebank::seal::SealFile::load_all(&catalog.layout.seals())?;
all.extend(catalog.validate_seals(&seals));
// The statistics are kept in a different file from the questions they
// describe, so the link between them is worth checking rather than assuming.
all.extend(catalog.calibration.validate(&catalog));
if all.is_empty() {
if !cli.quiet {
println!(
src/lib.rs
+4 -2
View File
@@ -106,9 +106,11 @@ pub mod migrate;
pub mod model;
pub mod util;
pub use util::{date, hash, markup, rng, yaml, zipfile};
pub use util::{citation, date, hash, markup, rng, yaml, zipfile};
pub use model::{assessment, bank, catalog, course, history, item, layout, seal, taxonomy};
pub use model::{
assessment, bank, calibration, catalog, course, history, item, layout, seal, taxonomy,
};
pub use course::fragment;
src/migrate.rs
+634 -5
View File
@@ -19,6 +19,9 @@
//! | [`options_plan`] and [`apply_options`] | option letters into names | a letter is a position, not an identity |
//! | [`stems`] | drops `version:` and `history:` | a stem's text is its identity |
//! | [`store_variants`] | fills the stored `variant` column | so other readers can group by option set |
//! | [`references`] | prose notes into citation fields | a DOI in a note is a link nobody can follow |
//! | [`order`] | removes the `order:` integers | objective order is derived, and targets have none |
//! | [`counters`] | drops the `-001` from item ids | a per-bank sequence number is not part of a name |
//!
//! # What the option migration does not touch
//!
@@ -96,7 +99,7 @@ pub struct Plan {
impl Plan {
/// The number of lines each planned file holds.
pub fn lines(&self) -> Vec<(PathBuf, usize)> {
pub fn lines(&self) -> Report {
self.files
.iter()
.map(|f| (f.path.clone(), f.text.lines().count()))
@@ -602,7 +605,7 @@ fn unqualify_line(line: &str) -> String {
///
/// Propagates read and write failures, including
/// [`Error::FeatureDisabled`] when the store is Parquet and the feature is off.
pub fn store_ids(root: &Path, write: bool) -> Result<Vec<(PathBuf, usize)>> {
pub fn store_ids(root: &Path, write: bool) -> Result<Report> {
let layout = Layout::new(root);
let store = Store::open(layout.data())?;
let mut out = Vec::new();
@@ -629,6 +632,15 @@ pub fn store_ids(root: &Path, write: bool) -> Result<Vec<(PathBuf, usize)>> {
Ok(out)
}
/// One file a migration changed, and how many things in it changed.
pub type Changed = (PathBuf, usize);
/// What a migration changed, file by file.
pub type Report = Vec<Changed>;
/// Old id to new id, for the migrations that rename things.
pub type Renames = BTreeMap<String, String>;
/// The per-item map from a pre-2.0 option letter to the name that replaces it.
pub type OptionMap = BTreeMap<String, BTreeMap<String, String>>;
@@ -844,7 +856,7 @@ pub fn options_plan(root: &Path) -> Result<(OptionMap, Vec<String>)> {
/// # Errors
///
/// Propagates read and write failures.
pub fn apply_options(root: &Path, map: &OptionMap, write: bool) -> Result<Vec<(PathBuf, usize)>> {
pub fn apply_options(root: &Path, map: &OptionMap, write: bool) -> Result<Report> {
let layout = Layout::new(root);
let mut out = Vec::new();
@@ -1132,7 +1144,7 @@ fn key_at_word(line: &str, key: &str) -> Option<usize> {
/// # Errors
///
/// Propagates read and write failures.
pub fn stems(root: &Path, write: bool) -> Result<Vec<(PathBuf, usize)>> {
pub fn stems(root: &Path, write: bool) -> Result<Report> {
let layout = Layout::new(root);
let mut out = Vec::new();
@@ -1221,7 +1233,7 @@ fn is_key(trimmed: &str, key: &str) -> bool {
/// # Errors
///
/// Propagates catalog, record, and store failures.
pub fn store_variants(root: &Path, write: bool) -> Result<Vec<(PathBuf, usize)>> {
pub fn store_variants(root: &Path, write: bool) -> Result<Report> {
let layout = Layout::new(root);
let catalog = crate::catalog::Catalog::load(root)?;
@@ -1265,6 +1277,475 @@ pub fn store_variants(root: &Path, write: bool) -> Result<Vec<(PathBuf, usize)>>
Ok(out)
}
/// Drops the trailing counter from every item id.
///
/// `q-fastq-quality-line-001` becomes `q-fastq-quality-line`. The counter was a
/// per-bank sequence number, which says when an item was written — something git
/// knows — and collides with another bank's numbering the moment an item moves.
///
/// This is a rename, not a normalization, and that distinction decides how it
/// has to be done. Stripping `bank::` from an id recovered a name that was
/// already inside it, so an old reference could simply be read as the new one.
/// A counter carries no such fallback: `q-x-001` and `q-x` are two unrelated
/// strings, and every file that names one has to be rewritten in the same pass
/// or the join to four terms of response data quietly breaks.
///
/// So: banks, assessment records, seals, and the response store, or nothing.
/// Two ids that would collide after stripping abort the whole migration rather
/// than merging two questions into one.
///
/// # Arguments
///
/// * `root` - the course directory.
/// * `write` - whether to write, or only to count.
///
/// # Returns
///
/// The renames, and one entry per file that changes with how many references it
/// holds.
///
/// # Errors
///
/// Returns [`Error::Invalid`] when two ids would collide, listing both, and
/// propagates load and write failures.
pub fn counters(root: &Path, write: bool) -> Result<(Renames, Report)> {
let layout = Layout::new(root);
let catalog = crate::catalog::Catalog::load(root)?;
let mut rename: Renames = BTreeMap::new();
let mut taken: Renames = BTreeMap::new();
let mut collisions = Vec::new();
for entry in &catalog.entries {
let Some(bare) = strip_counter(&entry.uid) else {
taken.insert(entry.uid.clone(), entry.uid.clone());
continue;
};
if let Some(first) = taken.get(&bare) {
collisions.push(format!(
"`{}` and `{first}` would both become `{bare}`. Give one of them a name that \
says what it asks rather than when it was written.",
entry.uid
));
continue;
}
taken.insert(bare.clone(), entry.uid.clone());
rename.insert(entry.uid.clone(), bare);
}
if !collisions.is_empty() {
return Err(Error::Invalid(collisions));
}
if rename.is_empty() {
return Ok((rename, Vec::new()));
}
let mut touched = Vec::new();
// Banks: the definition, and any `supersedes` pointing at one.
for path in yaml::list_yaml(&layout.banks())? {
let text = std::fs::read_to_string(&path).map_err(|e| Error::io(&path, e))?;
let (out, n) = rename_ids(&text, &rename, &["- id", "supersedes"]);
if n > 0 {
if write {
yaml::write_text(&path, &out)?;
}
touched.push((relative(root, &path), n));
}
}
// Records: the `item:` on every placement.
for path in yaml::list_yaml(&layout.assessments())? {
let text = std::fs::read_to_string(&path).map_err(|e| Error::io(&path, e))?;
let (out, n) = rename_ids(&text, &rename, &["item"]);
if n > 0 {
if write {
yaml::write_text(&path, &out)?;
}
touched.push((relative(root, &path), n));
}
}
// Seals go through the model, not the text, because the ids are inside the
// digest and a seal that has been rewritten has to say so.
for path in yaml::list_yaml(&layout.seals())? {
let mut seal = crate::seal::SealFile::load(&path)?;
let n = seal.rename_items(&rename);
if n > 0 {
if write {
seal.save(&path)?;
}
touched.push((relative(root, &path), n));
}
}
// The store, where the id is the join key.
let store = Store::open(layout.data())?;
for path in store.files()? {
let mut rows = store::read_flat(&path)?;
let mut n = 0;
for row in &mut rows {
if let Some(new) = rename.get(&row.item_ref) {
row.item_ref = new.clone();
n += 1;
}
}
if n > 0 {
if write {
store::write_flat(&path, &rows)?;
}
touched.push((relative(root, &path), n));
}
}
Ok((rename, touched))
}
/// An id with a trailing `-123` removed, or `None` when it has none.
fn strip_counter(id: &str) -> Option<String> {
let (head, tail) = id.rsplit_once('-')?;
if head.is_empty() || tail.is_empty() || !tail.chars().all(|c| c.is_ascii_digit()) {
return None;
}
Some(head.to_string())
}
/// Rewrites the value of the named keys wherever it is an id being renamed.
///
/// Keyed on the field name so that an id appearing in prose — a rationale that
/// mentions the item it replaced, a note — is left alone. A rename that edited
/// every matching string in the file would also edit the sentences about it.
fn rename_ids(text: &str, rename: &Renames, keys: &[&str]) -> (String, usize) {
let mut out: Vec<String> = Vec::new();
let mut count = 0;
for line in text.lines() {
let mut replaced = None;
for key in keys {
let Some(at) = field_value(line, key) else {
continue;
};
let value = &line[at.0..at.1];
// Canonicalized before the lookup, so a record that still carries a
// pre-2.0 `bank::` qualifier is renamed rather than skipped. Left
// alone it would keep pointing at an id the bank no longer has.
let Some(new) = rename.get(crate::item::canonical_id(value)) else {
continue;
};
replaced = Some(format!("{}{new}{}", &line[..at.0], &line[at.1..]));
count += 1;
break;
}
out.push(replaced.unwrap_or_else(|| line.to_string()));
}
let mut joined = out.join("\n");
if text.ends_with('\n') {
joined.push('\n');
}
(joined, count)
}
/// The byte range of a `key: value` scalar on one line, quotes excluded.
fn field_value(line: &str, key: &str) -> Option<(usize, usize)> {
let needle = format!("{key}:");
let at = line.find(&needle)?;
let before = &line[..at];
if before.chars().next_back().is_some_and(|c| {
c.is_ascii_alphanumeric() || c == '_' || (c == '-' && !key.starts_with('-'))
}) {
return None;
}
let rest = &line[at + needle.len()..];
let lead = rest.len() - rest.trim_start().len();
let value = rest.trim_start();
let quoted = value.starts_with(['"', '\'']);
let start = at + needle.len() + lead + usize::from(quoted);
let inner = &line[start..];
let end = inner
.find(|c: char| {
c == '"' || c == '\'' || c == ',' || c == '}' || c == ']' || c.is_whitespace()
})
.map(|n| start + n)
.unwrap_or(line.len());
(end > start).then_some((start, end))
}
/// Removes the hand-kept `order:` integers.
///
/// Nothing is written in their place. An objective's teaching order is its
/// position in a lecture's `teaches` list, which `migrate split` already
/// produced. A target has no order at all — an objective's targets are a set of
/// question templates, sampled from rather than worked through — so the integer
/// was asserting a sequence that was never taught. Where a list has to be
/// printed, [`crate::course::CourseFile::targets`] orders it by ceiling.
///
/// Two hundred and four integers in a real course, each of which had to be
/// bumped by hand when a target was inserted in the middle.
///
/// # Arguments
///
/// * `root` - the course directory.
/// * `write` - whether to write, or only to count.
///
/// # Returns
///
/// One entry per file that changes, with how many `order:` lines it loses.
///
/// # Errors
///
/// Propagates catalog and write failures, and refuses when the lectures do not
/// declare what they teach, since objective order would then have no source.
pub fn order(root: &Path, write: bool) -> Result<Report> {
let layout = Layout::new(root);
let course = CourseFile::load_dir(root)?;
if course.lectures.values().all(|l| l.teaches.is_empty())
&& !course.learning_objectives.is_empty()
{
return Err(Error::usage(
"no lecture declares what it teaches, so objective order would have nothing to come from. Run `coursebank migrate split` first."
.to_string(),
));
}
let mut out = Vec::new();
let mut files = vec![layout.course_file()];
files.extend(yaml::list_yaml(&layout.objectives())?);
for path in files {
if !path.is_file() {
continue;
}
let text = std::fs::read_to_string(&path).map_err(|e| Error::io(&path, e))?;
let (rewritten, dropped) = rewrite_order(&text);
if rewritten == text {
continue;
}
if write {
yaml::write_text(&path, &rewritten)?;
}
out.push((relative(root, &path), dropped));
}
Ok(out)
}
/// Removes every `order:` line.
///
/// Nothing replaces them. An objective's position comes from where its lecture
/// lists it in `teaches`, and a target has no position to derive: an
/// objective's targets are a set of question templates rather than steps in a
/// sequence, so the integer was asserting an order that was never taught.
fn rewrite_order(text: &str) -> (String, usize) {
let mut out: Vec<&str> = Vec::new();
let mut dropped = 0;
for line in text.lines() {
if key_at(line, 4).as_deref() == Some("order") {
dropped += 1;
continue;
}
out.push(line);
}
let mut joined = out.join("\n");
if text.ends_with('\n') {
joined.push('\n');
}
(joined, dropped)
}
/// Turns the citations written into `note:` fields into real fields.
///
/// A hand-built bibliography collects entries whose journal, volume, pages, and
/// DOI are all sitting in the one field that means "anything else worth saying".
/// Nothing can use them there: a reading list cannot link a DOI it cannot see,
/// and an export to Hayagriva or BibTeX has no journal to put in `parent` or
/// `journal`.
///
/// Textual, like the rest of this module, and for a reason specific to this
/// file: a bibliography is usually ordered the way its author thinks about it —
/// books, then the papers that matter — and the model holds it in a map, so a
/// serde round trip would alphabetize all of it to change twenty entries.
///
/// A field the entry already declares is never overwritten. Where the note
/// disagrees with it, the note's version is left in place for a person to look
/// at rather than silently replacing something that was typed deliberately.
///
/// # Arguments
///
/// * `root` - the course directory.
/// * `write` - whether to write, or only to count.
///
/// # Returns
///
/// One entry per file that changes, with how many notes were taken apart, and
/// one message per disagreement found.
///
/// # Errors
///
/// Propagates read and write failures.
pub fn references(root: &Path, write: bool) -> Result<(Report, Vec<String>)> {
let layout = Layout::new(root);
let mut touched = Vec::new();
let mut notes = Vec::new();
// Either file may hold the bibliography: its own, or the course file that
// has not been split yet.
for path in [layout.references_file(), layout.course_file()] {
if !path.is_file() {
continue;
}
let text = std::fs::read_to_string(&path).map_err(|e| Error::io(&path, e))?;
let (rewritten, count, said) = expand_notes(&text);
notes.extend(said);
if count > 0 {
if write {
yaml::write_text(&path, &rewritten)?;
}
touched.push((relative(root, &path), count));
}
}
Ok((touched, notes))
}
/// Replaces each reference's `note:` line with the fields it was carrying.
fn expand_notes(text: &str) -> (String, usize, Vec<String>) {
let lines: Vec<String> = text.lines().map(str::to_string).collect();
let Some(section) = blocks(&lines, 0)
.into_iter()
.find(|b| b.key == "references")
else {
return (text.to_string(), 0, Vec::new());
};
// Which lines belong to which entry, so a note is parsed with its own year
// and checked against its own fields.
let mut rewritten: BTreeMap<usize, Vec<String>> = BTreeMap::new();
let mut said = Vec::new();
let mut count = 0;
let start = lines.len() - section.lines.len();
for entry in blocks(&section.body(), 2) {
let offset = lines
.iter()
.enumerate()
.skip(start)
.find(|(_, l)| **l == entry.lines[0])
.map(|(i, _)| i);
let Some(offset) = offset else { continue };
let Some((note_at, note)) = entry
.lines
.iter()
.enumerate()
.find_map(|(i, l)| scalar(l, 4, "note").map(|v| (i, v)))
else {
continue;
};
let year = entry
.lines
.iter()
.find_map(|l| scalar(l, 4, "year"))
.and_then(|v| v.parse::<u32>().ok());
let parsed = crate::citation::parse(&note, year);
if parsed.container.is_none() && parsed.doi.is_none() {
continue;
}
let declared = |field: &str| entry.lines.iter().any(|l| scalar(l, 4, field).is_some());
let mut out: Vec<String> = Vec::new();
for (field, value, quote) in [
("container", parsed.container.clone(), false),
("volume", parsed.volume.clone(), true),
("pages", parsed.pages.clone(), true),
("doi", parsed.doi.clone(), true),
] {
let Some(value) = value else { continue };
if declared(field) {
said.push(format!(
"`{}` already declares {field}; the note's `{value}` was left in place",
entry.key
));
continue;
}
out.push(format!(" {field}: {}", render(&value, quote)));
}
if let Some(note) = &parsed.note {
out.push(format!(" note: {}", render(note, true)));
}
// A note whose every part was already declared is not a change.
if out.is_empty() {
continue;
}
if said
.iter()
.any(|m| m.starts_with(&format!("`{}`", entry.key)))
&& parsed.note.is_some()
&& out.len() == 1
{
continue;
}
rewritten.insert(offset + note_at, out);
count += 1;
}
if count == 0 {
return (text.to_string(), 0, said);
}
let mut out: Vec<String> = Vec::new();
for (i, line) in lines.iter().enumerate() {
match rewritten.get(&i) {
Some(replacement) => out.extend(replacement.clone()),
None => out.push(line.clone()),
}
}
let mut joined = out.join("\n");
if text.ends_with('\n') {
joined.push('\n');
}
(joined, count, said)
}
/// The value of a `key: value` line at an exact indentation, unquoted.
fn scalar(line: &str, indent: usize, key: &str) -> Option<String> {
if key_at(line, indent).as_deref() != Some(key) {
return None;
}
let value = line[indent + key.len() + 1..].trim();
if value.is_empty() {
return None;
}
for quote in ['\'', '"'] {
if let Some(inner) = value
.strip_prefix(quote)
.and_then(|v| v.strip_suffix(quote))
{
return Some(inner.replace("''", "'"));
}
}
Some(value.to_string())
}
/// A YAML scalar, quoted when it has to be.
///
/// Numbers are quoted whether they need it or not: `volume: 48` reads back as an
/// integer and the field is a string, so the file would stop loading.
fn render(value: &str, quote: bool) -> String {
let risky = quote
|| value.contains(": ")
|| value.ends_with(':')
|| value.contains(" #")
|| value.starts_with([
'[', '{', '&', '*', '!', '|', '>', '%', '@', '`', '\'', '"', '-',
]);
if risky {
format!("'{}'", value.replace('\'', "''"))
} else {
value.to_string()
}
}
/// One block of YAML: a key, the lines under it, and the comments above it.
#[derive(Debug, Clone, Default)]
struct Block {
@@ -1952,6 +2433,154 @@ items:
);
}
#[test]
fn the_order_integers_are_dropped_and_nothing_replaces_them() {
let text = r#"learning_objectives:
lo-x:
text: 'Read the formats.'
unit: u1
order: 2
level_ceiling: 2
learning_targets:
t-a:
text: 'First.'
objective: lo-x
order: 1
level_ceiling: 1
t-b:
text: 'Second.'
objective: lo-x
order: 2
level_ceiling: 2
"#;
let (out, dropped) = rewrite_order(text);
assert_eq!(dropped, 3, "one on the objective, one on each target");
assert!(!out.contains("order:"), "{out}");
// Nothing is written in their place: the objective's position comes
// from its lecture's `teaches`, and a target has no position. Matched
// on the inserted form rather than on `targets:`, which is a substring
// of the `learning_targets:` section header.
assert!(!out.contains("targets: ["), "{out}");
// Everything else is untouched, prose and ceilings included.
assert!(out.contains("text: 'Read the formats.'"), "{out}");
assert!(out.contains("level_ceiling: 2"), "{out}");
assert!(out.contains(" t-a:"), "{out}");
// Idempotent.
let (again, dropped) = rewrite_order(&out);
assert_eq!(dropped, 0);
assert_eq!(again, out);
}
#[test]
fn a_note_becomes_the_fields_it_was_carrying() {
let text = r#"references:
altschul1997gapped:
label: GBLAST97
kind: article
role: supplemental
title: 'Gapped BLAST and PSI-BLAST'
year: 1997
note: 'Nucleic Acids Res 25:3389-3402. doi:10.1093/nar/25.17.3389'
brown2013next:
label: BSM
kind: book
title: Next-generation DNA sequencing informatics
year: 2013
"#;
let (out, count, said) = expand_notes(text);
assert_eq!(count, 1);
assert!(said.is_empty(), "{said:?}");
assert!(out.contains(" container: Nucleic Acids Res"), "{out}");
assert!(out.contains(" volume: '25'"), "{out}");
assert!(out.contains(" pages: '3389-3402'"), "{out}");
assert!(out.contains(" doi: '10.1093/nar/25.17.3389'"), "{out}");
// Nothing left of the note, and nothing invented to replace it.
assert!(!out.contains("note:"), "{out}");
assert!(!out.contains("issue:"), "{out}");
// The book had no note and is untouched, label and all.
assert!(out.contains(" label: BSM"), "{out}");
assert!(out.contains(" brown2013next:"), "{out}");
}
#[test]
fn a_field_already_declared_is_never_overwritten() {
let text = r#"references:
steinegger2017mmseqs2:
kind: article
title: MMseqs2
year: 2017
note: 'Nat Biotechnol 35:1026-1028. doi:10.1038/nbt.3988'
doi: '10.1038/nbt.3988'
"#;
let (out, _, said) = expand_notes(text);
assert!(
said.iter().any(|m| m.contains("already declares doi")),
"{said:?}"
);
// One doi line, the one that was typed deliberately.
assert_eq!(out.matches("doi:").count(), 1, "{out}");
assert!(out.contains(" container: Nat Biotechnol"), "{out}");
}
#[test]
fn a_trailing_counter_is_recognized_and_nothing_else_is() {
assert_eq!(
strip_counter("q-fastq-quality-line-001").as_deref(),
Some("q-fastq-quality-line")
);
assert_eq!(
strip_counter("q-blast-seed-14").as_deref(),
Some("q-blast-seed")
);
// Already bare.
assert_eq!(strip_counter("q-fastq-quality-line"), None);
// A number that is part of the name, not a counter after it.
assert_eq!(strip_counter("q-fastqc-3prime-decay"), None);
assert_eq!(strip_counter("q-001"), Some("q".to_string()));
assert_eq!(strip_counter("q-x-"), None);
}
#[test]
fn a_rename_touches_the_named_fields_and_leaves_prose_alone() {
let mut rename = BTreeMap::new();
rename.insert("q-x-001".to_string(), "q-x".to_string());
let bank = r#"items:
- id: q-x-001
supersedes: q-x-001
stem: Which line?
design:
rationale: >-
Replaces q-x-001, which had two defensible answers.
"#;
let (out, n) = rename_ids(bank, &rename, &["- id", "supersedes"]);
assert_eq!(n, 2, "the id and the supersedes, not the sentence");
assert!(out.contains(" - id: q-x\n"), "{out}");
assert!(out.contains(" supersedes: q-x\n"), "{out}");
// The rationale is prose about the item; editing it would rewrite the
// sentence as well as the reference.
assert!(out.contains("Replaces q-x-001, which had"), "{out}");
}
#[test]
fn a_record_that_never_dropped_its_bank_prefix_is_still_renamed() {
let mut rename = BTreeMap::new();
rename.insert("q-x-001".to_string(), "q-x".to_string());
let flow = " - { number: 1, item: \"b-1-2::q-x-001\", key: [o-a] }\n";
let (out, n) = rename_ids(flow, &rename, &["item"]);
assert_eq!(n, 1);
// Left alone it would point at an id the bank no longer has.
assert!(out.contains("item: \"q-x\""), "{out}");
let block = " item: q-x-001\n";
let (out, n) = rename_ids(block, &rename, &["item"]);
assert_eq!(n, 1);
assert_eq!(out, " item: q-x\n");
}
#[test]
fn lecture_slugs_match_the_house_naming() {
assert_eq!(lecture_slug("L1.2"), "l-1-2");
src/model.rs
+1
View File
@@ -28,6 +28,7 @@
pub mod assessment;
pub mod bank;
pub mod calibration;
pub mod catalog;
pub mod course;
pub mod history;
src/model/bank.rs
+6 -32
View File
@@ -530,38 +530,12 @@ fn validate_item(
}
// --- calibration plausibility ------
if let Some(c) = &it.calibration {
if let Some(p) = c.p_value {
if !(0.0..=1.0).contains(&p) {
issues.push(format!(
"calibration.p_value must be between 0 and 1, got {p}"
));
}
}
if let Some(r) = c.point_biserial {
if !(-1.0..=1.0).contains(&r) {
issues.push(format!(
"calibration.point_biserial must be between -1 and 1, got {r}"
));
}
}
for option in c.option_stats.keys() {
if it.option(option).is_none() {
issues.push(format!(
"calibration.option_stats has `{option}`, which is not an option of this item"
));
}
}
if let Some(irt) = &c.irt {
if irt.a <= 0.0 {
issues.push(format!("calibration.irt.a must be positive, got {}", irt.a));
}
if let Some(cp) = irt.c {
if !(0.0..1.0).contains(&cp) {
issues.push(format!("calibration.irt.c must be in [0, 1), got {cp}"));
}
}
}
if it.calibration.is_some() {
issues.push(
"has a `calibration:` block, but statistics live in analysis/calibration.yaml since \
2.0. A bank's diff should be a change of intent, not the output of a grading run."
.to_string(),
);
}
// --- retirement -----
src/model/calibration.rs
+838
View File
@@ -0,0 +1,838 @@
// SPDX-License-Identifier: Prosperity-3.0.0
// Copyright Scientific Computing Studio
// Source: https://git.scient.ing/education/coursebank
//! Where the statistics live, which is not in the bank.
//!
//! A bank file is a reviewed artifact: someone wrote the question, someone
//! argued about the distractors, and the diff on it should be a change of
//! intent. Statistics are neither reviewed nor intended — they are what
//! happened — and writing them back into the bank means every grading run
//! produces a diff on a file whose history is supposed to be about wording.
//!
//! So the evidence lives in `analysis/`, in two kinds of file:
//!
//! | File | Format | Rewritten? | Holds |
//! |:--|:--|:--|:--|
//! | `analysis/administrations/<id>-items.csv` | CSV | never | one row per question |
//! | `analysis/administrations/<id>-options.csv` | CSV | never | one row per question and option |
//! | `analysis/calibration.yaml` | YAML | by `calibrate` | the pooled per-item view |
//!
//! The format follows the shape. An administration record is a table — fixed
//! columns, one row per question, machine-written, never hand-edited — so it is
//! CSV: one line per item rather than fifteen, which diffs better, and it loads
//! straight into pandas or DuckDB, which is much of the point of committing it.
//! The pooled view is not a table. It is three levels deep, item to variant to
//! option history, with fitted parameters and variable-length lists, and as CSV
//! that would be three files joined by keys — a relational schema for the one
//! file a person actually reads in a pull request. That stays YAML.
//!
//! Neither is Parquet, and the reason is the review workflow: a binary file
//! shows nothing in a diff and cannot be merged. Parquet is right for `data/`
//! precisely because that is bulk, ignored, and never reviewed.
//!
//! An administration file is written once and not touched again, for the same
//! reason a seal is not: it is a record of a thing that happened on a day. The
//! calibration file is the accepted rollup — what `lint` compares your
//! predictions against, and what a report reads — and `calibrate` proposes
//! changes to it as a diff you review before committing.
//!
//! Both are meant to be committed. Neither can carry student data, and that is
//! a property of the types rather than a promise: there is no field for a
//! student key, an identifier, a section, or an ability estimate, and the
//! structures reject unknown keys, so a file carrying one fails to load rather
//! than being quietly accepted. Everything per-person stays in `data/`, which
//! is what your `.gitignore` is for.
//!
//! # Linking back to the bank
//!
//! By item id, which since 2.0 names the item course-wide and has no file name
//! in it, and by variant digest, which says which option set the numbers
//! describe. A record also carries the stem digest it was measured against, so
//! [`CalibrationFile::validate`] can say that an item has been reworded since —
//! the statistics then describe a question that no longer exists under that id.
use std::collections::BTreeMap;
use std::path::{Path, PathBuf};
use serde::{Deserialize, Serialize};
use crate::course::SCHEMA_VERSION;
use crate::date::Date;
use crate::error::{Error, Result};
use crate::item::{Calibration, IrtModel, IrtParams, OptionStat};
use crate::taxonomy::Flag;
use crate::yaml;
/// The file name of the pooled calibration store, under `analysis/`.
pub const CALIBRATION_FILE: &str = "calibration.yaml";
/// The pooled per-item statistics: `analysis/calibration.yaml`.
///
/// Keyed by item id. This is the file `calibrate` rewrites and the one
/// everything else reads; [`crate::catalog::Catalog::load`] fills each item's
/// in-memory calibration from it, so nothing downstream has to know where the
/// numbers came from.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct CalibrationFile {
/// Schema version.
#[serde(
default = "default_version",
deserialize_with = "yaml::flexible_string"
)]
pub schema_version: String,
/// One entry per calibrated item, by item id.
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
pub items: BTreeMap<String, Calibration>,
}
impl CalibrationFile {
/// Loads the store, or an empty one when the file does not exist yet.
///
/// Absence is not an error: a course that has not graded anything has no
/// statistics, and every command that reads them has to work anyway.
///
/// # Arguments
///
/// * `path` - the file, usually `analysis/calibration.yaml`.
///
/// # Returns
///
/// The store.
///
/// # Errors
///
/// Returns [`Error::Yaml`] when the file exists and does not parse.
pub fn load(path: &Path) -> Result<CalibrationFile> {
if !path.is_file() {
return Ok(CalibrationFile::default());
}
yaml::read(path)
}
/// Writes the store.
///
/// # Arguments
///
/// * `path` - the destination.
///
/// # Errors
///
/// Returns [`Error::Io`] on a write failure.
pub fn save(&self, path: &Path) -> Result<()> {
yaml::write(path, self)
}
/// The calibration recorded for one item.
///
/// # Arguments
///
/// * `item` - the item id.
///
/// # Returns
///
/// The record, or `None` when the item has never been calibrated.
pub fn get(&self, item: &str) -> Option<&Calibration> {
self.items.get(item)
}
/// Checks the store against the bank it describes.
///
/// The checks that matter for a file kept apart from what it refers to: a
/// record for an item that no longer exists, an option id the item does not
/// have, and — the one worth having — statistics measured against a stem
/// that has since been reworded, which since 2.0 means they describe a
/// different question wearing the same id.
///
/// # Arguments
///
/// * `catalog` - the loaded course.
///
/// # Returns
///
/// One message per problem, empty when the store agrees with the bank.
pub fn validate(&self, catalog: &crate::catalog::Catalog) -> Vec<String> {
let mut issues = Vec::new();
for (id, calibration) in &self.items {
let Some(entry) = catalog.get(id) else {
issues.push(format!(
"calibration for `{id}`: no such item. Statistics outlive an item only if \
it is retired, not deleted — a retired item keeps its id so its numbers \
still mean something."
));
continue;
};
let item = &entry.item;
for variant in &calibration.variants {
for option in variant.option_stats.keys() {
if item.option(option).is_none() {
issues.push(format!(
"calibration for `{id}`: variant `{}` has statistics for `{option}`, \
which is not an option of this item",
short(&variant.variant)
));
}
}
if !variant.key.is_empty()
&& variant.variant != item.variant_digest(&variant.key, &variant.distractors)
{
issues.push(format!(
"calibration for `{id}`: variant `{}` was measured against an option set \
that has since been reworded, so its numbers describe wording no \
student now sees",
short(&variant.variant)
));
}
}
for option in calibration.options.keys() {
if item.option(option).is_none() {
issues.push(format!(
"calibration for `{id}`: an option history names `{option}`, which is \
not an option of this item"
));
}
}
}
issues
}
}
/// What one administration measured: `analysis/administrations/<id>.yaml`.
///
/// Written once, when the exam is analyzed, and never rewritten. It is the
/// audit trail under [`CalibrationFile`]: the pooled numbers say an item sits
/// at 0.63, and these say which exams that came from and what each one saw.
/// Keeping them also means the history survives losing `data/`, which is
/// ignored by git and rotates.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct MeasurementFile {
/// Schema version.
#[serde(
default = "default_version",
deserialize_with = "yaml::flexible_string"
)]
pub schema_version: String,
/// What was administered, and how it was analyzed.
pub administration: MeasurementMeta,
/// One entry per question, in the order it was printed.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub items: Vec<Measurement>,
}
/// What an administration was, for a reader two years later.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct MeasurementMeta {
/// The administration id these numbers came from.
pub id: String,
/// The assessment that was administered.
pub assessment: String,
/// The term.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub term: Option<String>,
/// The date it was given.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub date: Option<Date>,
/// The forms in play.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub forms: Vec<String>,
/// How many examinees the numbers pool over.
///
/// The one number to read before any of the others. A point-biserial on
/// twenty-seven students is a different kind of claim than one on three
/// hundred, and nothing below records how thin it is.
pub n_examinees: usize,
/// The item response model fitted, when one was.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub model: Option<IrtModel>,
/// When the analysis was run.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub generated: Option<Date>,
/// The version of the tool that ran it, since the numbers depend on it.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub coursebank: Option<String>,
}
/// One question's statistics from one administration.
///
/// Cohort aggregates only. There is deliberately no per-section or per-form
/// breakdown: those get small, and a small cell crossed with anything else is
/// how an aggregate stops being one.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct Measurement {
/// The item id.
pub item: String,
/// The question number it was printed as.
pub number: u32,
/// The option set administered. See [`crate::item::Item::variant_digest`].
#[serde(default, skip_serializing_if = "Option::is_none")]
pub variant: Option<String>,
/// The stem as administered. See [`crate::item::Item::stem_digest`].
#[serde(default, skip_serializing_if = "Option::is_none")]
pub stem_digest: Option<String>,
/// Examinees who saw it.
pub n: usize,
/// Proportion correct.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub p_value: Option<f64>,
/// Corrected item-total point-biserial correlation.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub point_biserial: Option<f64>,
/// Upper-minus-lower-group discrimination index.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub discrimination_index: Option<f64>,
/// The option ids keyed correct, so a row in the options file says whether
/// it describes the answer or a distractor without a join.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub key: Vec<String>,
/// Per-option behaviour, by option id.
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
pub option_stats: BTreeMap<String, OptionStat>,
/// Fitted parameters, when the sample supported a fit.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub irt: Option<IrtParams>,
/// Machine-detected problems with this question on this administration.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub flags: Vec<Flag>,
}
/// The columns of an items file, and the only ones accepted on read.
///
/// An allowlist rather than a type-level guarantee. In YAML the structures
/// reject unknown keys, so a file carrying a student column could not be
/// loaded; CSV readers are tolerant of extra columns, so the same assurance has
/// to be an explicit check. This is it, and [`MeasurementFile::read_csv`]
/// refuses any header not named here.
pub const ITEM_COLUMNS: [&str; 16] = [
"administration_id",
"assessment",
"term",
"date",
"forms",
"n_examinees",
"coursebank",
"generated",
"item",
"number",
"variant",
"stem_digest",
"n",
"p_value",
"point_biserial",
"discrimination_index",
];
/// The columns of an options file, and the only ones accepted on read.
pub const OPTION_COLUMNS: [&str; 9] = [
"administration_id",
"item",
"number",
"option",
"keyed",
"selection_rate",
"point_biserial",
"upper_group_rate",
"lower_group_rate",
];
/// One row of an items file.
#[derive(Debug, Clone, Serialize, Deserialize)]
struct ItemRow {
administration_id: String,
assessment: String,
term: String,
date: String,
forms: String,
n_examinees: usize,
coursebank: String,
generated: String,
item: String,
number: u32,
variant: String,
stem_digest: String,
n: usize,
p_value: String,
point_biserial: String,
discrimination_index: String,
}
/// One row of an options file.
#[derive(Debug, Clone, Serialize, Deserialize)]
struct OptionRow {
administration_id: String,
item: String,
number: u32,
option: String,
keyed: bool,
selection_rate: String,
point_biserial: String,
upper_group_rate: String,
lower_group_rate: String,
}
impl MeasurementFile {
/// The two file names this administration writes, items first.
///
/// # Arguments
///
/// * `dir` - usually `analysis/administrations`.
///
/// # Returns
///
/// The items path and the options path.
pub fn paths(&self, dir: &Path) -> (PathBuf, PathBuf) {
let stem = crate::course::slugify(&self.administration.id);
(
dir.join(format!("{stem}-items.csv")),
dir.join(format!("{stem}-options.csv")),
)
}
/// Writes the two files, refusing to overwrite either.
///
/// An administration is a thing that happened once, so replacing its record
/// is a deliberate act: delete the files first if you mean to re-analyze.
///
/// Two files rather than one because the data is two shapes — one row per
/// question, one row per question and option — and a single sparse table
/// serves neither. The administration's metadata repeats on every row,
/// which is what makes each file independently loadable and is the same
/// convention the response store already uses.
///
/// # Arguments
///
/// * `dir` - the directory to write into.
///
/// # Returns
///
/// The paths written.
///
/// # Errors
///
/// Returns [`Error::Usage`] when either file exists, and [`Error::Io`] or
/// [`Error::Csv`] on a write failure.
pub fn write_csv(&self, dir: &Path) -> Result<Vec<PathBuf>> {
let (items_path, options_path) = self.paths(dir);
for path in [&items_path, &options_path] {
if path.exists() {
return Err(Error::usage(format!(
"{} already records this administration. It happened once, so replacing it \
is a deliberate act: delete it first if you mean to re-analyze.",
path.display()
)));
}
}
std::fs::create_dir_all(dir).map_err(|e| Error::io(dir, e))?;
let meta = &self.administration;
let mut items = csv::Writer::from_path(&items_path).map_err(|e| Error::Csv {
path: items_path.clone(),
source: e,
})?;
let mut options = csv::Writer::from_path(&options_path).map_err(|e| Error::Csv {
path: options_path.clone(),
source: e,
})?;
for measurement in &self.items {
items
.serialize(ItemRow {
administration_id: meta.id.clone(),
assessment: meta.assessment.clone(),
term: meta.term.clone().unwrap_or_default(),
date: meta.date.map(|d| d.to_string()).unwrap_or_default(),
forms: meta.forms.join(";"),
n_examinees: meta.n_examinees,
coursebank: meta.coursebank.clone().unwrap_or_default(),
generated: meta.generated.map(|d| d.to_string()).unwrap_or_default(),
item: measurement.item.clone(),
number: measurement.number,
variant: measurement.variant.clone().unwrap_or_default(),
stem_digest: measurement.stem_digest.clone().unwrap_or_default(),
n: measurement.n,
p_value: number(measurement.p_value),
point_biserial: number(measurement.point_biserial),
discrimination_index: number(measurement.discrimination_index),
})
.map_err(|e| Error::Csv {
path: items_path.clone(),
source: e,
})?;
for (option, stat) in &measurement.option_stats {
options
.serialize(OptionRow {
administration_id: meta.id.clone(),
item: measurement.item.clone(),
number: measurement.number,
option: option.clone(),
keyed: measurement.key.iter().any(|k| k == option),
selection_rate: number(stat.selection_rate),
point_biserial: number(stat.point_biserial),
upper_group_rate: number(stat.upper_group_rate),
lower_group_rate: number(stat.lower_group_rate),
})
.map_err(|e| Error::Csv {
path: options_path.clone(),
source: e,
})?;
}
}
items.flush().map_err(|e| Error::io(&items_path, e))?;
options.flush().map_err(|e| Error::io(&options_path, e))?;
Ok(vec![items_path, options_path])
}
/// Reads one administration back from its two files.
///
/// # Arguments
///
/// * `items_path` - the items file. The options file is found beside it.
///
/// # Returns
///
/// The administration, with its per-option statistics reattached.
///
/// # Errors
///
/// Returns [`Error::Csv`] on a parse failure and [`Error::Invalid`] when a
/// file carries a column that is not in [`ITEM_COLUMNS`] or
/// [`OPTION_COLUMNS`] — which is how a student column is caught.
pub fn read_csv(items_path: &Path) -> Result<MeasurementFile> {
let options_path = PathBuf::from(
items_path
.to_string_lossy()
.replace("-items.csv", "-options.csv"),
);
let mut reader = open_csv(items_path, &ITEM_COLUMNS)?;
let mut meta = MeasurementMeta::default();
let mut items: Vec<Measurement> = Vec::new();
for row in reader.deserialize::<ItemRow>() {
let row = row.map_err(|e| Error::Csv {
path: items_path.to_path_buf(),
source: e,
})?;
meta = MeasurementMeta {
id: row.administration_id.clone(),
assessment: row.assessment.clone(),
term: some(&row.term),
date: parse_date(&row.date),
forms: row
.forms
.split(';')
.filter(|f| !f.is_empty())
.map(str::to_string)
.collect(),
n_examinees: row.n_examinees,
model: meta.model,
generated: parse_date(&row.generated),
coursebank: some(&row.coursebank),
};
items.push(Measurement {
item: row.item,
number: row.number,
variant: some(&row.variant),
stem_digest: some(&row.stem_digest),
n: row.n,
p_value: parse(&row.p_value),
point_biserial: parse(&row.point_biserial),
discrimination_index: parse(&row.discrimination_index),
..Measurement::default()
});
}
if options_path.is_file() {
let mut reader = open_csv(&options_path, &OPTION_COLUMNS)?;
for row in reader.deserialize::<OptionRow>() {
let row = row.map_err(|e| Error::Csv {
path: options_path.clone(),
source: e,
})?;
let Some(target) = items.iter_mut().find(|i| i.number == row.number) else {
continue;
};
if row.keyed && !target.key.contains(&row.option) {
target.key.push(row.option.clone());
}
target.option_stats.insert(
row.option,
OptionStat {
selection_rate: parse(&row.selection_rate),
point_biserial: parse(&row.point_biserial),
upper_group_rate: parse(&row.upper_group_rate),
lower_group_rate: parse(&row.lower_group_rate),
},
);
}
}
Ok(MeasurementFile {
schema_version: default_version(),
administration: meta,
items,
})
}
/// Reads every administration under a directory, oldest first.
///
/// # Arguments
///
/// * `dir` - usually `analysis/administrations`.
///
/// # Returns
///
/// The administrations, empty when the directory does not exist.
///
/// # Errors
///
/// Propagates read failures.
pub fn load_all(dir: &Path) -> Result<Vec<MeasurementFile>> {
if !dir.is_dir() {
return Ok(Vec::new());
}
let mut paths: Vec<PathBuf> = std::fs::read_dir(dir)
.map_err(|e| Error::io(dir, e))?
.filter_map(|e| e.ok().map(|e| e.path()))
.filter(|p| p.to_string_lossy().ends_with("-items.csv"))
.collect();
paths.sort();
let mut out = Vec::new();
for path in &paths {
out.push(MeasurementFile::read_csv(path)?);
}
out.sort_by(|a, b| {
a.administration
.date
.cmp(&b.administration.date)
.then(a.administration.id.cmp(&b.administration.id))
});
Ok(out)
}
}
/// Opens a CSV and refuses any column that is not on the allowlist.
fn open_csv(path: &Path, allowed: &[&str]) -> Result<csv::Reader<std::fs::File>> {
let mut reader = csv::Reader::from_path(path).map_err(|e| Error::Csv {
path: path.to_path_buf(),
source: e,
})?;
let headers = reader
.headers()
.map_err(|e| Error::Csv {
path: path.to_path_buf(),
source: e,
})?
.clone();
let unexpected: Vec<&str> = headers.iter().filter(|h| !allowed.contains(h)).collect();
if !unexpected.is_empty() {
return Err(Error::Invalid(vec![format!(
"{}: unexpected column(s) {}. These files are committed, so they hold cohort \
aggregates and nothing else — anything per-student belongs in data/, which is \
ignored.",
path.display(),
unexpected.join(", ")
)]));
}
Ok(reader)
}
/// A float as a CSV cell, empty when absent.
fn number(value: Option<f64>) -> String {
value.map(|v| format!("{v}")).unwrap_or_default()
}
/// A CSV cell as a float, absent when empty or unparseable.
fn parse(cell: &str) -> Option<f64> {
cell.trim().parse().ok()
}
/// A CSV cell as a string, absent when empty.
fn some(cell: &str) -> Option<String> {
(!cell.trim().is_empty()).then(|| cell.trim().to_string())
}
/// A CSV cell as a date, absent when empty or unparseable.
fn parse_date(cell: &str) -> Option<Date> {
some(cell).and_then(|c| c.parse().ok())
}
/// The schema version new files are written with.
fn default_version() -> String {
SCHEMA_VERSION.to_string()
}
/// A digest shortened for a message.
fn short(digest: &str) -> String {
digest.chars().take(8).collect()
}
#[cfg(test)]
mod tests {
use super::*;
use crate::item::VariantCalibration;
fn tmp(tag: &str) -> std::path::PathBuf {
let p = std::env::temp_dir().join(format!("coursebank-cal-{tag}-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&p);
std::fs::create_dir_all(&p).unwrap();
p
}
#[test]
fn an_absent_store_is_empty_rather_than_an_error() {
let dir = tmp("absent");
let store = CalibrationFile::load(&dir.join(CALIBRATION_FILE)).unwrap();
assert!(store.items.is_empty());
assert!(store.get("q-x").is_none());
}
#[test]
fn the_store_round_trips() {
let dir = tmp("round");
let path = dir.join(CALIBRATION_FILE);
let mut store = CalibrationFile::default();
store.items.insert(
"q-x".to_string(),
Calibration {
n_examinees: Some(27),
p_value: Some(0.63),
variants: vec![VariantCalibration {
variant: "4c81fa".into(),
n_examinees: Some(27),
..VariantCalibration::default()
}],
..Calibration::default()
},
);
store.save(&path).unwrap();
let back = CalibrationFile::load(&path).unwrap();
assert_eq!(back.get("q-x").unwrap().p_value, Some(0.63));
assert_eq!(back.get("q-x").unwrap().variants.len(), 1);
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn an_administration_round_trips_through_two_csvs() {
let dir = tmp("csv");
let file = MeasurementFile {
schema_version: default_version(),
administration: MeasurementMeta {
id: "e1-2026f".into(),
assessment: "e1".into(),
term: Some("2026f".into()),
date: Some(Date::new(2026, 9, 15).unwrap()),
forms: vec!["A".into(), "B".into()],
n_examinees: 27,
model: None,
generated: Some(Date::new(2026, 9, 27).unwrap()),
coursebank: Some("0.0.0".into()),
},
items: vec![Measurement {
item: "q-fastq-quality-length-match".into(),
number: 1,
variant: Some("237d62f9af222f78".into()),
stem_digest: Some("8b22e0".into()),
n: 27,
p_value: Some(0.5926),
point_biserial: Some(0.31),
discrimination_index: None,
key: vec!["o-fourth-line".into()],
option_stats: [
(
"o-fourth-line".to_string(),
OptionStat {
selection_rate: Some(0.5926),
point_biserial: Some(0.31),
upper_group_rate: None,
lower_group_rate: None,
},
),
(
"o-third-line".to_string(),
OptionStat {
selection_rate: Some(0.1852),
point_biserial: Some(-0.18),
upper_group_rate: None,
lower_group_rate: None,
},
),
]
.into_iter()
.collect(),
irt: None,
flags: Vec::new(),
}],
};
let written = file.write_csv(&dir).unwrap();
assert_eq!(written.len(), 2, "one table per shape");
assert!(written[0].ends_with("e1-2026f-items.csv"));
assert!(written[1].ends_with("e1-2026f-options.csv"));
// One line per question, loadable on its own: the administration's
// metadata repeats on the row, as the response store already does.
let items = std::fs::read_to_string(&written[0]).unwrap();
assert!(
items.starts_with("administration_id,assessment,term,date,forms"),
"{items}"
);
assert!(
items.contains("e1-2026f,e1,2026f,2026-09-15,A;B,27"),
"{items}"
);
let options = std::fs::read_to_string(&written[1]).unwrap();
assert!(options.contains("o-fourth-line,true"), "{options}");
assert!(options.contains("o-third-line,false"), "{options}");
let back = MeasurementFile::read_csv(&written[0]).unwrap();
assert_eq!(back.administration.id, "e1-2026f");
assert_eq!(back.administration.n_examinees, 27);
assert_eq!(back.administration.forms, vec!["A", "B"]);
assert_eq!(back.items.len(), 1);
assert_eq!(back.items[0].p_value, Some(0.5926));
assert_eq!(back.items[0].key, vec!["o-fourth-line"]);
assert_eq!(back.items[0].option_stats.len(), 2);
assert_eq!(back.items[0].discrimination_index, None);
assert_eq!(MeasurementFile::load_all(&dir).unwrap().len(), 1);
// It happened once, so the record is not replaced by accident.
let err = file.write_csv(&dir).unwrap_err().to_string();
assert!(err.contains("deliberate"), "{err}");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn a_student_column_is_refused_on_read() {
let dir = tmp("identifiers");
let path = dir.join("e1-2026f-items.csv");
std::fs::write(
&path,
"administration_id,assessment,item,number,n,student_key\n e1-2026f,e1,q-x,1,27,abc123\n",
)
.unwrap();
// In YAML this fell out of the type, which rejects unknown keys. A CSV
// reader tolerates extra columns, so the same assurance has to be an
// explicit allowlist — and this is the test that it is one.
let err = MeasurementFile::read_csv(&path).unwrap_err().to_string();
assert!(err.contains("student_key"), "{err}");
assert!(err.contains("cohort aggregates"), "{err}");
let _ = std::fs::remove_dir_all(&dir);
}
}
src/model/catalog.rs
+18
View File
@@ -75,6 +75,12 @@ pub struct Catalog {
pub entries: Vec<Entry>,
/// Bank metadata by bank id.
pub banks: BTreeMap<String, crate::bank::BankMeta>,
/// The statistics, loaded from `analysis/` rather than from the banks.
///
/// Each entry's [`crate::item::Item::calibration`] is filled from this at
/// load, so everything downstream reads one item and does not have to know
/// that the numbers and the wording come from different files.
pub calibration: crate::calibration::CalibrationFile,
/// Map from global id to index into `entries`.
index: BTreeMap<String, usize>,
}
@@ -104,7 +110,10 @@ impl Catalog {
entries: Vec::new(),
banks: BTreeMap::new(),
index: BTreeMap::new(),
calibration: crate::calibration::CalibrationFile::default(),
};
catalog.calibration =
crate::calibration::CalibrationFile::load(&catalog.layout.calibration_file())?;
let mut problems = Vec::new();
let mut files = yaml::list_yaml(&catalog.layout.banks())?;
@@ -145,6 +154,15 @@ impl Catalog {
if !problems.is_empty() {
return Err(Error::Invalid(problems));
}
// Statistics are attached here rather than parsed from the bank, which
// is why an item can be read as one thing while its wording and its
// evidence live in files with different review cycles.
for entry in &mut catalog.entries {
if let Some(calibration) = catalog.calibration.items.get(&entry.uid) {
entry.item.calibration = Some(calibration.clone());
}
}
Ok(catalog)
}
src/model/course.rs
+69 -9
View File
@@ -800,15 +800,25 @@ pub struct Objective {
/// The lectures that develop it.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub lectures: Vec<String>,
/// Position in teaching order, low first.
/// Position in teaching order, low first. Derived; authoring it is
/// deprecated.
///
/// The registry is a map, so declaration order is lost on load, and sorting
/// The registry is a map, so declaration order is lost on load and sorting
/// by id would put `lo-enthalpy` before `lo-first-law` when the second is a
/// prerequisite of the first. Anything that prints objectives in the order
/// you teach them, a lecture page above all, needs this. Objectives without
/// it sort last, by id.
/// prerequisite of the first. Something has to supply the order.
///
/// Since 2.0 that something is [`Lecture::teaches`], which is a sequence:
/// the position of an objective in the list of what a lecture covers, and
/// the position of the lecture in the course, together say when it is
/// taught. [`fragment::assemble`] fills this in from those two, so a course
/// that used to carry thirty-nine hand-kept integers now carries none, and
/// inserting an objective is a one-line edit rather than a renumber.
///
/// An authored value still wins, so a 1.0 course loads unchanged.
/// `coursebank migrate order` removes them.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub order: Option<u32>,
/// The highest level you intend to assess this objective at. Assembling an
/// item above the ceiling is a warning: either the item overreaches or the
/// ceiling needs raising.
@@ -865,8 +875,20 @@ pub struct Target {
///
/// Ordered within its objective rather than across the course, so two
/// targets under different objectives never compete for a position and
/// inserting one renumbers nothing outside its own group.
#[serde(default, skip_serializing_if = "Option::is_none")]
/// Retained only so a pre-2.0 course still loads. Ignored.
///
/// Targets do not have an order. An objective's targets are a set of
/// question templates, not steps in a sequence: they are not taught in
/// order, an exam samples from them rather than working through them, and
/// the study workflow reads the list as a checklist and counts what it can
/// do cold. A position would assert a sequence that does not exist.
///
/// Where a list has to be printed, [`CourseFile::targets`] orders it by
/// ceiling and then by id. Where one target genuinely depends on another,
/// that is [`Target::prerequisites`], which says so directly.
///
/// `coursebank migrate order` removes it.
#[serde(default, skip_serializing)]
pub order: Option<u32>,
/// The highest level you intend to assess this target at. Omit it to inherit
/// the objective's.
@@ -1183,6 +1205,33 @@ impl CourseFile {
));
}
}
// A manuscript with no journal is a citation nobody can print. The
// fields exist; a note that carries them instead is data the reading
// list cannot link and the bibliography exporters cannot use.
if matches!(
reference.kind,
ReferenceKind::Article | ReferenceKind::Preprint
) && reference.container.is_none()
{
issues.push(format!(
"reference `{key}`: an {} needs a `container` — the journal, preprint \
server, or proceedings it appeared in. Run `coursebank migrate references` \
if it is sitting in the `note`.",
match reference.kind {
ReferenceKind::Preprint => "preprint",
_ => "article",
}
));
}
if let Some(note) = &reference.note {
if crate::citation::looks_like_a_citation(note) {
issues.push(format!(
"reference `{key}`: the note still carries a citation. Volume, pages, \
and DOI have their own fields, and a DOI in a note is a link nobody \
can follow. `coursebank migrate references` takes it apart."
));
}
}
if let Some(pmid) = &reference.pmid {
if !pmid.trim().chars().all(|c| c.is_ascii_digit()) {
issues.push(format!(
@@ -1646,9 +1695,20 @@ impl CourseFile {
.filter(|(_, target)| target.objective == objective)
.map(|(id, _)| id)
.collect();
// By ceiling, then by id. A target is a question template rather than a
// step in a sequence — an objective's targets are not taught in an
// order, and an exam samples from them — so there is no teaching order
// to print. What there is is depth, and grouping by it puts the
// checklist in the order the study methods apply: recall for a Level 1
// target, explanation for Level 2, variations and written solutions
// above that. Ties break by id so the list is stable.
ids.sort_by_key(|id| {
let target = &self.learning_targets[*id];
(target.order.unwrap_or(u32::MAX), (*id).clone())
(
self.effective_level_ceiling(id)
.map(|l| l.code())
.unwrap_or(0),
(*id).clone(),
)
});
ids.into_iter().map(String::as_str).collect()
}
src/model/course/fragment.rs
+108
View File
@@ -535,6 +535,27 @@ impl Merge {
}
}
// Teaching order, from the two sequences that already declare it: the
// lectures in course order, and each lecture's `teaches` list. This is
// what lets an objective stop carrying a hand-kept integer.
let mut position = 0u32;
let ordered: Vec<String> = self.lectures.keys().cloned().collect();
for lecture_id in ordered {
let teaches = self
.lectures
.get(&lecture_id)
.map(|l| l.teaches.clone())
.unwrap_or_default();
for objective_id in teaches {
position += 1;
if let Some(objective) = self.objectives.get_mut(&objective_id) {
if objective.order.is_none() {
objective.order = Some(position);
}
}
}
}
// A target with no lecture of its own is taught wherever its objective
// is. Collected first: the read of `objectives` and the write to
// `targets` cannot overlap in one pass.
@@ -800,6 +821,93 @@ units:
assert!(course.origin("nothing-like-this").is_none());
}
#[test]
fn teaching_order_is_derived_from_the_two_sequences_that_declare_it() {
let root = tmp("order");
write(&root, "course.yaml", ROOT);
write(
&root,
"lectures/l-1-2.yaml",
"lectures:\n L1.2:\n title: One\n teaches: [lo-second, lo-first]\n",
);
write(
&root,
"lectures/l-1-3.yaml",
"lectures:\n L1.3:\n title: Two\n teaches: [lo-third]\n",
);
write(
&root,
"objectives/lo-first.yaml",
r#"learning_objectives:
lo-first:
text: A.
lo-second:
text: B.
lo-third:
text: C.
"#,
);
let course = assemble(&root).unwrap();
// Position in `teaches`, not id order: the lecture lists `lo-second`
// first and that is what teaching it first means.
assert_eq!(course.learning_objectives["lo-second"].order, Some(1));
assert_eq!(course.learning_objectives["lo-first"].order, Some(2));
assert_eq!(course.learning_objectives["lo-third"].order, Some(3));
assert_eq!(
course.lecture_objectives("L1.2"),
vec!["lo-second", "lo-first"]
);
}
#[test]
fn targets_print_by_ceiling_rather_than_in_a_sequence() {
let root = tmp("target-order");
write(&root, "course.yaml", ROOT);
write(
&root,
"lectures/l-1-2.yaml",
"lectures:\n L1.2:\n title: One\n teaches: [lo-x]\n",
);
write(
&root,
"objectives/lo-x.yaml",
r#"learning_objectives:
lo-x:
text: A.
level_ceiling: 3
learning_targets:
t-predict:
text: Predict the effect.
objective: lo-x
level_ceiling: 3
t-define:
text: Define the term.
objective: lo-x
level_ceiling: 1
t-explain:
text: Explain the mechanism.
objective: lo-x
level_ceiling: 2
"#,
);
let course = assemble(&root).unwrap();
assert!(course.validate().is_empty(), "{:?}", course.validate());
// Shallowest first, which is the order the study methods apply in:
// recall, then explanation, then variations. Not id order, which would
// put `t-define` after `t-predict` for no reason at all.
assert_eq!(
course.targets("lo-x"),
vec!["t-define", "t-explain", "t-predict"]
);
// A target with no ceiling of its own inherits the objective's, so it
// sorts where that puts it.
assert_eq!(course.learning_targets["t-define"].order, None);
}
#[test]
fn teaching_the_same_objective_from_two_lectures_unions() {
let root = tmp("union");
src/model/item.rs
+8 -1
View File
@@ -157,7 +157,14 @@ pub struct Item {
pub design: Option<Design>,
/// What the evidence says, accumulated across administrations.
#[serde(default, skip_serializing_if = "Option::is_none")]
/// What the statistics say, filled in from `analysis/` at load.
///
/// Read from the store and never written back: `skip_serializing` means a
/// bank file cannot acquire a `calibration:` block by being round-tripped
/// through this type. A bank is a reviewed artifact whose diff should be a
/// change of intent, and every grading run would otherwise produce a diff
/// on it. See [`crate::calibration`].
#[serde(default, skip_serializing)]
pub calibration: Option<Calibration>,
/// The last review decision recorded for this item.
src/model/layout.rs
+20
View File
@@ -64,6 +64,24 @@ impl Layout {
self.root.join("banks")
}
/// Directory holding the statistics, which are kept out of the banks.
///
/// Committed, unlike [`Layout::data`]: everything under here is a cohort
/// aggregate with no student in it. See [`crate::calibration`].
pub fn analysis(&self) -> PathBuf {
self.root.join("analysis")
}
/// The pooled per-item calibration store.
pub fn calibration_file(&self) -> PathBuf {
self.analysis().join(crate::calibration::CALIBRATION_FILE)
}
/// Directory holding one immutable record per administration.
pub fn measurements(&self) -> PathBuf {
self.analysis().join("administrations")
}
/// Directory holding assessment records.
pub fn assessments(&self) -> PathBuf {
self.root.join("assessments")
@@ -117,6 +135,8 @@ impl Layout {
pub fn create_all(&self) -> Result<()> {
for dir in [
self.root.clone(),
self.analysis(),
self.measurements(),
self.lectures(),
self.objectives(),
self.banks(),
src/model/seal.rs
+80
View File
@@ -124,6 +124,26 @@ pub struct SealMeta {
/// from the file's own contents; [`SealFile::verify`] rebuilds a seal from the
/// live course and compares this against it.
pub digest: String,
/// The digests this file had before it was rewritten, oldest first.
///
/// A seal is meant to be tamper-evident, which puts a migration that
/// renames an id inside it in an awkward position: the id is part of
/// [`SealFile::digest_input`], so rewriting the file invalidates the digest
/// that vouched for it, and recomputing it quietly would produce a record
/// that looks untouched and is not.
///
/// So a migration does both — recomputes the digest and leaves the old one
/// here. What the seal then says is the honest thing: these are the
/// contents, this is what they hash to, and here is what the file hashed to
/// before each rewrite. Anyone holding an earlier copy, a backup or a git
/// revision, can check it against the right entry.
///
/// Deliberately outside [`SealFile::digest_input`]: a field that recorded
/// past digests and was itself covered by the current one could not be
/// appended to without invalidating what it describes.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub superseded_digests: Vec<String>,
}
/// What wrote a seal.
@@ -458,6 +478,7 @@ pub fn build(catalog: &Catalog, record: &AssessmentFile, opts: &Options) -> Resu
content: opts.content,
dropped,
digest: String::new(),
superseded_digests: Vec::new(),
},
items,
forms: sealed_forms,
@@ -787,6 +808,39 @@ impl SealFile {
buf
}
/// Renames the items this seal froze, keeping the digest honest.
///
/// The only sanctioned way to rewrite a seal. It recomputes the digest,
/// because the ids are part of what the digest covers, and records the
/// previous one in [`SealMeta::superseded_digests`], because a recomputed
/// digest with no trace of the recompute is a record that claims never to
/// have been touched.
///
/// # Arguments
///
/// * `rename` - a map from old item id to new.
///
/// # Returns
///
/// How many placements were renamed. Zero leaves the file alone, digest
/// included.
pub fn rename_items(&mut self, rename: &BTreeMap<String, String>) -> usize {
let mut renamed = 0;
for item in &mut self.items {
if let Some(new) = rename.get(&item.item) {
item.item = new.clone();
renamed += 1;
}
}
if renamed == 0 {
return 0;
}
let previous = std::mem::take(&mut self.seal.digest);
self.seal.superseded_digests.push(previous);
self.seal.digest = self.recompute_digest();
renamed
}
/// Recomputes this file's digest from its own contents.
///
/// # Returns
@@ -1195,6 +1249,7 @@ mod tests {
content: true,
dropped: Vec::new(),
digest: String::new(),
superseded_digests: Vec::new(),
},
items: vec![SealedItem {
number: 1,
@@ -1343,6 +1398,31 @@ mod tests {
assert_eq!(short("sha256:0123456789abcdef"), "sha256:01234567");
}
#[test]
fn renaming_an_item_recomputes_the_digest_and_says_it_did() {
let mut seal = sample();
seal.seal.digest = seal.recompute_digest();
let original = seal.seal.digest.clone();
assert!(seal.check_self().is_empty());
let mut rename = BTreeMap::new();
rename.insert(seal.items[0].item.clone(), "q-renamed".to_string());
assert_eq!(seal.rename_items(&rename), 1);
// The digest covers the ids, so it has to move — and the file has to
// admit that it moved rather than looking untouched.
assert_eq!(seal.items[0].item, "q-renamed");
assert_ne!(seal.seal.digest, original);
assert_eq!(seal.seal.superseded_digests, vec![original]);
assert!(seal.check_self().is_empty(), "{:?}", seal.check_self());
// A rename that matches nothing leaves the file entirely alone.
let steady = seal.seal.digest.clone();
assert_eq!(seal.rename_items(&BTreeMap::new()), 0);
assert_eq!(seal.seal.digest, steady);
assert_eq!(seal.seal.superseded_digests.len(), 1);
}
#[test]
fn round_trips_through_yaml() {
let file = sample();
src/util.rs
+1
View File
@@ -9,6 +9,7 @@
//! replaces a dependency that would otherwise need to keep working for as long as
//! a course repository needs to stay readable.
pub mod citation;
pub mod date;
pub mod hash;
pub mod markup;
src/util/citation.rs
+330
View File
@@ -0,0 +1,330 @@
// SPDX-License-Identifier: Prosperity-3.0.0
// Copyright Scientific Computing Studio
// Source: https://git.scient.ing/education/coursebank
//! Pulling a citation apart when it was written as prose.
//!
//! A bibliography assembled by hand tends to collect entries like
//!
//! ```text
//! note: 'Nucleic Acids Res 25:3389-3402. doi:10.1093/nar/25.17.3389'
//! ```
//!
//! which is a complete citation in a field that means "anything else worth
//! saying". Nothing can use it: a reading list cannot link the DOI, an export to
//! Hayagriva or BibTeX has no journal to put in `parent` or `journal`, and the
//! `container`, `volume`, `pages`, and `doi` fields sit empty beside it.
//!
//! [`parse`] takes such a note apart. What it cannot account for it leaves in
//! the note, which is the important half of the contract: a note reading
//! `'Bioinformatics 18:440-445. Origin of spaced seeds.'` yields the journal,
//! the volume, the pages, and a note that still says where spaced seeds came
//! from. Nothing is discarded and nothing is invented — an issue number that was
//! never written down stays absent, even when a publisher's DOI happens to
//! encode one.
/// The parts of a citation recovered from a note.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct Parsed {
/// The journal, proceedings, or book the work appeared in.
pub container: Option<String>,
/// The volume.
pub volume: Option<String>,
/// The page range, as `first-last`.
pub pages: Option<String>,
/// The DOI, bare.
pub doi: Option<String>,
/// What the note still says after the citation is removed.
pub note: Option<String>,
}
/// Takes a citation apart, leaving the rest of the note alone.
///
/// # Arguments
///
/// * `note` - the note as written.
/// * `year` - the record's year, which is how a trailing year is recognized as
/// part of a conference name rather than part of the title of the venue.
///
/// # Returns
///
/// The parts found. Every field is independently optional: a note that carries
/// only a DOI yields only a DOI.
pub fn parse(note: &str, year: Option<u32>) -> Parsed {
let mut out = Parsed::default();
let mut rest = note.trim().to_string();
if let Some((container, volume, pages, tail)) = citation(&rest) {
out.container = Some(container);
out.volume = Some(volume);
out.pages = Some(pages);
rest = tail;
}
if let Some((doi, tail)) = doi(&rest) {
out.doi = Some(doi);
rest = tail;
}
// A venue with no volume or pages — a conference, usually — is named by the
// clause that ends in the year the work was published.
if out.container.is_none() {
if let Some(y) = year {
if let Some((container, tail)) = venue(&rest, y) {
out.container = Some(container);
rest = tail;
}
}
}
// The year belongs to the record, not to the name of the venue.
if let (Some(container), Some(y)) = (&out.container, year) {
let suffix = format!(" {y}");
if let Some(trimmed) = container.strip_suffix(&suffix) {
out.container = Some(trimmed.trim_end().to_string());
}
}
let rest = rest.trim().trim_start_matches('.').trim().to_string();
out.note = (!rest.is_empty()).then_some(rest);
out
}
/// Finds `Journal 25:3389-3402` or `Journal 48, 443-453` at the start.
///
/// The volume is the first digit run that follows a space and is followed by a
/// separator and a page range. Requiring the whole shape is what keeps a year in
/// a conference name (`Proc. FOCS 2000.`) from being read as a volume.
///
/// # Returns
///
/// The container, volume, page range, and whatever followed.
fn citation(text: &str) -> Option<(String, String, String, String)> {
let bytes = text.as_bytes();
let mut at = 0;
while at < bytes.len() {
// A volume follows a space, so that a digit inside a name is not one.
if !(bytes[at].is_ascii_digit() && at > 0 && bytes[at - 1] == b' ') {
at += 1;
continue;
}
let volume_start = at;
let volume_end = digits(bytes, volume_start);
let mut cursor = spaces(bytes, volume_end);
// The separator between volume and pages is a colon or a comma.
if cursor < bytes.len() && (bytes[cursor] == b':' || bytes[cursor] == b',') {
cursor = spaces(bytes, cursor + 1);
let first_start = cursor;
let first_end = digits(bytes, first_start);
if first_end > first_start {
let dash = text[first_end..]
.strip_prefix('-')
.or_else(|| text[first_end..].strip_prefix('\u{2013}'));
if let Some(after_dash) = dash {
let last_offset = text.len() - after_dash.len();
let last_end = digits(bytes, last_offset);
if last_end > last_offset {
let container = text[..volume_start].trim_end_matches([' ', ',']);
if !container.is_empty() {
return Some((
container.to_string(),
text[volume_start..volume_end].to_string(),
format!(
"{}-{}",
&text[first_start..first_end],
&text[last_offset..last_end]
),
text[last_end..].to_string(),
));
}
}
}
}
}
at = volume_end;
}
None
}
/// Finds a `doi:10.…` anywhere in the text.
///
/// # Returns
///
/// The DOI and the text with it removed.
fn doi(text: &str) -> Option<(String, String)> {
let lower = text.to_ascii_lowercase();
let at = lower.find("doi:")?;
let after = text[at + 4..].trim_start();
let offset = text.len() - after.len();
let end = after
.find(char::is_whitespace)
.map(|n| offset + n)
.unwrap_or(text.len());
let doi = text[offset..end].trim_end_matches('.');
if !doi.starts_with("10.") {
return None;
}
let mut remainder = String::from(text[..at].trim_end());
let tail = text[end..].trim();
if !tail.is_empty() {
if !remainder.is_empty() {
remainder.push(' ');
}
remainder.push_str(tail);
}
Some((doi.to_string(), remainder))
}
/// Finds a leading clause ending in the publication year: `Proc. FOCS 2000.`
///
/// # Returns
///
/// The clause without its trailing period, and whatever followed.
fn venue(text: &str, year: u32) -> Option<(String, String)> {
let needle = format!("{year}.");
let at = text.find(&needle)?;
let clause = text[..at + needle.len() - 1].trim();
if clause.is_empty() {
return None;
}
Some((
clause.to_string(),
text[at + needle.len()..].trim().to_string(),
))
}
/// The end of a run of ASCII digits starting at `from`.
fn digits(bytes: &[u8], from: usize) -> usize {
let mut at = from;
while at < bytes.len() && bytes[at].is_ascii_digit() {
at += 1;
}
at
}
/// The end of a run of spaces starting at `from`.
fn spaces(bytes: &[u8], from: usize) -> usize {
let mut at = from;
while at < bytes.len() && bytes[at] == b' ' {
at += 1;
}
at
}
/// Whether a note still looks like it is carrying a citation.
///
/// Used by validation to say so, rather than leaving a note that a reading list
/// cannot link and an export cannot use.
///
/// # Arguments
///
/// * `note` - the note as written.
///
/// # Returns
///
/// `true` when a volume and page range, or a DOI, can be found in it.
pub fn looks_like_a_citation(note: &str) -> bool {
citation(note.trim()).is_some() || doi(note.trim()).is_some()
}
#[cfg(test)]
mod tests {
use super::*;
/// Every article note in a real course bibliography, which is where the
/// shapes below come from. Two separator styles, DOIs in three positions,
/// a conference with no volume, and prose that has to survive.
#[test]
fn a_journal_citation_comes_apart() {
let p = parse(
"Nucleic Acids Res 25:3389-3402. doi:10.1093/nar/25.17.3389",
Some(1997),
);
assert_eq!(p.container.as_deref(), Some("Nucleic Acids Res"));
assert_eq!(p.volume.as_deref(), Some("25"));
assert_eq!(p.pages.as_deref(), Some("3389-3402"));
assert_eq!(p.doi.as_deref(), Some("10.1093/nar/25.17.3389"));
assert_eq!(p.note, None);
// The DOI encodes volume 25, issue 17. Nothing infers the issue from
// it: a field nobody wrote down stays empty.
}
#[test]
fn an_abbreviation_keeps_its_final_period() {
let p = parse("J. Mol. Biol. 48, 443-453.", Some(1970));
assert_eq!(p.container.as_deref(), Some("J. Mol. Biol."));
assert_eq!(p.volume.as_deref(), Some("48"));
assert_eq!(p.pages.as_deref(), Some("443-453"));
assert_eq!(p.note, None);
}
#[test]
fn prose_after_a_citation_stays_in_the_note() {
let p = parse(
"Bioinformatics 18:440-445. Origin of spaced seeds.",
Some(2002),
);
assert_eq!(p.container.as_deref(), Some("Bioinformatics"));
assert_eq!(p.note.as_deref(), Some("Origin of spaced seeds."));
// A caveat the author wrote is the last thing to throw away.
let p = parse("J Mol Biol 215:403-410. Verify before use.", Some(1990));
assert_eq!(p.note.as_deref(), Some("Verify before use."));
let p = parse(
"Bioinformatics 25:2078-2079. doi:10.1093/bioinformatics/btp352. Author list is the \
core set plus the 1000 Genomes Data Processing Subgroup; verify.",
Some(2009),
);
assert_eq!(p.doi.as_deref(), Some("10.1093/bioinformatics/btp352"));
assert_eq!(
p.note.as_deref(),
Some(
"Author list is the core set plus the 1000 Genomes Data Processing Subgroup; \
verify."
)
);
}
#[test]
fn a_conference_has_a_year_where_a_volume_would_be() {
let p = parse(
"Proc. FOCS 2000. doi:10.1109/SFCS.2000.892127. The FM-index. Theory background.",
Some(2000),
);
assert_eq!(p.container.as_deref(), Some("Proc. FOCS"));
// No volume and no pages were written, so none are invented — and the
// 2000 in the DOI is not mistaken for either.
assert_eq!(p.volume, None);
assert_eq!(p.pages, None);
assert_eq!(p.doi.as_deref(), Some("10.1109/SFCS.2000.892127"));
assert_eq!(p.note.as_deref(), Some("The FM-index. Theory background."));
}
#[test]
fn a_note_with_nothing_to_find_is_left_whole() {
let p = parse("Origin of the MAPQ score.", Some(2008));
assert_eq!(p.container, None);
assert_eq!(p.note.as_deref(), Some("Origin of the MAPQ score."));
}
#[test]
fn a_multi_word_journal_is_not_cut_at_a_number() {
let p = parse("Advances in Mathematics 20, 367-387.", Some(1976));
assert_eq!(p.container.as_deref(), Some("Advances in Mathematics"));
assert_eq!(p.volume.as_deref(), Some("20"));
}
#[test]
fn what_validation_looks_for() {
assert!(looks_like_a_citation("Nat Methods 12:59-60."));
assert!(looks_like_a_citation("doi:10.1038/nmeth.3176"));
assert!(!looks_like_a_citation("Origin of minimizers."));
assert!(!looks_like_a_citation("Verify before use."));
// A page range with no volume is not a citation shape.
assert!(!looks_like_a_citation("see pages 12-14"));
}
}