refactor: improve student report

src/analysis/diagnostic.rs

@@ -23,9 +23,21 @@
 //! exam is given.
 //!
 //! What a student does get, per missed question: the number, its level, the
-//! objectives it measured, whether they answered it, and the feedback written for
+//! objectives it measured and what those objectives ask, whether they answered
-//! the specific option they chose. That last part is why authoring distractors
+//! it, the feedback written for the specific option they chose, the hint written
-//! carefully pays off twice.
+//! for that same option, and the item's own `review` citations. That is why
+//! authoring distractors carefully pays off twice.
+//!
+//! Two further fields are available and off by default, because they are the two
+//! that trade a student's understanding against reusing the question. The
+//! misconception is written to you about the student; the worked solution is the
+//! solutions document. See [`Options::misconceptions`] and [`Options::solutions`].
+//!
+//! It is worth being clear about what the default already discloses. The
+//! per-option feedback on a missed question routinely names the right answer,
+//! because that is what makes it useful. A report handed to sixty students is
+//! therefore already a partial answer key for the questions those students
+//! missed, with or without the two optional fields.
 //!
 //! Two consequences of that invariant are worth stating because they are easy to
 //! undo by accident:
@@ -46,6 +58,11 @@
 //! resolves each weak objective through the course's own reading registry, so a
 //! student is pointed at `KKW §6.2` with the sentence you wrote about what to take
 //! from it, rather than at the name of a chapter.
+//!
+//! [`LectureFocus`] answers the question a student actually asks, which is where
+//! to start. It ranks the lectures behind the missed questions by how many
+//! objectives went wrong in each, so a reading list of eleven sections becomes an
+//! ordered afternoon.
 
 use std::collections::{BTreeMap, BTreeSet};
 
@@ -54,8 +71,9 @@ use serde::Serialize;
 use crate::assessment::AssessmentFile;
 use crate::catalog::Catalog;
 use crate::classical::Analysis;
-use crate::course::{CourseFile, ReadingRole};
+use crate::course::{CourseFile, ReadingRole, Reference};
 use crate::irt::Fit;
+use crate::item::Citation;
 use crate::responses::{Response, ResponseSet};
 use crate::students::{Cohort, Mastery, StudentSummary};
 use crate::taxonomy::Level;
@@ -72,6 +90,27 @@ pub struct Options {
     pub questions: bool,
     /// Whether to include the feedback written for the option the student chose.
     pub feedback: bool,
+    /// Whether to include the hint written for that option.
+    ///
+    /// On by default. A hint is the question you would ask a student who was
+    /// reconsidering that option, so it gives them somewhere to start rather than
+    /// a verdict to accept.
+    pub hints: bool,
+    /// Whether to name the misconception the chosen distractor was written to
+    /// catch.
+    ///
+    /// Off by default, and not because it is unsafe. The text is written to you,
+    /// about the student, in the third person, and next to the feedback written
+    /// for them it reads like a chart note.
+    pub misconceptions: bool,
+    /// Whether to include the worked solution for a missed question.
+    ///
+    /// Off by default. [`crate::item::Solution::explanation`] is the derivation,
+    /// the estimate, and the argument for the key over its neighbours: the body of
+    /// the solutions document. Turning this on hands that to every student who
+    /// missed the question, which is the right call for a question you will not
+    /// use again and the wrong one for a bank you reuse each term.
+    pub solutions: bool,
     /// How many objectives to build a study plan for.
     pub focus_limit: usize,
     /// How many readings to list per objective.
@@ -86,6 +125,9 @@ impl Default for Options {
             comparison: true,
             questions: true,
             feedback: true,
+            hints: true,
+            misconceptions: false,
+            solutions: false,
             focus_limit: 4,
             readings_per_objective: 2,
             ability: false,
@@ -105,6 +147,12 @@ pub struct StudentDiagnostic {
     /// The institutional id, when identifiers were kept.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub sid: Option<String>,
+    /// Their email, when the export carried one and identifiers were kept.
+    ///
+    /// Absent rather than blank when the platform did not report it, so a
+    /// template prints nothing instead of an empty label.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub email: Option<String>,
     /// Which form they sat.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub form: Option<String>,
@@ -124,6 +172,10 @@ pub struct StudentDiagnostic {
     /// One row per question, with no question in it.
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub questions: Vec<QuestionRow>,
+    /// Which lectures to go back to, the one that would repay the most time
+    /// first.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub review_lectures: Vec<LectureFocus>,
     /// What to read, grouped by objective.
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub study: Vec<StudyGroup>,
@@ -253,6 +305,13 @@ pub struct QuestionRow {
     pub level: Option<u8>,
     /// The objectives it measured.
     pub objectives: Vec<String>,
+    /// What those objectives ask, in the words the course uses with students.
+    ///
+    /// The objective, not the question. It is printed in full in the objectives
+    /// table already; repeating it next to a missed question saves a student
+    /// working out which of thirty-six rows this one belonged to.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub objective_texts: Vec<String>,
     /// Whether it was answered correctly.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub correct: Option<bool>,
@@ -269,9 +328,68 @@ pub struct QuestionRow {
     /// The feedback written for the option this student chose.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub feedback: Option<String>,
+    /// The hint written for that option: where to look, not what the answer was.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub hint: Option<String>,
+    /// The misconception that option was written to catch, when
+    /// [`Options::misconceptions`] is on.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub misconception: Option<String>,
+    /// The worked solution, when [`Options::solutions`] is on.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub worked: Option<String>,
     /// Where the material was taught.
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub taught_in: Vec<String>,
+    /// What to read again about this question, from the item's own `review`
+    /// citations rather than from the objective's reading list.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub review: Vec<ItemReading>,
+}
+
+/// One citation to read again after missing a question.
+#[derive(Debug, Clone, Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct ItemReading {
+    /// A short citation, e.g. `KKW §2.1`.
+    pub citation: String,
+    /// The work's full title, for a student who does not recognise the label.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub title: Option<String>,
+    /// A link, when the citation resolves to one.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub url: Option<String>,
+}
+
+/// One lecture worth going back to, with the evidence for saying so.
+///
+/// Ranked by how many *objectives* went wrong rather than how many questions
+/// did. Missing four questions on one objective is one thing to relearn; missing
+/// four questions across four objectives is four, and the second is the lecture
+/// to reread first even though the arithmetic looks identical.
+#[derive(Debug, Clone, Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct LectureFocus {
+    /// The lecture id, e.g. `L1.4`.
+    pub lecture: String,
+    /// Its title.
+    pub title: String,
+    /// Where the slides live, when the course records that.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub url: Option<String>,
+    /// How many distinct objectives from this lecture were missed.
+    pub n_objectives: usize,
+    /// How many questions from this lecture were missed.
+    pub n_questions: usize,
+    /// Which questions, so a student can line this up with their paper.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub questions: Vec<u32>,
+    /// The slides those questions came from, when the items record them.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub slides: Vec<u32>,
+    /// The objectives that went wrong here, in the course's own words.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub objectives: Vec<String>,
 }
 
 /// What to read about one objective.
@@ -338,6 +456,15 @@ pub fn student(
     let rows = set.for_student(&summary.student_key);
     let form = rows.first().and_then(|r| r.form.clone());
 
+    // The summary carries the name; the email only ever existed on the response
+    // rows, and both are already absent from a pseudonymized store, so neither
+    // needs a flag here.
+    let name = summary
+        .name
+        .clone()
+        .or_else(|| rows.iter().find_map(|r| r.name.clone()));
+    let email = rows.iter().find_map(|r| r.email.clone());
+
     let levels = summary
         .levels
         .iter()
@@ -403,6 +530,11 @@ pub fn student(
         Vec::new()
     };
 
+    // Built from the response rows rather than from `questions`, so a report with
+    // `--no-questions` still says where to go back to; it just does not name the
+    // question numbers.
+    let review_lectures = lecture_focus(catalog, &rows, opts);
+
     let study = focus
         .iter()
         .take(opts.focus_limit)
@@ -417,8 +549,9 @@ pub fn student(
 
     StudentDiagnostic {
         student_key: summary.student_key.clone(),
-        name: summary.name.clone(),
+        name,
         sid: summary.sid.clone(),
+        email,
         form,
         score: Score {
             points: summary.points,
@@ -440,6 +573,7 @@ pub fn student(
         strengths,
         focus,
         questions,
+        review_lectures,
         study,
     }
 }
@@ -457,22 +591,45 @@ fn question_row(
     opts: &Options,
 ) -> QuestionRow {
     let blank = row.selected.is_empty() && row.eliminated.is_empty();
+    let missed = row.credit < 0.999;
     let mut feedback = None;
+    let mut hint = None;
+    let mut misconception = None;
+    let mut worked = None;
     let mut taught_in = Vec::new();
+    let mut review = Vec::new();
 
     if let Some(uid) = row.item_ref.as_deref() {
         if let Some(entry) = catalog.get(uid) {
-            if opts.feedback && row.credit < 0.999 {
+            if missed {
                 if let Some(letter) = row.chosen().first() {
-                    // `student_text` falls back to `explanation`, which is written
+                    if let Some(choice) = entry.item.option(letter) {
-                    // for a grader and often names the right answer. A student
+                        if opts.feedback {
-                    // report must not print it.
+                            // `student_text` falls back to `explanation`, which is
-                    feedback = entry.item.option(letter).and_then(|choice| {
+                            // written for a grader and often names the right
-                        choice
+                            // answer. A student report must not print it.
-                            .feedback_student
+                            feedback = choice
-                            .clone()
+                                .feedback_student
-                            .or_else(|| choice.misconception.clone())
+                                .clone()
-                    });
+                                .or_else(|| choice.misconception.clone());
+                        }
+                        if opts.hints {
+                            hint = choice.hint.clone();
+                        }
+                        // When an option carries no student feedback, the
+                        // fallback above already printed this text. The same
+                        // sentence twice under two labels reads as a bug.
+                        if opts.misconceptions && choice.misconception != feedback {
+                            misconception = choice.misconception.clone();
+                        }
+                    }
+                }
+
+                if let Some(solution) = entry.item.solution.as_ref() {
+                    if opts.solutions {
+                        worked = solution.explanation.clone();
+                    }
+                    review = item_readings(&catalog.course, &solution.review);
                 }
             }
             for source in &entry.item.sources {
@@ -503,6 +660,16 @@ fn question_row(
         position: row.form_position.filter(|p| *p != row.item_number),
         level: row.level.map(|l| l.code()),
         objectives: row.learning_objectives.clone(),
+        objective_texts: if missed {
+            row.learning_objectives
+                .iter()
+                .map(|id| catalog.course.objective_text(id))
+                .collect()
+        } else {
+            // Only where it earns its space. Every question already carries its
+            // objective ids, and the objectives table prints all of them.
+            Vec::new()
+        },
         correct: row.correct,
         credit: row.credit,
         bonus: row.bonus,
@@ -512,10 +679,167 @@ fn question_row(
             .then(|| class_rates.get(&row.item_number).copied())
             .flatten(),
         feedback,
+        hint,
+        misconception,
+        worked,
         taught_in,
+        review,
     }
 }
 
+/// Resolves an item's `review` citations against the course reference registry.
+///
+/// # Arguments
+///
+/// * `course` - the course, for its reference labels and base URLs.
+/// * `citations` - the item's citations.
+///
+/// # Returns
+///
+/// One entry per citation that resolves to something printable.
+fn item_readings(course: &CourseFile, citations: &[Citation]) -> Vec<ItemReading> {
+    let mut out = Vec::new();
+    for citation in citations {
+        let reference = citation
+            .reference
+            .as_deref()
+            .and_then(|key| course.references.get(key).map(|r| (key, r)));
+
+        let (label, title, url) = match reference {
+            Some((key, reference)) => (
+                reference.label.as_deref().unwrap_or(key).to_string(),
+                Some(reference.title.clone()),
+                resolve_citation_url(citation, reference),
+            ),
+            None => (citation.display(), None, citation.url.clone()),
+        };
+        if label.is_empty() {
+            continue;
+        }
+
+        let citation_text = match (&citation.text, &citation.locator) {
+            (Some(text), _) => text.clone(),
+            (None, Some(locator)) => format!("{label} {locator}"),
+            (None, None) => label,
+        };
+        out.push(ItemReading {
+            citation: citation_text,
+            title,
+            url,
+        });
+    }
+    out
+}
+
+/// A citation's own URL, else the reference's `base_url` joined with its `path`.
+fn resolve_citation_url(citation: &Citation, reference: &Reference) -> Option<String> {
+    if let Some(url) = &citation.url {
+        return Some(url.clone());
+    }
+    let path = citation.path.as_deref()?;
+    let base = reference.base_url.as_deref()?;
+    Some(match (base.ends_with('/'), path.starts_with('/')) {
+        (true, true) => format!("{base}{}", &path[1..]),
+        (false, false) => format!("{base}/{path}"),
+        _ => format!("{base}{path}"),
+    })
+}
+
+/// Ranks the lectures behind a student's missed questions.
+///
+/// A lecture earns its place by how many distinct objectives went wrong in it,
+/// then by how many questions, then by id so the order is stable between runs.
+///
+/// # Arguments
+///
+/// * `catalog` - the loaded course, for objective and lecture titles.
+/// * `rows` - this student's responses.
+/// * `opts` - what to include; `questions` decides whether numbers are named.
+///
+/// # Returns
+///
+/// The lectures, the one that would repay the most time first.
+fn lecture_focus(catalog: &Catalog, rows: &[&Response], opts: &Options) -> Vec<LectureFocus> {
+    /// What has accumulated for one lecture so far.
+    #[derive(Default)]
+    struct Tally {
+        objectives: BTreeSet<String>,
+        questions: BTreeSet<u32>,
+        slides: BTreeSet<u32>,
+    }
+
+    let course = &catalog.course;
+    let mut tallies: BTreeMap<String, Tally> = BTreeMap::new();
+
+    for row in rows.iter().filter(|r| r.counts() && r.credit < 0.999) {
+        // Two routes to a lecture, and both are wanted. The objective registry
+        // knows which lectures develop an objective; the item knows which lecture
+        // it was written from, which is the finer answer when an objective spans
+        // several.
+        let mut lectures: BTreeSet<String> = BTreeSet::new();
+        let mut slides: BTreeMap<String, BTreeSet<u32>> = BTreeMap::new();
+
+        if let Some(entry) = row.item_ref.as_deref().and_then(|uid| catalog.get(uid)) {
+            for source in &entry.item.sources {
+                lectures.insert(source.lecture.clone());
+                slides
+                    .entry(source.lecture.clone())
+                    .or_default()
+                    .extend(source.slides.iter().copied());
+            }
+        }
+        for objective in &row.learning_objectives {
+            if let Some(entry) = course.learning_objectives.get(objective) {
+                lectures.extend(entry.lectures.iter().cloned());
+            }
+        }
+
+        for lecture in lectures {
+            let tally = tallies.entry(lecture.clone()).or_default();
+            tally.objectives.extend(row.learning_objectives.clone());
+            tally.questions.insert(row.item_number);
+            if let Some(numbers) = slides.get(&lecture) {
+                tally.slides.extend(numbers.iter().copied());
+            }
+        }
+    }
+
+    let mut out: Vec<LectureFocus> = tallies
+        .into_iter()
+        .map(|(lecture, tally)| {
+            let record = course.lectures.get(&lecture);
+            LectureFocus {
+                title: record
+                    .map(|l| l.title.clone())
+                    .unwrap_or_else(|| lecture.clone()),
+                url: record.and_then(|l| l.slides_url.clone()),
+                n_objectives: tally.objectives.len(),
+                n_questions: tally.questions.len(),
+                questions: if opts.questions {
+                    tally.questions.iter().copied().collect()
+                } else {
+                    Vec::new()
+                },
+                slides: tally.slides.iter().copied().collect(),
+                objectives: tally
+                    .objectives
+                    .iter()
+                    .map(|id| course.objective_text(id))
+                    .collect(),
+                lecture,
+            }
+        })
+        .collect();
+
+    out.sort_by(|a, b| {
+        b.n_objectives
+            .cmp(&a.n_objectives)
+            .then(b.n_questions.cmp(&a.n_questions))
+            .then(a.lecture.cmp(&b.lecture))
+    });
+    out
+}
+
 /// Resolves an objective to readings.
 ///
 /// # Arguments
@@ -1139,6 +1463,7 @@ mod tests {
             student_key: "s-1".into(),
             name: None,
             sid: None,
+            email: None,
             form: None,
             score: Score {
                 points: 1.0,
@@ -1154,6 +1479,7 @@ mod tests {
             strengths: Vec::new(),
             focus: Vec::new(),
             questions: Vec::new(),
+            review_lectures: Vec::new(),
             study: Vec::new(),
         })
         .unwrap();

src/cli.rs

@@ -673,6 +673,21 @@ pub(crate) enum ReportCommand {
         /// Leave per-option feedback out of the Typst report.
         #[arg(long)]
         no_feedback: bool,
+        /// Leave out the hint written for the option the student chose.
+        #[arg(long)]
+        no_hints: bool,
+        /// Name the misconception each chosen distractor was written to catch.
+        ///
+        /// Written for you rather than for them, so it reads clinically next to
+        /// the feedback. Off by default for that reason, not for secrecy.
+        #[arg(long)]
+        misconceptions: bool,
+        /// Include the worked solution for every missed question.
+        ///
+        /// This is the solutions document. Reasonable for a question you will
+        /// not use again, and a way to publish your bank if you reuse it.
+        #[arg(long)]
+        solutions: bool,
         /// Use this template instead of the usual lookup.
         #[arg(long)]
         template: Option<PathBuf>,

src/commands/analysis.rs

@@ -497,6 +497,9 @@ pub(crate) fn report(cli: &Cli, sub: &ReportCommand) -> Result<Outcome> {
             no_markdown,
             no_questions,
             no_feedback,
+            no_hints,
+            misconceptions,
+            solutions,
             template,
             json,
         } => {
@@ -551,10 +554,24 @@ pub(crate) fn report(cli: &Cli, sub: &ReportCommand) -> Result<Outcome> {
                 comparison: !no_comparison,
                 questions: !no_questions,
                 feedback: !no_feedback,
+                hints: !no_hints,
+                misconceptions: *misconceptions,
+                solutions: *solutions,
                 ability: *ability,
                 ..diagnostic::Options::default()
             };
 
+            // Said once, at the moment it would matter, rather than left in the
+            // help text where nobody reads it twice.
+            if *solutions && !cli.quiet {
+                println!(
+                    "! --solutions puts the worked solution for every missed question into \
+                     {} report(s). Reusing these items next term means reusing them against \
+                     students who may have seen this page",
+                    cohort.students.len()
+                );
+            }
+
             let mut written = 0usize;
             let mut used_embedded = false;
             for summary in &cohort.students {

src/export/typst/diagnostic.rs

@@ -148,6 +148,10 @@ impl Meta {
         out.insert("assessment", assessment);
         out.insert("generator", generator);
         out.insert("policy", policy);
+        // `students-tested` is the honest name: it is how many people sat this
+        // assessment, which is not the enrolment. `class-size` stays as an alias
+        // so a template forked before this change keeps working.
+        out.insert("students-tested", Value::Int(self.n_students as i64));
         out.insert("class-size", Value::Int(self.n_students as i64));
         out.insert("extra", extra_value(config));
         out
@@ -180,6 +184,7 @@ pub fn student_value(diagnostic: &StudentDiagnostic, config: &RenderConfig) -> V
     out.insert("student-key", Value::str(&diagnostic.student_key));
     out.insert_some("name", diagnostic.name.as_ref().map(Value::str));
     out.insert_some("sid", diagnostic.sid.as_ref().map(Value::str));
+    out.insert_some("email", diagnostic.email.as_ref().map(Value::str));
     out.insert_some("form", diagnostic.form.as_ref().map(Value::str));
 
     let mut score = Value::dict();
@@ -296,17 +301,96 @@ pub fn student_value(diagnostic: &StudentDiagnostic, config: &RenderConfig) -> V
                     value.insert("bonus", Value::Bool(question.bonus));
                     value.insert("blank", Value::Bool(question.blank));
                     value.insert_some("class-rate", question.class_rate.map(Value::Float));
-                    value.insert_some(
+                    value.insert(
-                        "feedback",
+                        "objective-texts",
-                        question
+                        Value::Array(
-                            .feedback
+                            question
-                            .as_ref()
+                                .objective_texts
-                            .map(|text| markup_value(text, content)),
+                                .iter()
+                                .map(|text| markup_value(text, content))
+                                .collect(),
+                        ),
                     );
+                    for (key, text) in [
+                        ("feedback", question.feedback.as_ref()),
+                        ("hint", question.hint.as_ref()),
+                        ("misconception", question.misconception.as_ref()),
+                        ("worked", question.worked.as_ref()),
+                    ] {
+                        value.insert_some(key, text.map(|t| markup_value(t, content)));
+                    }
                     value.insert(
                         "taught-in",
                         Value::Array(question.taught_in.iter().map(|s| Value::str(s)).collect()),
                     );
+                    value.insert(
+                        "review",
+                        Value::Array(
+                            question
+                                .review
+                                .iter()
+                                .map(|reading| {
+                                    let mut entry = Value::dict();
+                                    entry.insert("citation", Value::str(&reading.citation));
+                                    entry.insert_some(
+                                        "title",
+                                        reading.title.as_ref().map(Value::str),
+                                    );
+                                    entry.insert_some("url", reading.url.as_ref().map(Value::str));
+                                    entry
+                                })
+                                .collect(),
+                        ),
+                    );
+                    value
+                })
+                .collect(),
+        ),
+    );
+
+    out.insert(
+        "review-lectures",
+        Value::Array(
+            diagnostic
+                .review_lectures
+                .iter()
+                .map(|lecture| {
+                    let mut value = Value::dict();
+                    value.insert("lecture", Value::str(&lecture.lecture));
+                    value.insert("title", Value::str(&lecture.title));
+                    value.insert_some("url", lecture.url.as_ref().map(Value::str));
+                    value.insert("objectives-missed", Value::Int(lecture.n_objectives as i64));
+                    value.insert("questions-missed", Value::Int(lecture.n_questions as i64));
+                    value.insert(
+                        "questions",
+                        Value::Array(
+                            lecture
+                                .questions
+                                .iter()
+                                .map(|n| Value::Int(*n as i64))
+                                .collect(),
+                        ),
+                    );
+                    value.insert(
+                        "slides",
+                        Value::Array(
+                            lecture
+                                .slides
+                                .iter()
+                                .map(|n| Value::Int(*n as i64))
+                                .collect(),
+                        ),
+                    );
+                    value.insert(
+                        "objectives",
+                        Value::Array(
+                            lecture
+                                .objectives
+                                .iter()
+                                .map(|text| markup_value(text, content))
+                                .collect(),
+                        ),
+                    );
                     value
                 })
                 .collect(),