Repository navigation
Many bugfixes and performance optimizations - #17
Merged
Merged
Conversation
addParent pointed this Branch at the new parent but left it in its old parent's children, never attached the new parent to the old parent, and left the given siblings in their old parents' children. It also replaced the children of a Branch passed as the new parent, orphaning them. The new parent now takes this Branch's place among its old parent's children. The siblings are detached from wherever they were, and the new parent's existing children are kept (this Branch and the siblings are appended, as addChild does). Behavior change: addParent on a non-root Branch now changes the old parent's children (it replaces this Branch with the new parent), and a Branch passed as the new parent keeps its children. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
replace put the replacement in the parent's children but never set replacement.parent, left the replaced Branch's parent pointing into the tree, and left the replacement in its old parent's children. Set both links and detach the replacement first. Replacing a root threw a TypeError; it now throws a clear error. Behavior change: after replace, the replaced Branch is a root (parent === null), and a replacement taken from elsewhere in a tree is removed from its old position. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Tests and builds ran through npx, so each run used whatever versions were current. Pin jest 30.5.2 and rollup 4.63.5 (which reproduces the committed dist/patristic.js byte for byte) and call them from the npm scripts directly. Add a workflow that runs on pushes to master and on pull requests, across Node 20, 22 and 24. Since the tests run against dist/, it rebuilds first and fails if the committed dist/patristic.js doesn't match src/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Add clone, descendants, getLeaves, path and hasDescendant on the neighbor-joining trees, and toNewick, getLeaves, copy, getDescendant and distanceTo on caterpillar trees that parse. Single runs were noisy for the fast operations, so each timing is now the median of up to 7 runs (fewer when a run takes over a second). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
eachBefore, eachAfter, getDescendants, getLeaves, getDescendant, isConsistent, fixParenthood and isDescendantOf recursed once per level, so a caterpillar tree of a few thousand tips overflowed the call stack; parseNewick (via fixDistances) failed on 100,000 tips. Use explicit stacks instead. eachBefore and eachAfter share one traversal that reads each Branch's children the way the recursive forEach did, so callbacks that edit the tree (as simplify's does) see the same visiting order as before. getDescendants and getLeaves also fill one array rather than copying each subtree's array into its parent's, which was O(n * depth). isConsistent no longer runs parent.children.includes for every descendant (quadratic in the number of children), and returns false for a tree with a cycle rather than overflowing the stack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
toNewick, toObject and parseJSON recursed once per level, so deep trees overflowed the call stack. Build their output with explicit stacks; the output is unchanged. A 100,000-tip caterpillar now round-trips through Newick and through objects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
copy serialized the subtree to a JSON string and parsed it back, and clone went through toObject and parseJSON; both built a full intermediate representation, and copy overflowed the stack on deep trees inside JSON.stringify. Build the new Branches directly. The result is the same: new Branches carrying the original ids and lengths, with depth, height and value recomputed from the copy's root. Behavior change (edge case): copy no longer turns non-finite lengths (NaN, Infinity) into 0 as a side effect of JSON serialization; it keeps them, as clone always did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Add tests for the cases the next commits' rewrites could get wrong: paths that only go up or down, lookups that should fail from a subtree, hasLeaf on a leaf itself, and toMatrix agreeing exactly with distanceTo on a larger tree with a polytomy and on a subtree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
distanceTo, distanceBetween and sources (and so targets) found the MRCA by climbing from both Branches, then climbed again from each Branch to the MRCA with depthOf. Record each ancestor's distance during the first climb instead. Lengths are summed in the same order, so results are bit-for-bit unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
path inserted each Branch below the MRCA at a fixed position with splice, shifting the rest of the array every time: O(depth^2) for long paths. Collect the downward half and append it in reverse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Both built an array of the whole subtree on every call, then searched it. For a Branch, climb from it instead (O(depth)); for an id, search the subtree and stop at the first match. A Branch is now found by following parent links up, where it was found by following children down; the two agree on any consistent tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
toMatrix called distanceTo for every pair of leaves, each of which climbed to the MRCA and back: O(n^2 * depth), 0.8 s for 1,000 leaves. Instead, carry each subtree's list of (leaf, distance to the subtree's root) up the tree, and fill in the distance for each pair of leaves at their MRCA, when two children's lists meet. Every pair is visited once, so this is O(n^2). Distances are summed in the same order as before, so the matrix is bit-for-bit unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
addChild(data) assigned `parent` into the caller's object before passing it to the Branch constructor, which keeps it as `data`. So the caller's input was mutated, and every Branch built by addChild (including every Branch from parseNewick) held a reference to its original parent in `data`. That reference went stale on reroot and kept a detached subtree's old tree alive. Set the parent on the Branch instead. Behavior change: addChild no longer adds a `parent` key to the object it's given, and `branch.data.parent` is no longer set for Branches created by addChild or parseNewick. Nothing in patristic reads `data`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Every Branch built a random UUID-style string with a regex replace callback per character, which dominated construction: a million Branches took 3.6 s, and parsing a 100,000-tip Newick string 1.2 s. Use an incrementing counter, and assign properties directly rather than through Object.assign. A million Branches now take 30 ms, and that parse 0.23 s. Behavior change: `_guid` is now a number, unique within the loaded copy of the library, rather than a random UUID-like string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
sumRows called parseFloat on every cell, then checked that the result was a number (it always is) and added the raw cell anyway. So numeric strings were concatenated rather than summed, as they were in the rest of neighbor joining. Convert cells with Number when parseMatrix copies the matrix, and sum rows plainly. Numeric input gives the same tree. Behavior change: parseMatrix now accepts distances given as numeric strings (e.g. read from a CSV file); before, they produced a wrong tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
excise spread the excised Branch's children into splice's arguments, which overflows the call stack somewhere between 100,000 and 200,000 children in Node 22. Splice off the tail and push the children and the tail back instead, keeping the parent's children array (and its order) as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
getMRCA put every ancestor of one Branch into a Set, and the distance helper put them into a Map, at a cost of a hash insertion per level. List both Branches' ancestors in arrays and match them up from the root down instead. distanceTo between the ends of a 100,000-deep caterpillar drops from 21 ms to 5 ms per call. Results are bit-for-bit unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Step 1 loose ends and step 2: O(n²) toMatrix, no recursion, faster lookups
Move the constructor and the methods assigned to Branch.prototype into a class body, in the same order, with their bodies unchanged apart from indentation; the module-level helpers follow the class. This is the shape TypeScript can type, and it's done separately from the conversion so that the conversion's diff is about types. Behavior change: Branch's methods are now non-enumerable, as class methods are, so `for...in` over a Branch no longer lists them. Calling Branch without `new` throws a TypeError saying so; before, it threw one on `this` being undefined. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Rename src/main.js to src/main.ts and type it under `strict` (plus noImplicitOverride, noImplicitReturns and noFallthroughCasesInSwitch). `npm run build` now compiles with tsc (pinned at 7.0.2) into build/, which is git-ignored, and rollup bundles that into dist/patristic.js as before. `npm run typecheck` runs tsc without emitting. The doc script reads src/main.ts. Exported types: Id (string | number), BranchData, BranchObject, DistanceMatrix and Link. Methods that return the Branch they're called on are typed to return `this`. Runtime behavior is unchanged: the bundle has the same exports and prototype methods as before, and toMatrix, the distance functions, path, hasDescendant, hasLeaf, copy, clone and parseMatrix give bit-for-bit identical results on random trees. Where the old code throws a TypeError on a root Branch (isolate, isChildOf, the parent lookups in consolidate and fixDistances), non-null assertions keep it doing so rather than changing behavior as part of the conversion. Two fields of the neighbor-joining state that nothing read (labelToTaxon, PNewick) and an unused argument to sortWithIndices are gone. dist/patristic.js changes throughout because tsc prints the code with its own formatting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Build dist/patristic.mjs alongside the UMD bundle, and point the package's `exports` at it for `import` (and `module` for bundlers that read it). `require` and the UMD script tag still get dist/patristic.js, which is unchanged. A "./*" export keeps every file in the package reachable by path, e.g. patristic/dist/patristic.min.js. The ES module has the same named exports, plus a default export holding all of them, so `import patristic from "patristic"` (which worked against the CommonJS build) keeps working. That entry point is src/patristic.mts; src/main.ts is renamed src/patristic.ts so that tsc emits files named like the bundles. Behavior change: `import` of "patristic" now loads the ES module build. Code that both imports and requires the package gets two copies of Branch, and `instanceof` doesn't hold across them. The default export is a frozen namespace object, where the CommonJS exports object could be modified. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Copy tsc's declarations into dist/ next to the bundles they describe: dist/patristic.d.ts for the UMD build and dist/patristic.d.mts for the ES module build, and point `types` and each `exports` condition at them. @arethetypeswrong/cli reports no problems under node10, node16 (from CommonJS and from ESM) and bundler resolution. `npm run typecheck` now also compiles test/types/, which imports the package by name from an ES module and a CommonJS module under nodenext resolution, and uses @ts-expect-error to check that misuse is caught. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Run `npm run typecheck` (the library and the declaration tests) in the test workflow, and fail if a build leaves anything in dist/ modified or untracked, now that the build writes the ES module bundle and the declarations as well as dist/patristic.js. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Enable exactOptionalPropertyTypes, noPropertyAccessFromIndexSignature
and noUnusedLocals. For the first, BranchData's optional fields now say
they accept undefined, which the constructor treats as "use the
default", so callers compiling with exactOptionalPropertyTypes can pass
e.g. `{ id: undefined }`; the declaration tests now compile with it.
Left off: noUnusedParameters, which only flags rotate's unused
`recursive` parameter (part of its public signature), and
noUncheckedIndexedAccess, which flags about 100 array reads, nearly all
in the neighbor-joining and toMatrix loops that step 4 rewrites.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Mention in the README that the package now works with import as well as require and a script tag, and that it ships TypeScript declarations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Step 3: TypeScript source, ES module build, type declarations
Record the Newick output of parseMatrix on four deterministic matrices (two of random Euclidean distances, two of small integers with many exact ties) and test that it's reproduced exactly. The upcoming rewrite of neighbor joining onto typed arrays has to make the same joins, break ties the same way and do the same floating point arithmetic. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
parseMatrix kept the distance matrix as an array of arrays, re-sorted rows with Array.prototype.sort and a comparator (about two thirds of its time), marked removed rows in a Set and with -1 written into the matrix, and attached a sortIndices property to plain arrays. Move the run into a NeighborJoining class that holds: - the lower triangle of the matrix in one Float64Array, - a Uint8Array of removed flags, - each row's sorted distances and their columns in a Float64Array and an Int32Array, allocated once and reused, - and sorts rows with a stable merge argsort on typed arrays, with no comparator calls. The algorithm is unchanged, step for step, including the search bound that counts the joined row's interim sum, so the output is bit-for-bit the same: test/nj.test.js's recorded trees (including integer matrices full of ties) still match, as do 20 random matrices compared against the previous build. 2,000 taxa take 1.1 s instead of 2.5 s. Behavior changes, for invalid input only: parseMatrix reads only the lower triangle (matrix[i][j] with i > j) and takes the diagonal to be zero, so an asymmetric matrix or a nonzero diagonal no longer affects the result. A distance of exactly -1, which was the removed-row marker, is no longer ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
parseMatrix(matrix, labels, { float32: true }) stores the distance
matrix and each row's sorted distances in Float32Arrays rather than
Float64Arrays. For 2,000 taxa, that's 40 MB of typed arrays instead
of 64 MB (the column indices are Int32 either way). Distances keep
about 7 significant digits; arithmetic is still done in doubles. The
default is unchanged.
The options are typed as the exported ParseMatrixOptions, and labels
may now be null in the types as well as at run time, so options can
be given without labels.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Step 4a: neighbor joining on typed arrays (2.6–2.9× faster)
count() set each Branch's value to the number of Branches in its subtree. d3-hierarchy's count, which d3's partition and treemap layouts rely on for sizes, counts leaves: 1 for a leaf, and the sum of the children's values otherwise. Do the same; partition and treemap now lay out a Branch tree exactly as they do d3's hierarchy of it. fixDistances still sets value to the number of Branches in each subtree, as before. Behavior change: after count(), an internal Branch's value is the number of leaves below it rather than the number of Branches in its subtree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Each tree now lives in a Store: parallel arrays with one slot per
Branch. Links are Int32Arrays (parent, first and last child, next and
previous sibling); length and value are Float64Arrays; depth, height
and representing are Int32Arrays; ids and data are plain arrays. A
Branch is a view onto a slot, made the first time it's asked for and
kept, so a Branch is always the same object, and properties set on it
(such as a layout's x and y) stay with it.
parseNewick, parseJSON, parseMatrix, copy and clone build their trees
directly in a Store without making Branch objects, and toNewick,
toObject, toMatrix, fixDistances, count, getLeaves, getDescendants,
getDescendant, getMRCA and the distance methods work on slots. A
100,000-tip tree takes 20 MB instead of 56 MB once parsed, and 31 MB
with a Branch object made for every node.
Links are stored once, so they can't disagree, and edits keep them
consistent. Attaching a Branch from another tree moves its subtree into
this tree's Store, keeping each moved Branch's identity. Results of
parsing, serialization, toMatrix, the distance methods and parseMatrix
are unchanged, checked against the previous build on random trees.
Behavior changes:
- id, length, depth, height, value, representing, data, parent and
children are accessors on Branch.prototype rather than own
properties, so Object.keys, spreading and for...in no longer list
them.
- `children` returns a new array each time; changing that array doesn't
change the tree. Assigning an array of Branches to `children` makes
them the children, in order, detaching those left out.
- Setting `parent` (or setParent) moves the Branch to be the last child
of its new parent, or detaches it when set to null. Before, it only
changed the Branch's own pointer. new Branch({ parent }) also adds
the Branch to the parent's children.
- Making a Branch a descendant of itself throws "Cannot make a Branch a
descendant of itself." So trees can't become inconsistent:
isConsistent always returns true, and fixParenthood has nothing to do.
- depth, height and representing are stored as 32-bit integers.
- The data of Branches made by parseNewick, parseJSON, parseMatrix,
copy and clone is an empty object (it was {}, { id, length } or
{ children }).
- excise leaves the excised Branch detached and childless; it used to
keep stale links to its old parent and children.
- invert on a Branch whose parent isn't the root puts it in its
parent's place; before, the grandparent kept listing the old parent.
- isolate on a root throws "Cannot isolate a root Branch." (a TypeError,
as before, with a clearer message).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
d3-hierarchy marks leaves by leaving `children` undefined, and its tree, cluster and pack layouts rely on that: with an empty array they threw or produced NaN. A leaf's `children` is now undefined; assigning null or an empty array to `children` makes a Branch a leaf. All five tested d3 layouts now lay out a Branch tree exactly as they lay out d3's own hierarchy of it. Behavior change: a leaf's `children` is undefined rather than []. Code like `branch.children.length` or `branch.children.forEach(...)` needs to allow for leaves (isLeaf(), or `branch.children ?? []`), and the TypeScript type is now `Branch[] | undefined`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Each Store keeps a TreeIndex for the tree last asked about, rebuilt when the tree's structure has changed since (every link, unlink, added slot and reorder counts a version): - the tree's slots in pre-order, each slot's position in that order and its subtree's size, so a subtree is a contiguous run of the order and "is d under a" is two comparisons; - each slot's number of edges from the root; - built when first needed (and again after an id changes), where each id appears in the order. getDescendant and hasDescendant / hasLeaf by id look the id up and take the first match in pre-order inside the subtree, as before. isDescendantOf and hasDescendant with a Branch compare positions. getMRCA and the distance methods climb only from each Branch to the MRCA, the deeper one first, instead of listing both Branches' ancestors to the root; lengths are summed in the same order, so distances are bit-for-bit unchanged. getDescendants and getLeaves read the order. Distances are deliberately not taken as differences of distances from the root: that rounds differently from summing the path, and distanceTo would stop agreeing exactly with toMatrix. distanceTo between the ends of a 100,000-deep caterpillar takes 0.5 ms instead of 6 ms, and repeated lookups by id no longer scan the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Explain that parent and children work as in d3-hierarchy and always agree, that changing the array children returns doesn't change the tree, and that d3's layouts work on a tree directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Step 4b: trees in typed arrays, Branch as a view, d3 layouts work directly
`npm run doc` documented every JSDoc comment, so the API reference listed Store, TreeIndex, NeighborJoining, ArgSorter, move, attach, Replacer and Branch's _store and _index fields. Tag them @Private, which documentation.js leaves out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
The publish workflow ran on Node 16 (which jest 30, rollup 4 and the test suite don't support) and published whatever was committed. Run it on Node 22 with the v4 checkout and setup-node actions, and before `npm publish`, rebuild and fail if dist/ doesn't match, type-check and run the tests, as the test workflow does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Bump the version to 0.7.0 (package.json, package-lock.json and the exported `version`), rebuild dist/ including patristic.min.js, regenerate docs/README.md, and add a CHANGELOG.md describing the release: the breaking changes from storing trees in typed arrays and following d3-hierarchy, the fixes, the new ES module build, TypeScript declarations and parseMatrix option, and the performance changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Release 0.7.0
Test what the regex tokenizer does with valid input, which the scanner
replacing it must keep: whitespace around punctuation is ignored,
spaces inside and underscores in unquoted labels are kept, a missing
semicolon and empty labels and lengths are accepted, and lengths take
any JavaScript number format.
Two bugs are recorded as test.failing: the label of a tree that's a
single Branch ("A;") is dropped, and text after the first ";" is parsed
into the same tree.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
parseNewick split the string with a regex and assigned tokens by
looking at the one before. Replace that with a scanner that walks the
string once and builds the tree as it goes (still without recursion):
- Labels may be quoted with single or double quotes, so they can hold
spaces, commas, colons, parentheses, semicolons and brackets; a
doubled quote stands for one ('O''Brien').
- Comments in square brackets are skipped wherever they appear.
- Unquoted labels behave as before: spaces inside them and underscores
are kept, and whitespace around punctuation is ignored.
- Malformed input throws a SyntaxError naming the problem and its
position: a missing ')', a ')' without a matching '(', a ',' outside
parentheses, or an unterminated quoted label or comment.
On 900 Newick strings (random trees, compact and with whitespace
scattered around the punctuation, with and without the semicolon) the
old and new parsers build identical trees. Parsing a 100,000-tip tree
takes 117 ms instead of 190 ms.
Behavior changes:
- A tree that's a single Branch keeps its label ("A;" was read as an
unlabeled Branch).
- Parsing stops at the first ";"; text after it used to be parsed into
the same tree.
- Square brackets start comments, so "A[x]" is now labeled "A".
- A label starting with a quote is read as a quoted label.
- Malformed input throws instead of producing a tree.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
toNewick wrote ids as they were, so an id with a comma, colon,
parenthesis or semicolon produced Newick that couldn't be read back.
Write ids containing whitespace, punctuation, brackets or quotes in
single quotes, doubling any single quotes inside. Every id now
round-trips through toNewick and parseNewick.
Behavior change: ids containing whitespace (e.g. "Homo sapiens") are
now written quoted ('Homo sapiens'). Newick doesn't allow blanks in
unquoted labels, and other tools may split them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
NHX comments ([&&NHX:key=value:key=value]) carry per-Branch annotations such as species and duplication events. parseNewick reads them into the data.nhx of the Branch they follow, as an object of strings, and toNewick writes data.nhx back after the Branch's label and length. Other comments are still skipped. BranchData's type gains the optional nhx field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Add an Unreleased section listing the scanner's breaking changes, new features, fixes and speedup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Step 5: single-pass Newick scanner with quoted labels, comments and NHX
parseMatrix(matrix, labels, { method: "bionj" }) builds the tree with
BIONJ (Gascuel 1997). It chooses pairs and branch lengths as neighbor
joining does, but sets the new node's distance to each other node to
lambda * (D[i][k] - d_i) + (1 - lambda) * (D[j][k] - d_j), with lambda
chosen (and clamped to [0, 1]) to minimize the variance of those
distances, tracked in a variance matrix updated alongside. With lambda
= 1/2 that's neighbor joining's update.
BIONJ reuses the RapidNJ search: it finds the exact minimum whatever
the update, because each pair's current distance is always in the
sorted row of whichever of its nodes was made last. NeighborJoining
keeps the variances in a second lower-triangle array (Float32 with the
float32 option) and has a BIONJ join step; neighbor joining's join is
untouched, so its output stays bit-for-bit the same.
Tests compare BIONJ with a textbook O(n^3) implementation
(test/builders-reference.js) on random matrices: the same tree, and the
same distances to 1e-9 under one of two variants. With four nodes left,
neighbor joining's criterion is always exactly tied between
complementary pairs, so rounding picks one; that doesn't change
neighbor joining's tree, but does change BIONJ's branch lengths (as in
Gascuel's own implementation). Over 500 random matrices, the results
matched the reference exactly with one choice or the other.
An unknown method throws "Unknown parseMatrix method ...".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
parseMatrix(matrix, labels, { method: "upgma" }) builds a rooted,
ultrametric tree by UPGMA: it repeatedly merges the two closest
clusters into a node at half their distance, setting each child's
branch length to that height minus its own, and takes the merged
cluster's distance to every other cluster to be the size-weighted mean
of its parts'. Each row remembers its nearest neighbor, refreshed only
when a merge affects it, so finding the closest pair takes O(n) per
merge rather than O(n^2). Ties go to the lowest-numbered pair, as in
the textbook scan. It builds the tree directly in a Store, and the
float32 option applies.
Tests check:
- the worked example from Wikipedia's UPGMA article (5S ribosomal RNA),
including every branch length;
- that every leaf is the same distance from the root;
- the same clades and the same distances (to 1e-9) as a textbook
O(n^3) implementation on random matrices;
- two taxa, and that the input matrix is left alone.
2,000 taxa take 0.64 s (neighbor joining: 1.07 s).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Add BIONJ and UPGMA as parseMatrix methods
The coverage/ directory was a report from May 2025, before the rewrite. It's generated by npm run coverage, so it's ignored now. package.json had no files field, so the npm package held the whole repository: tests, demo, paper and the stale coverage report. It now holds dist/, the README, the changelog and the license, which takes it from 52 files to 9. Public behavior change: deep imports of anything outside dist/, such as patristic/test/..., no longer resolve from the published package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
tsc writes source maps (with the sources inlined), and a small Rollup plugin feeds them into the bundles, so dist/patristic.js.map and dist/patristic.mjs.map map back to src/patristic.ts. Jest's coverage follows them, so npm run coverage reports lines of the source rather than of the UMD bundle, whose wrapper counted as untested branches. Test helpers are left out of the report. The maps are deterministic, so CI's dist staleness check covers them. Public change: the bundles end with a sourceMappingURL comment, and the package includes the two .map files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
parseMatrix([[0]]) and parseMatrix([]) threw "RangeError: Invalid array length" from the neighbor joining setup. Now a single taxon makes a single leaf, and an empty matrix throws "parseMatrix needs at least one taxon.", as UPGMA already did. The check moves from upgma() into parseMatrix so all three methods share it. Found while filling gaps in the coverage report. Public behavior change: the error and result above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Adds tests for every line and branch of src/patristic.ts that no test
reached:
- Branch: new Branch({ parent }); the depth, height and data setters;
addChild() and addParent() with no argument; addParent on a root
given a detached Branch from the same tree; hasChild by id on a leaf;
invert below the root; isConsistent on corrupted storage;
isDescendantOf by id and with a falsy argument; isolate on a root;
normalize() and sum() with their defaults; toNewick(true); toString
with a replacer and width; distanceTo a detached Branch; traversing a
lone leaf.
- parseJSON with missing ids and lengths and holes in children.
- NHX fields without a value, and empty annotations.
- Neighbor joining on matrices that break the triangle inequality,
where the joined node can have the largest row sum, against a
textbook implementation (the BIONJ reference with lambda fixed at
1/2).
- BIONJ with two identical taxa, whose zero variance keeps lambda at
1/2.
- UPGMA where averaging rounds a distance below its inputs, so a
cluster's nearest neighbor changes.
These describe existing behavior, so they pass as written.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
UPGMA searched for the first row not yet removed to find the root, but each join keeps the lower-numbered of its two rows, so row 0 is never removed and the search never moved. The coverage report flagged the loop as unreachable. No change in behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Jest's coverage threshold is 100% of statements, branches, functions and lines of src/patristic.ts, and the test workflow runs npm run coverage in place of npm test, so a change that leaves code untested fails CI. Removing one test makes the run fail with the shortfall listed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Full test coverage, source maps, and a trimmed npm package
Bump the version to 0.7.1 (package.json, package-lock.json and the exported version), rebuild dist/ including patristic.min.js and the source maps, regenerate docs/README.md, and turn the changelog's Unreleased section into 0.7.1: the Newick scanner (quoted labels, comments, NHX, errors on malformed input), BIONJ and UPGMA, source maps, the smaller npm package, and the parseMatrix fix for empty and single-taxon matrices. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMtaXDcaMf7a4LWHGMAAsd
Release 0.7.1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.