Skip to content

Read a list operator's bare string children as what they look like - #1847

Merged
dqnykamp merged 12 commits into
Doenet:mainfrom
dqnykamp:infer-list-value-type
Sep 6, 2026
Merged

dqnykamp merged 12 commits into
Doenet:mainfrom
dqnykamp:infer-list-value-type

Conversation

@dqnykamp

@dqnykamp dqnykamp commented Sep 6, 2026

Copy link
Copy Markdown
Member

The problem

Markup that says exactly what it means produced nothing:

<sort>d a b</sort>              <!-- rendered "" -->
<tally>apple fig apple</tally>  <!-- counted nothing -->
<shuffle>d a b</shuffle>        <!-- rendered "" -->

Each reported that a type attribute was required, then ignored the string entirely. Every component reading its children as a list of comparable values behaved this way — <sort>, <shuffle>, <sortIndices>, <tally>, <argMin>, <argMax>, <indexOf>, <searchSorted>.

Requiring the attribute was a real choice, not an oversight: read as text by default, <tally>1 2 10</tally> would order its categories 1, 10, 2. But the price was that the most natural thing a beginner writes returns an empty result.

What changed

Bare strings are read by their content. Every whitespace-separated piece naming a number makes the list numeric; anything else makes it text.

markup before now
<sort>10 2 1</sort> `` + warning 1, 2, 10
<sort>d a b</sort> `` + warning a, b, d
<sort>10 2 x</sort> `` + warning 10, 2, x
<sort>1/2 2 1</sort> `` + warning 0.5, 1, 2
<tally>apple fig apple</tally> nothing apple, fig -> 2, 1

This is the rule the values already followed. When they arrive as components, allAreNumeric is true only if every one of them is numeric, and a single text among numbers sends the whole list to a text comparison. Inferring the same way for strings means an author who writes 1 10 3 and an author who references a <numberList> get the same answer, and 1 10 x reads as text either way. It is one rule now, not two.

type survives as an override for when the look is misleading: 007 008 counts the numbers 7 and 8, and type="text" keeps the leading zeros. It still governs bare strings only — a referenced component keeps the type it already has.

An invalid type is dropped, not replaced

type="bad" used to become math. It is now reported and discarded, so the string children are read exactly as they would be with no type at all.

Replacing it read them as maths, which is visible wherever the values are not maths: <tally type="txt">apple fig apple</tally> reported its categories as a p p l e and f i g, and now reports apple and fig.

The drop reaches the string children only. categories and target resolve an invalid type by their own route, which still replaces it (Invalid type txt, setting type to number), so <tally type="txt" categories="apple fig">apple fig apple</tally> still reads every category as NaN, matches nothing, counts 0, 0 and reports that a category was named twice — one NaN reads like another. That is unchanged by this PR: the same document produces the same counts and the same set of diagnostics on the base, with only the first warning's wording different. It is the deferred _componentWithSelectableType half of #1825, below.

The numeric test

A token is read with Doenet's math parsertextToAst, the one <math> and <number> both use, carrying Doenet's applied function symbols. Not with JavaScript's Number().

That distinction is deliberate, and it is why <sort>1e5 2</sort> is text while <sort type="number">1e5 2</sort> renders 2, 100000. parseScientificNotation is declared on 42 components, defaults to False, and recognizes an uppercase exponent only — so 1e3 is never scientific notation in DoenetML, under any setting. 0x10 and 0b101 are not a DoenetML notation at all; <number> accepts them only because it asks Number() before the parser, which is #1849. Inference declines to guess from a JavaScript literal what an instructor did not write. An author who wants an exponent read says so: <mathList parseScientificNotation="true">1E3 2 5E2</mathList> into a <sort> gives 2, 500, 1000.

Which math parser. Number.js reads its content with textToAst, configured with Doenet's own list of applied function names; me.fromText uses the parser library's shorter default, which has abs and nCr but not min, max, mean, median, sum, prod, count, std or variance. Read by that one, <sort>min(1,2) 3</sort> rendered 3, min(1,2) as text while <number>min(1,2)</number> is 1 and <sort>nCr(4,2) 3</sort> next to it read as numbers.

The result is tested with typeof v === "number" && !Number.isNaN(v), and neither half is spare:

  • Number.isFinite alone would rule out an infinity, which is a number the comparison handles — compareExtractedValues tests equality before subtracting, precisely so Infinity equals itself.
  • !Number.isNaN alone would admit the complex object that i evaluates to, on which every comparison is NaN. Such a value is called numeric and then never equals anything, not even itself. That exact mistake shipped in Add a barChart component, rendered with PreFigure #1829 and was caught by review; this is the same trap one layer down.

Tokens are split on whitespace, while the wrapping splits on whitespace outside parens. They part company only where whitespace falls inside parens, and only in the safe direction — the piece holding the unmatched ( is not a number under any reading, so <sort>(1 + 2) 4</sort> is read as text where <sort>(1+2) 4</sort> is read as numbers. The inference never calls a list numeric that the wrapping would then fill with something unreadable.

Diagnostics

Both affected codes are in the v0.7.26 lock and string-children-need-type is translated into 347 locales, so neither could be edited in place. packages/i18n/README.md describes the path, and lint:i18n names it directly when the last call site goes:

  • doenet-w0013 — retired. Nothing needs a type declared any more.
  • doenet-w0014 — retired. Its message id was invalid-type-defaulting-to-math, so repointing the code would have orphaned 347 translations of a key naming behavior that no longer exists.
  • doenet-w0145 — new, English only: "Invalid type X … Ignoring it and reading the values as though no type had been given."

Retiring keeps each code's entry and message, so no other locale file is touched. lint:i18n passes: 603 keys across 348 locales, 242 codes, 3 retired.

Compatibility

Of the eight components, only <sort> and <shuffle> have shipped in 0.7.26 — the rest are unreleased. Two shapes of released markup produced a non-empty result before and produce a different one now. Both were run on this branch and on its base to establish the before.

A reference next to a bare string. The string used to be dropped, so the document rendered the reference alone:

markup before now
<sort>$mi 3</sort> (<mathInput prefill="5"/>) 5 + 2 warnings 3, 5
<sort>$nl x</sort> (<numberList>3 1</numberList>) 1, 3 + 2 warnings 1, 3, x

A type that is not one of the four. It used to be replaced with math, so the strings were read — as maths. Dropping it instead reads them as what they look like, which changes both the rendered value and the replacement component type. Every one of these already reported the type as invalid:

markup before now
<sort type="txt">1/2 2 1</sort> 1/2, 1, 2 (math) 0.5, 1, 2 (number)
<shuffle type="txt">1/2 2 1</shuffle> 2, 1, 1/2 2, 1, 0.5
<sort type="txt">sqrt(4) 3</sort> sqrt(4), 3 2, 3
<sort type="txt">pi 3</sort> 3, π 3, 3.14
<sort type="letters">d a b</sort> a, b, d (math) a, b, d (text)

The changed replacement type is visible downstream: with <sort name="s" type="txt">1/2 2 1</sort>, $s[1].latex was \frac{1}{2} and is now 0.5, and <sum>$s</sum> was 1/2 + 1 + 2 and is now 3.5. With type="letters" the items become <text>, and $s[1].latex resolves to nothing at all where it used to render a.

Nothing changes for a released document that writes no type and has no bare strings, or that writes one of the four valid types.

Tests

Four tests asserted the replaced behavior and now assert the rule: the two "string children without type emit warning" tests, and the two "invalid type defaults to math" tests. New coverage for numeric inference, textual inference, one word sending a numeric-looking list to text, scientific and hexadecimal notation staying text, Doenet's applied function names (min, mean, nCr), and the 007 override. 140 pass across sort, shuffle, countoperators and listoperators.

src/test/diagnostics/diagnosticCodes.test.ts is also in the diff. It used <sort name="s">a b c</sort> as a source guaranteed to raise a warning, and that document is now correct, so the file failed on this branch — a suite outside the four above, which is why it was missed until a review cycle widened the net. It now takes its warning from a component this PR does not touch. With it, 159 pass across five files.

Verified by running rather than reading: which tokens evaluate to numbers (1/2 -> 0.5, 2^3 -> 8, 007 -> 7, pi -> 3.14159, i -> a complex object, x -> NaN, and 1e5 -> NaN from the math parser but 100000 from Number, which is why the parser is the one consulted), every example added to the reference pages, both before/after tables above, and a fuzz of 60 000 random tokens finding none that the inference calls a number and <number> then reads as NaN, and none carrying an unmatched ( that the inference calls a number.

Not in this PR

How categories and target resolve their own type — the _componentWithSelectableType half of #1825. This is also why an invalid type is only half dropped: the children stop being read as maths, but categories and target still turn txt into number. categories reaches the right answer by a different route (comparableCategory rereads a textual category against numeric values), so <tally categories="apple fig">apple fig apple</tally> counts correctly with no type written. target does not: <indexOf target="b">a b c</indexOf> reports 0 while <indexOf type="text" target="b">a b c</indexOf> reports 2, and <searchSorted target="b">a b c</searchSorted> reports 1 rather than 2. That is not a change here — the children produced nothing before, so the answers were the same — but inferring the children's type makes it the remaining half of the inconsistency, and the plumbing behind target wants its own change.

Nor a guard on the math parser: 18 or more consecutive unmatched opening delimiters in one token take the parser exponentially long — <math>((((((((((((((((((1+2</math> already takes 23 seconds on main, without any of this. Inference now reaches that parser from markup with no type attribute, so the same input freezes <sort> too, but the defect is in the parser and belongs there.


🤖 Generated with Claude Code

https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo

dqnykamp and others added 3 commits September 6, 2026 13:21
`<sort>d a b</sort>` rendered nothing. So did `<tally>apple fig apple</tally>`
and `<shuffle>d a b</shuffle>`: each reported that a `type` was required,
ignored the string and produced an empty result, for markup that says exactly
what it means.

Bare strings are now read by their content — every piece naming a number makes
the list numeric, anything else makes it text. That is the rule the values
already followed when they arrived as components, where `allAreNumeric` is true
only if every one of them is numeric and a single text sends the whole list to a
text comparison. Applying it to strings means `1 10 3` and a referenced
`<numberList>` get the same answer, and `1 10 x` reads as text either way.

The numeric test is `typeof v === "number" && !Number.isNaN(v)`, and neither
half is spare: `Number.isFinite` would rule out an infinity, which the
comparison handles, while a bare `!isNaN` would admit the complex object `i`
evaluates to, which is never equal to anything, not even itself.

An invalid `type` is now reported and dropped rather than replaced with `math`,
so it behaves as if unwritten. Replacing it is how `<tally type="txt">` came to
make every category `NaN` and then report a category named twice.

`doenet-w0013` and `doenet-w0014` are retired in place — one asked for a type
nothing needs, the other named a fallback that no longer happens — and
`doenet-w0145` replaces the second. Retiring keeps each code's entry and its
message, so none of the 347 translated catalogs is touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
`<sort>1e5 2</sort>` sorted as text and rendered `1e5, 2`, while
`<sort type="number">1e5 2</sort>` rendered `2, 100000`. The inference asked
the math parser alone whether a token names a number, but the `<number>` it
then goes on to create converts its content with `Number` first and reaches
for the math parser only when that yields `NaN`. The math parser has neither
scientific notation nor hexadecimal, so the two disagreed on every token
written that way and the inference contradicted the component it was about to
build.

Reading a token in the same two passes settles it: `1e3 5e2 2e4` sorts to
`500, 1000, 20000` and `0x10 9` to `9, 16`.

Two comments are corrected against the code they sit on. The claim that the
whitespace split and the paren-aware split "can only disagree about a token
containing a paren, which does not evaluate to a number under either
splitting" is false: `(1 + 2)` evaluates to 3 under the paren-aware split, so
`<sort>(1 + 2) 4</sort>` is read as text where `<sort>(1+2) 4</sort>` is read
as numbers. The disagreement runs one way only — the piece holding the
unmatched `(` never parses — and that is the property worth recording. The
sugar's own docstring, meanwhile, still said that without a `type` the strings
are left alone and a warning is issued.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
…ting

The `type` example demonstrated one override, `text` for a numeral meant as a
label. The `math` case had no example because the first attempt at one did not
work: contrasting `x+1` with `x + 1` fails, since the sugar splits on
whitespace and the second is three tokens.

`2x` against `2*x` needs no spaces. Neither evaluates to a number, so both are
read as text and compared letter by letter — two categories. Read as maths they
are the same expression, and the three values become one category counted three
times. Ran both before writing them down.

The introduction regains the clause about expressions, which was removed when
the earlier example failed rather than because the case was not real.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
dqnykamp and others added 5 commits September 6, 2026 13:44
`<sort>min(1,2) 3</sort>` rendered `3, min(1,2)` — read as two words and
ordered alphabetically — while `<number>min(1,2)</number>` is 1 and
`<sort type="number">min(1,2) 3</sort>` renders `1, 3`. Beside it,
`<sort>nCr(4,2) 3</sort>` read as numbers and rendered `3, 6`.

The two passes were already the ones `<number>` uses, but the second pass was
not the same parser. `Number.js` converts its content with `textToAst`, built
with Doenet's own list of applied function names; the inference asked
`me.fromText`, which uses the parser library's shorter default. The two lists
agree on `abs`, `sqrt`, `nCr` and the trigonometry, and part company on `min`,
`max`, `mean`, `median`, `sum`, `prod`, `count`, `std` and `variance` — every
one of which a `<number>` evaluates and the inference called a word.

Asking `textToAst` settles it. A fuzz of 60 000 random tokens finds none that
the inference now calls a number and `<number>` then reads as `NaN`, and the
paren asymmetry is unchanged: of 7 168 tokens carrying an unmatched `(`, none
is read as a number, so the whitespace split still only ever errs toward text.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
`gives every warning either a registered code or an English message` asserted
that its document produced at least one warning, and the document it used to
produce one was `<sort name="s">a b c</sort>`. Reading bare strings by their
content makes that document correct, so the test found no warnings and failed
on the assertion before it reached the property it is about.

An index that cannot be applied is a mistake in the markup rather than a
missing declaration, so it stays a warning however the components around it
change, and `doenet-w0100` is already pinned by its own test lower in the same
file. The neighboring test carries the same note about not resting on a
component that is merely next in line to be fixed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
The changeset and the PR both said that for `<sort>` and `<shuffle>` — the
only two of the eight that have shipped — the change reaches nothing that
worked. Running each shape against the branch and against its base says
otherwise, twice.

A reference beside a bare string produced a result before, from the reference
alone: `<sort>$mi 3</sort>` rendered `5` and dropped the `3`, and now renders
`3, 5`. And a `type` outside the four was replaced with `math` rather than
dropped, so the strings were read — as maths. `<sort type="txt">1/2 2 1</sort>`
rendered `1/2, 1, 2` and now renders `0.5, 1, 2`; `<sort type="txt">pi 3</sort>`
rendered `3, π` and now `3, 3.14`. The replacement type changes with the value,
which the document next to it can see: `$s[1].latex` was `\frac{1}{2}` and is
`0.5`, `<sum>$s</sum>` was `1/2 + 1 + 2` and is `3.5`, and under
`type="letters"` the items become text, so `.latex` resolves to nothing where
it used to render `a`.

Neither is a reason to keep the old behavior — both documents already reported
a problem — but a compatibility note that says nothing changes is worth less
than no note. The changeset now names both, and gains the reading that decides
whether a piece is a number, since `min(1,2)` counting as one is not obvious
from "naming a number".

`tally.mdx` loses a plural disagreement in the same sentence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
The last two cycles moved the inference toward `<number>`'s reading and reached
`Number()` itself, which admits every JavaScript numeric literal. That is the
wrong target. `parseScientificNotation` is declared on 42 components, defaults
to false, and recognizes an uppercase exponent only, so `1e3` is never
scientific notation in DoenetML under any setting — and `0x10` and `0b101` are
not a DoenetML notation at all. `<number>` reads them only because it asks
`Number()` before the parser, which is Doenet#1849 and has to wait for 0.8.

Inferring from a JavaScript literal what an instructor did not write is the
wrong direction for a language whose authors are teachers. So the `Number()`
pass is gone and the parser decides alone.

Kept from those cycles: the parser is Doenet's own `textToAst`, so `min(1,2)`
and `mean(1,2,3)` are numbers here exactly as they are inside a `<number>`.
That fix was about *which* math reading, and it was right.

`<sort>1e3 5e2 2e4</sort>` is three words again, and the test says why rather
than restating what. An author who wants the exponent read has
`<mathList parseScientificNotation="true">`, which a `<sort>` then orders
`2, 500, 1000` — verified.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
Removing the `Number()` pass left the prose around it describing a reading the
function no longer performs. `tokenIsRealNumber`'s docstring still opened with
"read in the two passes `<number>` reads its own content in — so `1e5` and
`0x10` are numbers", and the changeset still said a piece names a number
"whenever a `<number>` around it would be one, so `1/2`, `2^3`, `1e5`, `0x10`,
`pi` and `min(1,2)` all count". Ran both: `<sort>1e5 3</sort>` renders
`1e5, 3` as text and `<sort>0x10 3</sort>` renders `0x10, 3`. Both now say why
the parser decides alone, which is what the code does.

The second claim was that dropping an invalid `type` makes it behave exactly as
if unwritten, and that replacing it with `math` is how
`<tally type="txt" categories="apple fig">` came to read every category as
`NaN` and report a category named twice. Ran the document on this branch and on
its base: `0, 0` either way, with the same warning about a repeated category
and the same `Invalid type txt, setting type to number` from the attribute's
own resolution. The drop reaches the children and nothing else, so that symptom
is untouched — it is the deferred `_componentWithSelectableType` half of Doenet#1825.
The changeset, the code comment and the PR now scope the claim to the children
and cite a harm the drop really does fix: `<tally type="txt">apple fig apple</tally>`
reported its categories as `a p p l e` and `f i g`, and now reports `apple` and
`fig`.

`tally.mdx` regains the sentence saying that a written-out `type` also decides
how `categories` is read. It was dropped as part of the same rewrite and is
still true: `<tally type="math" categories="2x">2x 2*x 2x</tally>` counts 3
where the same markup without the `type` counts 2. `sort.mdx` gains one
sentence on what counts as a number, since the page asserted the rule without
ever showing that `1/2` and `pi` satisfy it — `<sort>1/2 2^3 pi sqrt(4)</sort>`
renders `0.5, 2, 3.14, 8`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Type inference exposes a known parser stall, and the generated tally attribute documentation still contradicts the new behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds automatic numeric/text inference for bare-string children in list operators.

Changes:

  • Infers bare-string types using Doenet’s math parser.
  • Replaces obsolete diagnostics and expands tests.
  • Updates reference documentation and release notes.
File summaries
File Description
.changeset/infer-list-value-type.md Records the user-visible behavior change.
packages/i18n/diagnostic-codes.lock.json Registers diagnostic doenet-w0145.
packages/i18n/locales/en/diagnostics.ftl Adds the invalid-type message.
packages/i18n/src/diagnostics.ts Adds and retires diagnostic codes.
packages/i18n/src/generated/messageKeys.ts Adds the generated message key.
packages/doenetml-worker-javascript/src/utils/listValues.js Implements type inference and invalid-type handling.
packages/doenetml-worker-javascript/src/test/tagSpecific/sort.test.ts Tests inferred sorting behavior.
packages/doenetml-worker-javascript/src/test/tagSpecific/shuffle.test.ts Tests inferred shuffling behavior.
packages/doenetml-worker-javascript/src/test/tagSpecific/countoperators.test.ts Tests inferred tally values.
packages/doenetml-worker-javascript/src/test/diagnostics/diagnosticCodes.test.ts Uses a persistent diagnostic scenario.
packages/docs-nextra/pages/reference/tally.mdx Documents tally inference and overrides.
packages/docs-nextra/pages/reference/sort.mdx Documents sort inference.
Review details
  • Files reviewed: 11/12 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/doenetml-worker-javascript/src/utils/listValues.js
Comment thread packages/docs-nextra/pages/reference/tally.mdx Outdated
…uestion

The generated attribute table said `type` was something bare string children
"require", directly under a page saying they no longer do. The description on
`<tally>`, and the shared one behind `<argMin>`, `<argMax>`, `<indexOf>` and
`<searchSorted>`, now say what omitting it does; `<sort>` and `<shuffle>` said
only "component type to sort children as" and say the same thing too.

`tokenIsRealNumber` also stops calling the parser on a token whose brackets do
not close. Such a token never names a number, and asking is expensive: parsing
a run of unmatched openers is exponential. That halves the cost of the case —
an untyped depth-16 token measured 9.3 s before the guard and 4.8 s after,
because the parse was happening twice.

What is left is not this file's: a plain `<text>` holding the same token costs
about 5 s on its own, on `main`, today. Bare strings did not become components
at all before this change, so untyped markup could not reach it; now it can.
That is measured on Doenet#1852 rather than worked around here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Bare strings are still discarded when mixed with explicitly authored component children.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/doenetml-worker-javascript/src/components/CountOperators.js:163

  • “All numbers” is plural, so “makes” is grammatically incorrect. Rephrase this generated-reference description and regenerate the schema.
                "Component type to interpret bare string children as. Omit it and they are read as what they look like: all numbers makes the list numeric, anything else makes it text. Also overrides how `categories` is read, which is otherwise text.",
  • Files reviewed: 15/17 changed files
  • Comments generated: 5
  • Review effort level: Balanced

Comment thread packages/doenetml-worker-javascript/src/utils/listValues.js
Comment thread packages/doenetml-worker-javascript/src/components/Shuffle.js Outdated
Comment thread packages/doenetml-worker-javascript/src/components/Sort.js Outdated
Comment thread packages/doenetml-worker-javascript/src/utils/listIndexOperators.js Outdated
…ee with the code

The four `type` descriptions said "all numbers makes the list numeric",
reading as a subject-verb disagreement where the reference pages already
said "every piece naming a number makes the list numeric". Take the
pages' wording, which is also the more precise of the two: what is being
asked of each piece is that it name a number, not that it be a numeral.

The state-variable comment in `CountOperators.js` still described the
behavior this branch replaced -- bare children with no fallback, asked to
declare a type -- three lines under a comment describing inference.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation, diagnostics, generated artifacts, documentation, and focused tests are consistent with the stated behavior.

Review details
  • Files reviewed: 15/17 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

dqnykamp and others added 2 commits September 6, 2026 16:13
The section demonstrated `number`, `text` and `boolean` on inputs that
sort the same way without any of them, and then showed a referenced list
keeping its own type -- which also works out unchanged when `type` is
absent. Three examples, none of which gave a reason to write the
attribute.

Replaced with the cases where the reading actually differs, each shown
against the same input with no `type`: numerals that are labels, where
being read as numbers reorders them and drops their leading zeros;
expressions that are one value as maths and two as text; and booleans,
where alphabetical order hides the difference until the capitalization
varies.

Also says plainly that `type="number"` has no use, since a piece that
names a number is already read as one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
The bare-string paragraph explained inference and `type` immediately
above an attribute table listing `type` and an attribute example
demonstrating it on the two cases where it matters.

The `categories` paragraph was closer to a duplicate still: "omit it and
they are the distinct values present, in sorted order" is the attribute's
own description, rendered by `AttrPropDisplay` four lines below, and the
attribute and property examples both show it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016YdEWccVoUPCJoDYxKmHRo
@dqnykamp
dqnykamp merged commit c5f39d5 into Doenet:main Sep 6, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants