⬆️ Add simple upgrade/downgrade package for MyST AST #1802

agoose77 · 2025-01-23T15:03:38Z

This PR adds a simple package that sets an API for upgrading and downgrading an AST. Over time, we can use this to define imperative motions between AST schema versions.

For now, this PR implements a new "version 2" of the MyST schema which drops the use of number in favour of enumerator in footnotes.

Next Steps

Hook this package into AST ingest routes in mystmd
Separately update this package to pull in newer-than-mystmd releases

changeset-bot · 2025-01-23T15:03:42Z

🦋 Changeset detected

Latest commit: d01c5a7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
myst-compat	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

rowanc1

I think the upgrade downgrade should be on the output of what is written to json in the site.

{
   version: 1,
   sha: "",
   mdast: {},
   citations: {order:} ... etc,
}

That way we can execute and move around.

Currently the upgrade/downgrade is on the mdast only (likely the bulk of the changes, but citations were a poor choice to do outside of the tree as an example...!).

rowanc1 · 2025-01-23T19:59:00Z

packages/myst-compat/src/downgrade/version_2_1.spec.ts

+import { downgrade } from './version_2_1.js';
+import type { Parent } from 'mdast';
+
+const SIMPLE_AST: Parent = {


I don't think it matters for now (as in we can get this in asap), however, I think this would be easier to manage with separate test dir with json files and then a global test runner.

fwkoch

This is looking good to me - appreciate how it is simple and explicit.

fwkoch · 2025-01-23T20:26:58Z

packages/myst-compat/src/upgrade/index.ts

+import { upgrade as upgrade1To2 } from './version_1_2.js';
+
+export function upgrade(from: string, to: string, ast: Parent): void {
+  if (from === '1' && to === '2') {


This means we need to be careful and explicit about bumping version in the myst metadata.

An alternative could be having tests against the content to determine if the upgrade is required (e.g. "if footnoteReference numbering is defined") - that could allow more flexibility to introduce compatibility updates that missed a version bump on the myst side.

I think the explicit version numbering is good, just musing on alternatives.

Maybe a slightly different thought - do we need/want a minor_version field here? Where we can have a bit more intentionality around what constitutes a major change (outputs) vs something that is pretty minor and can be dealt with with fallbacks in the theme, etc.

@fwkoch right now we need to do some work to ensure that we're talking about the same "version" (being the schema version), certainly.

I have flipped between thinking about using a minor version vs major version. These kind of changes are hard to categorise. Ultimately, my experience with Packaging leads me to feel that version numbers are not very good at communication compatibility when "compatibility" is not well-defined. And for MyST's AST schema, I think that's the case (it's used in so many different ways with differing assumptions).

The angle I've been working of late is to prefer handling compatibility explicitly by downgrading/upgrading, rather than implicitly encoding compatibility in the version number semantics.

Do either of you see a case where this might be a problem?

I agree that for the schema, major/minor versioning are less useful. Any change to the schema could be breaking for some client I think. So then a minor bump would only mean something more like "this change isn't breaking for us"

It gets extra fuzzy with how we've used myst-spec-ext - because we have that package, the entire MyST development has happened with a single, unchanged version of myst-spec despite the AST extending and evolving. Even further, the site JSON Rowan mentioned in his review is defined by myst-cli only. So - does the upgrade/downgrade logic consider all those aspects and extended fields, but only is able to apply when the spec version changes?

@fwkoch yes, this reminds me why I originally scoped the package to the AST; we don't export a promise about our page types.

Here's my view of where we go.

At some point in future, we move to 🔄 Redefine schema in terms of @types/mdast myst-spec#67

We add types for $.json files to myst-spec

myst-spec-ext becomes a shim that exports from myst-spec

We define a single version number for myst-spec that changes whenever we make an AST or auxiliary change.

We define a breaking change to be one that removes fields or changes types (in this case, "change" excludes "new fields").

This means that the MyST "spec" version is not just the MyST AST. I think that is probably a good thing, because the AST and the auxiliary state are intimately connected. This is especially pertinent now that MyST can consume .myst.json files generated by other tools.

Consequently, we have the myst.xref.json and myst.search.json files which are separately versioned.

In the meantime, we could redefine the interface of this package to be

interface File { mdast: Parent, }; export function upgrade(file: File, from: Version, to: Version) { }

This means that at some point we can grow the File interface.

@rowanc1 @fwkoch do you have any feelings about this approach?

We can not strongly type the interface. The file type will change over time, so that upgrade File type will also change (e.g. citations is in there now, and won't be in the future). The types are known after the version is known, and then we can cast to the right type, which can be File_v1 in this package, or something.

For the options I would prefer them in a dict, so that we can add to it over time { throwError: false, inPlace: true } etc. The upgrade options, should ideally be optional, and then the function should just upgrade to the latest.

upgrade(file); upgrade(file, { from: 1, to: 15 });

The upgrade process should stamp the version into the file if it isn't there (where we are right now, which is 1).

The mdast version and the file version should be the same, and I agree that myst.xref.json and myst.search.json have different version numbers.

I think we can strongly type the interface; we already do that here for the AST. The idea is not to exhaustively type the AST, though; only the types that you actually need to work with during the upgrade step.

The upgrade options, should ideally be optional, and then the function should just upgrade to the latest.

Yes, I think if we add the proper version field, we can do-away with the from version arg. We'll still want to if the version consuming the package is older than the latest, though.

agoose77 · 2025-01-24T11:18:23Z

I think the upgrade downgrade should be on the output of what is written to json in the site.
{
   version: 1,
   sha: "",
   mdast: {},
   citations: {order:} ... etc,
}
That way we can execute and move around.

Currently the upgrade/downgrade is on the mdast only (likely the bulk of the changes, but citations were a poor choice to do outside of the tree as an example...!).

I've been toying with the same idea too. Doing that would require that "version" here refers to both the AST version and the version of the page data. We probably can tie the two together.

agoose77 · 2025-02-05T12:03:59Z

packages/myst-compat/src/downgrade/index.ts

@@ -0,0 +1,17 @@
+import type { IFile } from '../types/index.js';


The idea here is that we accept unvalidated inputs, and enforce strict types inside the upgrade/downgrade routines

agoose77 · 2025-02-05T12:04:13Z

packages/myst-compat/src/types/index.ts

+export interface IFile {
+  version: any;
+  mdast: any;
+}


The generic "untyped input" interface.

agoose77 · 2025-02-05T12:06:03Z

packages/myst-compat/src/types/v1.ts

+export interface IFile {
+  version: '1';
+  mdast: any;
+}


A "more typed" per-version interface.

I don't imagine that we'll actually type mdast — this would require bundling the entire MyST JSON spec with each iteration, which is probably overkill. Ultimately, we're still mostly going to need to just tell the type system that what we did was allowed. Let's not drown in types along the way.

Agree that we don't need to do strong typing here at all.

rowanc1 · 2025-02-05T15:38:22Z

packages/myst-compat/src/index.ts

+import { upgrade } from './upgrade/index.js';
+import type { IFile } from './types/index.js';
+
+export function makeCompatible(src: IFile, to: string) {


Change this to opts: {to: string }?

Suggested change

export function makeCompatible(src: IFile, to: string) {

export function migrate(src: IFile, to: string) {

In meeting the version is required. Can add options as third arg.

rowanc1 · 2025-02-05T15:39:15Z

packages/myst-compat/src/upgrade/index.ts

+import { upgrade as upgrade1To2 } from './version_1_2.js';
+
+export function upgrade(src: IFile, to: string): void {
+  const { version: from } = src;


Suggested change

const { version: from } = src;

const { version: from = '1' } = src;

rowanc1 · 2025-02-05T15:49:13Z

I will take a pass through this and get it merged @agoose77.

agoose77 added 10 commits January 23, 2025 14:17

wip: initial myst-compat package

7551450

wip: ast up and downgrading

07d8263

test: add simple test

e7ac627

chore: run linter

5f610ff

fix: get tests working

eabe58a

fix: try to copy children where there is only one "output" object

47cadc4

fix: allow missing children

44fc61f

fix: improve types, support placeholders

a2dccaf

fix: add label and identifier information to outputs

427d85c

wip: footnotes

362008b

agoose77 mentioned this pull request Jan 23, 2025

⏬ Add mechanism for downgrading MyST AST #1734

Closed

agoose77 added 3 commits January 23, 2025 17:02

chore: add changeset

b3542f1

fix: update tests

87c938c

test: test upgrade

57d552f

agoose77 marked this pull request as ready for review January 23, 2025 17:41

chore: remove utils

35376ff

rowanc1 reviewed Jan 23, 2025

View reviewed changes

fwkoch reviewed Jan 23, 2025

View reviewed changes

agoose77 added 3 commits February 5, 2025 11:43

feat: add export for spec version

5cda3e9

wip: prepare to upgrade documents

95f0a94

fix: update tests

c7203e5

agoose77 commented Feb 5, 2025

View reviewed changes

agoose77 added 4 commits February 5, 2025 12:10

fix: plumb-in SPEC_VERSION to file writing

8e44066

fix: add missing file

fb8715f

chore: run linter

f3a4f89

chore: run linter

d01c5a7

rowanc1 reviewed Feb 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⬆️ Add simple upgrade/downgrade package for MyST AST #1802

⬆️ Add simple upgrade/downgrade package for MyST AST #1802

agoose77 commented Jan 23, 2025 •

edited

Loading

changeset-bot bot commented Jan 23, 2025 •

edited

Loading

rowanc1 left a comment

rowanc1 Jan 23, 2025

fwkoch left a comment

fwkoch Jan 23, 2025

rowanc1 Jan 23, 2025

agoose77 Jan 24, 2025 •

edited

Loading

fwkoch Jan 28, 2025

agoose77 Feb 3, 2025 •

edited

Loading

rowanc1 Feb 4, 2025

agoose77 Feb 4, 2025

agoose77 commented Jan 24, 2025

agoose77 Feb 5, 2025

agoose77 Feb 5, 2025

agoose77 Feb 5, 2025

rowanc1 Feb 5, 2025

rowanc1 Feb 5, 2025

rowanc1 Feb 5, 2025

rowanc1 Feb 5, 2025

rowanc1 commented Feb 5, 2025

		@@ -0,0 +1,17 @@
		import type { IFile } from '../types/index.js';

	export function makeCompatible(src: IFile, to: string) {
	export function migrate(src: IFile, to: string) {

	const { version: from } = src;
	const { version: from = '1' } = src;

⬆️ Add simple upgrade/downgrade package for MyST AST #1802

Are you sure you want to change the base?

⬆️ Add simple upgrade/downgrade package for MyST AST #1802

Conversation

agoose77 commented Jan 23, 2025 • edited Loading

Next Steps

changeset-bot bot commented Jan 23, 2025 • edited Loading

🦋 Changeset detected

rowanc1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fwkoch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agoose77 Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agoose77 Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agoose77 commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rowanc1 commented Feb 5, 2025

agoose77 commented Jan 23, 2025 •

edited

Loading

changeset-bot bot commented Jan 23, 2025 •

edited

Loading

agoose77 Jan 24, 2025 •

edited

Loading

agoose77 Feb 3, 2025 •

edited

Loading