Constrain the CultivationSetup unit slots, and make a new enum declare itself (#514, #518) - #517
Open
realmarcin wants to merge 2 commits into
Open
Constrain the CultivationSetup unit slots, and make a new enum declare itself (#514, #518)#517realmarcin wants to merge 2 commits into
realmarcin wants to merge 2 commits into
Conversation
Six slots carry a unit or a kind and none had a range, so linkml-validate accepted any string. Six enums now: TimeUnitEnum, VolumeUnitEnum, RateUnitEnum, TemperatureUnitEnum, PotentialUnitEnum, RetentionTimeTypeEnum. Symbols, not SCREAMING_CASE. A unit is not a category — °C and 1/h are what every source and reader already writes, and PER_HOUR has to be translated back before it means anything. The cost is that the values are not Python identifiers, so LinkML emits them through _addvals/setattr; the test checks the runtime enum rather than dir(), which is the check that would otherwise pass while a value was silently absent. The gate found two things on its first run. `retention_time_type: HYDRAULIC`, which I wrote in #512 against a slot whose description has always said "HRT" or "SRT" — a description is not a constraint. And the repo's own smoke test used `h^-1`, a third spelling of per-hour alongside `1/h` and the `PER_HOUR` I wrote in #515. One thing is deliberately narrowed rather than preserved: applied_potential_unit was documented as taking "mV vs SHE", making the value a unit and a reference electrode at once — not joinable across records and not convertible. The enum is {V, mV} and the reference moves to electrode_detail, where it can be stated in full. The smoke test exercised the old shape, so this changes what the schema allows; called out in the PR since it is a modelling decision, not a fix. Mutation-checked: removing the six ranges turns 7 of the 11 tests red, including the one that shells out to the real validator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Network integrity findingsWarnings only — a member with no interaction yet, or a participant matched by ontology id rather than by name, or one on a community-level interaction that resolves to no member. Reported, but does not fail the build. The full report is attached to the workflow run as an artifact. |
vocab/cultivation_terms.yaml states an invariant in its own header — the keys
under each enum must equal that enum's permissible values — and enforcement was
`ENUMS = ("CultivationModeEnum", "CultivationSystemEnum")`, a hardcoded tuple
with no completeness check. The invariant was a property of two names someone
typed, not of the file.
Adding six enums in this PR is the case that exposed it: none went red. That is
the right outcome, since they are units and would map to UO or UCUM rather than
METPO — but the repo could not tell a deliberate exemption from an oversight.
Same shape as #471, where nine tests stayed green because they pinned a
constant's value and never its use.
Every schema enum is now either staged or exempt with a written reason, and the
five that are genuine ontology candidates say "not yet staged" rather than
being quietly lumped in with bookkeeping. Reasons are asserted non-empty so the
dictionary cannot decay into an allow-list.
Mutation-checked: adding a `FakeNewEnum` to the schema turns it red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
realmarcin
changed the title
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 #515 (base
enrich-cultivation-183g), because it corrects a value that PR introduces. Review #515 first.What (#514)
Six
CultivationSetupslots carry a unit or a kind —working_volume_unit,operating_temperature_unit,feed_or_dilution_rate_unit,retention_time_unit,applied_potential_unit,retention_time_type— and none had a range, solinkml-validateaccepted any string. Six enums now constrain them.Symbols, not SCREAMING_CASE. A unit is not a category.
°Cand1/hare what every source and reader already writes, and the corpus was already using symbols (h,d,L/day,mL,mV). A slot that saysPER_HOURhas to be translated back before it means anything.The cost: these aren't valid Python identifiers. LinkML emits them via an
_addvals/setattrclassmethod rather than as class attributes, sodir(), a static import, and_defn.permissible_valuesall miss them. I checked this before building the rest — my first runtime assertion reported°Cas absent when it was present. The test now queries the runtime enum the way that actually works.What the gate caught on its first run
retention_time_type: HYDRAULIC— mine, from Record the bioreactor setup #183 could not reach without full text #512, against a slot whose description has said"HRT" or "SRT"since it was written. A description is not a constraint. NowHRT.h^-1in the repo's own smoke-test fixture — a third spelling of per-hour, alongside1/hand thePER_HOURI wrote in Two more cultivation setups, and stop the unit strings diverging (#183) #515. Now1/h.applied_potential_unitwas documented as taking"mV vs SHE", and the smoke test pinned that shape. That makes one value both a unit and a reference electrode: not joinable across records, not convertible, and not a unit.The enum is
{V, mV}; the reference electrode moves toelectrode_detail, where it can be stated in full rather than compressed into a suffix. No curated record used the old shape, so nothing real is lost — but this removes something the schema allowed and a test asserted. Reversible by addingmV vs SHE/V vs SHEas permissible values if you'd rather keep it.What review then found (#518)
vocab/cultivation_terms.yamlstates an invariant in its own header — vocab keys must equal enum permissible values — and enforcement was:A hardcoded tuple with no completeness check. Adding six enums here is the case that exposed it: none of them went red. That's the right outcome — units belong to UO/UCUM, not METPO — but the repo could not distinguish "deliberately exempt" from "nobody noticed". Same shape as #471, where nine tests stayed green because they pinned a constant's value and never its use.
Every one of the schema's 26 enums is now either staged or exempt with a written reason. The five that are genuine ontology candidates (
FunctionalRoleEnum,InteractionTypeEnum,AtmosphereEnum,MediaRelationshipEnum,EcologicalStateEnum) say "candidate, not yet staged" rather than being lumped in with bookkeeping — the exemption is "not done", not "not applicable". Reasons are asserted non-empty so the dictionary can't decay into an allow-list.Checks
just validate-all— exit 0 across the corpusjust lint,just validate-strict— exit 0uv run pytest tests/— 2387 passed, 16 skippedrange:lines turns 7 of 11 new tests red, includingtest_a_wrong_unit_is_actually_rejected, which shells out to the real validator and requires a non-zero exit onoperating_temperature_unit: CELSIUS. Adding aFakeNewEnumto the schema turns the The vocab-sync invariant applies to a hardcoded list of two enums and cannot notice a third #518 guard red. Both verified by reverting, running, and restoring.Closes #514, closes #518.
🤖 Generated with Claude Code