Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/myst-common/src/rule-severities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const RULE_DEFAULT_SEVERITY: Record<RuleId, 'error' | 'warn'> = {
[RuleId.validProjectConfig]: 'error', // validationOpts in packages/myst-cli/src/config.ts
[RuleId.configHasNoDeprecatedFields]: 'error', // Uses both error (2×) and warn (5×); fileError, fileWarn, errorLogFn, warningLogFn in packages/mys...
[RuleId.frontmatterIsYaml]: 'error', // fileError in packages/myst-transforms/src/frontmatter.ts
[RuleId.validPageFrontmatter]: 'error', // Uses both error (6×) and warn (7×); fileError, fileWarn, errorLogFn, warningLogFn in packages/mys...
[RuleId.validPageFrontmatter]: 'error', // Uses both error (6×) and warn (10×); fileError, fileWarn, errorLogFn, warningLogFn in packages/my...
[RuleId.validFrontmatterExportList]: 'error', // Uses both error (1×) and warn (2×); fileError, fileWarn in packages/myst-cli/src/build/utils/coll...
// Export rules
[RuleId.docxRenders]: 'error', // Uses both error (5×) and warn (3×); fileError, fileWarn, errorLogFn, warningLogFn in packages/mys...
Expand Down
87 changes: 86 additions & 1 deletion packages/myst-frontmatter/src/numbering/numbering.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest';
import { fillNumbering } from './validators';
import { fillNumbering, validateNumbering } from './validators';

describe('fillNumbering', () => {
it('empty numberings return empty', async () => {
Expand Down Expand Up @@ -115,3 +115,88 @@ describe('fillNumbering', () => {
});
});
});

describe('validateNumbering — counter aliasing (#34)', () => {
const opts = () => ({ messages: {}, property: 'numbering' }) as any;

it('passes through a bare proof-family counter target normalized to fully-qualified', () => {
const out = validateNumbering({ 'proof:lemma': { counter: 'theorem' } }, opts());
expect(out?.['proof:lemma']?.counter).toBe('proof:theorem');
});

it('leaves a fully-qualified target untouched', () => {
const out = validateNumbering({ 'proof:lemma': { counter: 'proof:theorem' } }, opts());
expect(out?.['proof:lemma']?.counter).toBe('proof:theorem');
});

it('warns and drops start/format/continue/scope/reset_on_part on an aliased kind', () => {
const o = opts();
const out = validateNumbering(
{
'proof:lemma': {
counter: 'theorem',
start: 5,
format: 'roman',
continue: true,
scope: 'section',
reset_on_part: true,
},
},
o,
);
// counter alias is kept; owner-fields are dropped
expect(out?.['proof:lemma']).toEqual({
counter: 'proof:theorem',
enabled: true,
});
const warns = o.messages.warnings ?? [];
expect(warns.length).toBeGreaterThanOrEqual(5);
for (const field of ['start', 'format', 'continue', 'scope', 'reset_on_part']) {
expect(warns.some((m: any) => m.message.includes(`'${field}'`))).toBe(true);
}
});

it('preserves owner-fields on a cross-family key (alias is ignored, not the rest of the entry)', () => {
// Regression for the Copilot review on PR #40: a user typo like
// `figure.counter: proof:theorem` is a cross-family alias the
// engine refuses to honor. The validator must NOT silently drop
// `figure.start`/`figure.scope` etc. just because `counter` is
// set — otherwise valid figure config disappears.
const o = opts();
const out = validateNumbering(
{
figure: {
counter: 'proof:theorem',
start: 5,
scope: 'section',
},
},
o,
);
// counter field is kept (engine emits the family warning), and
// figure-owned fields stay intact.
expect(out?.figure?.counter).toBe('proof:theorem');
expect(out?.figure?.start).toBe(5);
expect(out?.figure?.scope).toBe('heading_2');
// No owner-field drop warnings should have fired.
const warns = o.messages.warnings ?? [];
expect(warns.some((m: any) => m.message.includes("'start'"))).toBe(false);
expect(warns.some((m: any) => m.message.includes("'scope'"))).toBe(false);
});

it('does NOT drop label or template on an aliased kind (rendering stays per-kind)', () => {
const out = validateNumbering(
{
'proof:lemma': {
counter: 'theorem',
template: 'L. %s',
label: 'Lemma %s',
},
},
opts(),
);
expect(out?.['proof:lemma']?.template).toBe('L. %s');
expect(out?.['proof:lemma']?.label).toBe('Lemma %s');
expect(out?.['proof:lemma']?.counter).toBe('proof:theorem');
});
});
22 changes: 22 additions & 0 deletions packages/myst-frontmatter/src/numbering/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,28 @@ export type NumberingItem = {
* wins. `numbering.all.scope` is the project-wide default.
*/
scope?: NumberingScope;
/**
* Shared-counter alias (#34, LaTeX `\newtheorem{name}[other]{Heading}`
* parity). When set, this kind steps the *other* kind's counter slot
* instead of its own. The aliased kind keeps its own `label`/`template`
* for rendering — only counter mechanics are shared.
*
* Currently restricted to the proof family: `proof:lemma.counter:
* theorem` makes lemma share the theorem counter, so an interleaved
* `Lemma` between two `Theorem`s advances the next theorem number.
* Bare names within the proof family are normalized to the
* fully-qualified form (`theorem` → `proof:theorem`).
*
* Slot-owner-wins for counter mechanics: `start`, `format`, `continue`,
* `reset_on_part`, and `scope` are read from the slot owner. Setting
* those on an aliased proof-family kind emits a validator warning and
* is dropped. Cross-family aliasing and cycles are detected at
* transform time (in `myst-transforms/src/enumerate.ts` when the
* resolved-counter map is built) and degrade to per-kind behaviour
* with a warning — owner-fields on cross-family entries are left
* intact since the alias itself is ignored.
*/
Comment on lines +64 to +72

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair — the docstring was mine and it was wrong. Aligned in 4728068: now says cross-family + cycle detection happens at transform time (in myst-transforms/src/enumerate.ts when the resolved-counter map is built), and notes that owner-fields on cross-family entries are left intact since the alias is ignored anyway.

counter?: string;
};

/**
Expand Down
79 changes: 79 additions & 0 deletions packages/myst-frontmatter/src/numbering/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,48 @@ const NUMBERING_ITEM_KEYS = [
'label',
'reset_on_part',
'scope',
'counter',
];

/**
* Counter-mechanics fields that are read from the slot owner when a kind
* is aliased via `counter:` (#34). Setting these on an aliased kind has
* no effect — the slot's owner wins — so the validator warns and drops
* them to keep the rendered output consistent with the LaTeX
* `\newtheorem{name}[other]{Heading}` semantics this models.
*
* `label` and `template` (the "{Heading}" arg) are intentionally absent:
* they remain per-kind so an aliased `lemma` still renders "Lemma 1.2.2"
* while sharing the theorem counter slot.
*/
const COUNTER_OWNER_FIELDS = ['start', 'format', 'continue', 'reset_on_part', 'scope'] as const;

/**
* Is this kind a proof-family kind for the purposes of `counter:`
* aliasing (#34)? Mirrors `isProofFamilyKind` in
* `myst-transforms/src/enumerate.ts` — both files need to agree on the
* family membership rule but neither owns it conceptually, so each
* keeps a local copy rather than introducing a shared dependency.
*/
function isProofFamilyKindForCounter(kind: string): boolean {
return kind === 'proof' || kind.startsWith('proof:') || kind.startsWith('prf:');
}

/**
* Normalize a bare proof-family `counter:` target (`theorem`) to its
* fully-qualified form (`proof:theorem`). The key — not the value — is
* always fully qualified in the numbering object, so authors who write
* `proof:lemma: { counter: theorem }` (the LaTeX-natural form) and
* authors who write `counter: proof:theorem` (the verbose form) both
* get the same resolution. Cross-family targets are returned as-is so
* the engine's family check can warn on them.
*/
function normalizeCounterTarget(keyKind: string, target: string): string {
if (target.includes(':')) return target;
if (isProofFamilyKindForCounter(keyKind)) return `proof:${target}`;
return target;
}

const COUNTER_FORMATS: CounterFormat[] = ['arabic', 'alph', 'Alph', 'roman', 'Roman'];

const CONTINUE_STRINGS = ['continue', 'next'];
Expand Down Expand Up @@ -222,6 +262,13 @@ export function validateNumberingItem(
}
}
}
if (defined(value.counter)) {
const counter = validateString(value.counter, incrementOptions('counter', opts));
if (defined(counter)) {
output.counter = counter;
output.enabled = output.enabled ?? true;
}
}
if (Object.keys(output).length === 0) return undefined;
return output;
}
Expand Down Expand Up @@ -340,6 +387,38 @@ export function validateNumbering(input: any, opts: ValidationOptions): Numberin
}
}
});
// #34: post-process `counter:` aliases — normalize bare proof-family
// targets to their fully-qualified form, and warn (then drop) any
// counter-mechanics fields set on aliased kinds. Cycle detection and
// cross-family enforcement happen at runtime in
// `myst-transforms/src/enumerate.ts` where node-attached warnings are
// natural; here we only handle the per-entry concerns the validator
// already has full local knowledge of.
//
// The owner-field drop is gated on the *key* being a proof-family
// kind. Cross-family entries (`figure.counter: proof:theorem`) keep
// both their `counter` field (so the engine can emit its family
// warning) and all their owner-fields (`figure.start`, `figure.scope`,
// etc.) intact — otherwise a typo'd alias would silently delete
// valid configuration the engine then refuses to honor.
Object.entries(output)
.filter(([key]) => !NUMBERING_OPTIONS.includes(key))
.forEach(([key, item]) => {
if (!item?.counter) return;
const normalized = normalizeCounterTarget(key, item.counter);
if (normalized !== item.counter) item.counter = normalized;
if (!isProofFamilyKindForCounter(key)) return;
const itemOpts = incrementOptions(key, opts);
for (const field of COUNTER_OWNER_FIELDS) {
if (defined((item as any)[field])) {
validationWarning(
`'${field}' is read from the slot owner ('${item.counter}') when 'counter' is set; ignoring on '${key}'`,
itemOpts,
);
delete (item as any)[field];
}
}
Comment on lines +404 to +420

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 4728068. Gated the owner-field drop on isProofFamilyKindForCounter(key) so cross-family entries (e.g. figure.counter: proof:theorem) now keep their counter field (engine still emits the family warning) and all their owner-fields intact. Added a regression test in numbering.spec.ts pinning the figure.counter: proof:theorem, start: 5, scope: section case.

});
if (Object.keys(output).length === 0) return undefined;
return output;
}
Expand Down
Loading
Loading