Allow for regenerating test data for patched examples + tidy-ups #1081

alexdewar · 2026-01-15T15:42:37Z

Description

You currently can't regenerate test data for the patched examples used by tests. For now, there's just simple_divisible, but there will be more in future, so it seemed worth fixing.

I've fixed it by changing things so that you can pass a --patch flag to a few of the example subcommands, so that we can list and run them in the script. This option is hidden from the program help, as it's aimed at developers. In the long run, I'd like to unify the code paths used by the regression test code and the CLI code to do the patching etc., but I think we should do the refactoring for #1080 first.

Other changes:

Change script so you can optionally just update some of the models
Refactor code for working with examples into separate example module

Fixes #1077.

Type of change

Bug fix (non-breaking change to fix an issue)
New feature (non-breaking change to add functionality)
Refactoring (non-breaking, non-functional change to improve maintainability)
Optimization (non-breaking change to speed up the code)
Breaking change (whatever its nature)
Documentation (improve or add documentation)

Key checklist

All tests pass: $ cargo test
The documentation builds and looks OK: $ cargo doc
Update release notes for the latest release if this PR adds a new feature or fixes a bug
present in the previous release

Further checks

Code is commented, particularly in hard-to-understand areas
Tests added that prove fix is effective or that feature works

These are hidden from the help as ordinary users shouldn't need them.

This makes it consistent with the `just` command.

…ples too

codecov · 2026-01-15T15:44:47Z

Codecov Report

❌ Patch coverage is 64.15094% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.27%. Comparing base (25f3813) to head (404ad00).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
src/cli/example.rs	36.36%	21 Missing and 7 partials ⚠️
src/example.rs	85.36%	1 Missing and 5 partials ⚠️
src/example/patches.rs	80.95%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1081   +/-   ##
=======================================
  Coverage   82.26%   82.27%           
=======================================
  Files          53       55    +2     
  Lines        7416     7487   +71     
  Branches     7416     7487   +71     
=======================================
+ Hits         6101     6160   +59     
- Misses       1025     1029    +4     
- Partials      290      298    +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR enables regenerating test data for patched examples and includes refactoring improvements. The main motivation is to allow regeneration of test data for patched example models (like simple_divisible) that are used in tests.

Changes:

Added a --patch flag to the example subcommands (list, extract, run) to support patched examples
Refactored example-related code into a new example module with a patches submodule
Updated the test data regeneration script to support specifying individual models and patched examples
Simplified the run_regression_test_with_patches function signature by retrieving patches from the centralized location

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/lib.rs`	Added new `example` module export
`src/example.rs`	New module containing refactored code for working with example models
`src/example/patches.rs`	New module centralizing patch definitions for test cases
`src/cli/example.rs`	Updated to use refactored example module and added `--patch` flag support
`tests/regression.rs`	Updated to use centralized patch retrieval from `example::patches`
`tests/regression_simple_divisible.rs`	Simplified test function and renamed to match test case name
`tests/regenerate_test_data.sh`	New script replacing old one, with support for selective model regeneration and patched examples
`tests/regenerate_all_data.sh`	Removed (replaced by new script)
`justfile`	Updated to pass arguments to regenerate script
`docs/developer_guide/architecture_quickstart.md`	Added documentation about regenerating data for specific models and patched examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T15:46:36Z

tests/regenerate_test_data.sh

+
+if [ "$1" = --patch ]; then
+    shift 1
+    patch_examples=$@


The variable assignment should use quotes around $@ to properly handle arguments with spaces. Change to patch_examples=\"$@\".

Suggested change

patch_examples=$@

patch_examples="$@"

Copilot · 2026-01-15T15:46:37Z

tests/regenerate_test_data.sh

+    patch_examples=$@
+elif [ $# -gt 0 ]; then
+    examples=$@


The variable assignment should use quotes around $@ to properly handle arguments with spaces. Change to examples=\"$@\".

Suggested change

patch_examples=$@

elif [ $# -gt 0 ]; then

examples=$@

patch_examples="$@"

elif [ $# -gt 0 ]; then

examples="$@"

Copilot · 2026-01-15T15:46:37Z

src/cli/example.rs

-        new_path.display()
-    );
+        // NB: All patched models are based on `simple`, for now
+        let example = Example::from_name("simple").unwrap();


Using unwrap() here assumes that the 'simple' example always exists. Since this is a hardcoded value and the comment on line 106 indicates this is intentional, consider using expect() with a descriptive message to make the assumption explicit, e.g., expect(\"'simple' example must exist as base for all patched models\").

Suggested change

let example = Example::from_name("simple").unwrap();

let example = Example::from_name("simple")

.expect("'simple' example must exist as base for all patched models");

No, this should be fine as the simple example should always be present.

That said, there should probs be a test for this function instead.

src/example/patches.rs

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T16:04:58Z

tests/regenerate_test_data.sh

@@ -0,0 +1,49 @@
+#!/bin/sh
+set -ef


The shell option -e will cause the script to exit on error, but -f is not a valid POSIX shell option. You likely meant -eu (exit on error and treat unset variables as errors). The -f option disables filename globbing which is rarely the intended behavior for scripts like this.

Suggested change

set -ef

set -eu

Copilot · 2026-01-15T16:04:59Z

src/cli/example.rs

-        new_path.display()
-    );
+        // NB: All patched models are based on `simple`, for now
+        let example = Example::from_name("simple").unwrap();


Hard-coding 'simple' as the base example creates a hidden coupling between patch definitions and this code. If a patch is added that's based on a different example, this will silently use the wrong base. Consider storing the base example name alongside each patch in the PATCHES map, or adding validation that ensures all patches are actually based on 'simple'.

It'll take more changes than that, see #1080

src/example.rs

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/regenerate_test_data.sh

tsmbland

LGTM!

tsmbland · 2026-01-16T11:34:40Z

src/example/patches.rs

+/// Get all patches
+fn get_all_patches() -> PatchMap {
+    [(
+        "simple_divisible",


I think we'd want a short comment for what each patch represents/what functionality it's testing. Just an inline comment is probably fine for now, but if we have loads of patches it might be neater to define each with a function rather than just listing them all out in a vec. I could also imagine something more complex whereby the description can be printed with muse2 example info and saved to the README with muse2 example extract, but that's probably not necessary.

That's a good idea. I'll add a comment.

We might want to house the patches in a struct when we do #1080 at which point we could stick an info field in there too.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

alexdewar added 9 commits January 15, 2026 15:17

architecture_quickstart.md: Missing word

04ed85d

Separate code for working with examples from code for its CLI

4c22391

Store patches for patched examples in muse2 crate

d6df8dc

Add CLI commands to work with patched examples

a21ec24

These are hidden from the help as ordinary users shouldn't need them.

Rename: regenerate_all_data.sh => regenerate_test_data.sh

916812f

This makes it consistent with the `just` command.

Allow for just regenerating test data for certain examples

7721786

regenerate_test_data.sh: Don't suppress error messages

3326a94

regenerate_test_data.sh: Allow for regenerating data for patched exam…

ab6d02c

…ples too

Update docs for changes to regenerate_test_data.sh

77347eb

Copilot AI review requested due to automatic review settings January 15, 2026 15:42

Copilot started reviewing on behalf of alexdewar January 15, 2026 15:43 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 15, 2026 16:01

Copilot started reviewing on behalf of alexdewar January 15, 2026 16:02 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

Add test for extract_example

d4a8f51

Copilot AI review requested due to automatic review settings January 15, 2026 16:20

alexdewar force-pushed the regenerate-patch-test-data branch from 35c0d19 to d4a8f51 Compare January 15, 2026 16:20

Copilot started reviewing on behalf of alexdewar January 15, 2026 16:20 View session

Add longer doc comment for Example::extract()

a10a88e

Copilot AI reviewed Jan 15, 2026

View reviewed changes

tests/regenerate_test_data.sh Show resolved Hide resolved

alexdewar requested review from Aurashk and tsmbland January 15, 2026 16:44

tsmbland approved these changes Jan 16, 2026

View reviewed changes

alexdewar mentioned this pull request Jan 16, 2026

Allow for easily patching examples other than simple in test code #1080

Open

Add explanatory comment for simple_divisible

404ad00

Copilot AI review requested due to automatic review settings January 16, 2026 13:07

Copilot started reviewing on behalf of alexdewar January 16, 2026 13:07 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

alexdewar merged commit 0faa09b into main Jan 16, 2026
14 checks passed

alexdewar deleted the regenerate-patch-test-data branch January 16, 2026 15:47

	let example = Example::from_name("simple").unwrap();
	let example = Example::from_name("simple")
	.expect("'simple' example must exist as base for all patched models");

Allow for regenerating test data for patched examples + tidy-ups #1081

Allow for regenerating test data for patched examples + tidy-ups #1081

Uh oh!

Conversation

alexdewar commented Jan 15, 2026

Description

Type of change

Key checklist

Further checks

Uh oh!

codecov bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

alexdewar Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

alexdewar Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

tsmbland left a comment

Choose a reason for hiding this comment

Uh oh!

tsmbland Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

alexdewar Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 15, 2026 •

edited

Loading