Repository navigation
fix: report what a controlled instance lookup cannot resolve - #188
Merged
ehennestad merged 9 commits intoSep 11, 2026
Merged
ehennestad merged 9 commits into
ehennestad merged 9 commits into
Conversation
ehennestad
added this pull request to stack #189
September 10, 2026 20:06
ehennestad
changed the title
ehennestad
force-pushed
the
report-unresolved-controlled-instances
branch
from
September 11, 2026 02:45
7d76012 to
8155c91
Compare
Contributor
Contributor
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## resolve-instance-types-from-at-type #188 +/- ##
=======================================================================
+ Coverage 80.09% 80.19% +0.09%
=======================================================================
Files 423 423
Lines 4381 4387 +6
=======================================================================
+ Hits 3509 3518 +9
+ Misses 872 869 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ehennestad
force-pushed
the
report-unresolved-controlled-instances
branch
3 times, most recently
from
September 11, 2026 10:28
baee63a to
2891207
Compare
…hing A name that matches no controlled instance is let through on purpose, so that a term a user defined survives being written out and read back. But the warning that said so carried no identifier, so a caller holding only an IRI could not tell a term that resolved from one that did not, other than by parsing the message. instanceFromIRI hands back exactly such a term, with a freshly minted blank node id in place of the IRI it was given. The warning is now openMINDS:ControlledTerm:UnknownInstanceName, so it can be checked for, switched off, or stopped on. What is let through is unchanged. Closes #183. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The warning carried no identifier, and did not name the versions it had found or say which one it went on to use. Nothing downstream works with two versions of a type class on the path, so the message now names them, says the first is used, and says how to select one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
getControlledInstance built the path to an instance file from the type name. For the core module it pluralized with "s", which produces "accessibilitys" for a type stored under "accessibilities" and a 404 from GitHub; for sands it named a folder, "graphStructures", that no version of the instance library has. Folder names are pluralized type names that upstream renames, and no rule builds them from a type. Neither branch was reachable from the toolbox: the one caller passes "controlledTerms", and controlled terms are the one module stored under a fixed folder name. The function now accepts only that module and builds only that path. Instances of other modules are found through the InstanceLibrary, which reads the type each document declares. Closes #186. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
downloadRepository has checked the recorded commit before downloading since the note was written. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…exist yet getSingleton required its folder argument to be a folder, and so did the constructor. On a fresh installation the default location does not exist yet, so the validator rejected the call before the constructor could run the download that exists for exactly that case. It only worked because setup.m downloads the library separately, which an installed toolbox never runs: a user of the .mltbx whose first call touched controlled instances got a validation error instead of a download. The default location may now be missing; setting it downloads the library into it. A location a caller names still has to exist, because nothing is downloaded anywhere else, and that is checked before the library in use is touched, so a location that names nothing leaves a working library alone. The rejection carries an identifier and says where the library is downloaded to. Closes #185. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ne value Review of the fixes on this branch. Two comments explained the history of what they sat next to rather than what the code does. The one on the unknown-name warning justified the warning's identifier, which needs no justification; it now says why the condition is a warning and not an error. The one on the default library location in getSingleton is reduced to what the check is: only the default location is downloaded into, so any other has to exist already. That check also names the path utility the base branch moved the resolver to. getControlledInstance kept a module argument that could take one value, threaded through every local function and ignored at the end. It is gone from the signature and the local functions, and the one caller and the tests no longer pass it. What the function serves, and why only that, is now its help text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The warning said that only "the first" is used and to "select one", without saying first of what or one of what. It now says what the conflict is, that types have the same names in every version, and what to do: put one model version, and only that one, on the search path. The comment on the test for the unknown-name warning explained the warning's identifier at length. It now says what the test checks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
What it returns, when the library is created and downloaded, what a location other than the default one has to satisfy, what Reset and UseGit do, and when the handle it returns stops being the one in use. UseGit is described as it behaves: it leaves retrieving the library to the caller, since the pull it is named for is not implemented. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A location a caller named only had to exist. A folder with nothing in it passed, and what followed was worse than the error the check prevents: the library in use was deleted, the empty folder was read as a library with no versions, and nothing said so. A working library of seventeen thousand instances became one of none, silently, and the caller's handle to it was gone. The check now asks what the class asks of any folder it reads, that it hold at least one version folder, and both use the one definition. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ehennestad
force-pushed
the
report-unresolved-controlled-instances
branch
from
September 11, 2026 12:01
2891207 to
24a3d6e
Compare
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
1 participant
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.
Stacked on #184. Four small fixes to how the toolbox reports a controlled instance it cannot resolve, and one to how it gets hold of the library in the first place. Each is its own commit.
Why
#184 made the instance library read the type each instance declares and follow the model version. What was left were places that still could not say what had gone wrong, and one place that could not get started at all.
What
A name that matches no controlled instance is reported with an identifier (#183). The warning is deliberately permissive, so that a term a user defined survives being written out and read back, but it carried no identifier. A caller of
instanceFromIRI, which hands back exactly such a term with a fresh blank node id in place of the IRI it was given, could not tell a term that resolved from one that did not without parsing the message. It is nowopenMINDS:ControlledTerm:UnknownInstanceName. What is let through is unchanged.A missing default library location downloads instead of erroring (#185).
getSingletonand the constructor both required the folder to exist, so on a fresh installation the validator rejected the call before the constructor could run the download that exists for that case. It only worked becausesetup.mdownloads separately, which an installed toolbox never runs. The default location may now be missing. A location a caller names still has to exist, since nothing is downloaded anywhere else, and that is checked before the library in use is touched, so a location that names nothing leaves a working library alone.getControlledInstanceno longer builds folder names it cannot know (#186). For the core module it pluralized with"s", which yieldsaccessibilitysand a 404; for sands it named a folder no version of the library has. Neither branch was reachable from the toolbox: the one caller passes"controlledTerms", the one module stored under a fixed folder name. The function no longer takes a module at all, and builds only that path; everything else is found through theInstanceLibrary.More than one model version on the path is reported with an identifier, naming the versions found and which one is used.
A stale note in
downloadControlledInstancesabout checking the recorded commit is dropped;downloadRepositoryhas done that since it was written.Tests
ControlledTermTestchecks the unknown name warning by identifier.InstanceLibraryTestchecks that a location that does not exist is rejected by name and that the library already in use survives the rejection. That a missing default location downloads was checked against a fresh user folder with nothing under it: the library constructed and read 17,427 instances where it previously failed validation. It is not a unit test, since it downloads the repository.Closes #183, #185, #186.
🤖 Generated with Claude Code