Fix normative defects in tool registration, notification, and observation - #244
Merged
Merged
Conversation
…tion - registerTool() now writes to the internal context's tool map rather than indexing the model context struct itself (webmachinelearning#213) - perform an observation asserts the active document IS fully active, correcting an inverted assertion (webmachinelearning#214) - notify documents of a tool change uses inclusive descendant navigables so the registering document receives toolchange (webmachinelearning#215) - Editorial: drop a stray possessive in perform an observation
domfarolino
approved these changes
Aug 17, 2026
domfarolino
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for catching these tricky issues, much appreciated!
github-actions Bot
added a commit
that referenced
this pull request
Aug 17, 2026
) SHA: 3aafb9c Reason: push, by domfarolino Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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
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.
Three normative fixes, each a case where the algorithm text does not do what the surrounding spec says it does, plus one editorial nit in the same area.
registerTool()writes to the struct instead of its tool mapFixes #213
A [=model context=] struct has exactly one item,
tool map, so indexing the struct itself by tool name is not well-defined. Every other algorithm reads throughinternal context'stool map— seeunregister a tool,getTools(), andperform an observation.perform an observationhas an inverted assertionFixes #214
It asserts the traversable's active document is not fully active. A top-level traversable's active document is always fully active, and the algorithm then goes on to read that document's descendants, which only makes sense when it is.
registerTool(),getTools(), andexecuteTool()all reject when a document is not fully active.notify documents of a tool changeskips the registering documentFixes #215
Using
descendant navigablesexcludes the tool owner's own navigable, so the document that registered the tool never receivestoolchange. This contradicts the worked example immediately below the algorithm, which asserts thatParent toolchangealways logs beforeChild toolchangefor aregisterTool()call on the top-level document. The two comparable algorithms — ingetTools()andperform an observation— both useinclusive descendant navigables.Editorial
Removes a stray possessive in
perform an observation(|descendant|'s active document's.), matching the identical assignments innotify documents of a tool changeandgetTools().All three normative issues were filed by @Aravindargutus. No new link targets are introduced — every construct used already appears elsewhere in
index.bs.Preview | Diff