Merge main into #185 and resolve conflicts - #206
Merged
omri374 merged 14 commits intoSep 10, 2026
Merged
Conversation
* feat: branch-level aliases (LOC->LOCATION, ORG->ORGANIZATION)
Non-leaf hierarchy nodes can now declare raw aliases via a reserved
`_aliases` key, mirroring the alias lists leaf nodes already have. The key
is skipped by every tree-walk so it never becomes a canonical entity, and
`add_alias()` on a branch records the alias instead of creating a spurious
child leaf.
Moves the coarse labels LOC/ORG onto the LOCATION/ORGANIZATION branches, so
they canonicalize to the readable branch name and match a model's
LOCATION/ORGANIZATION at the exact (leaf) level, not only at branch level.
This is what lets coarse corpora such as TAB be scored at the exact level
against models that emit the long-form labels.
Because LOC/ORG stop being canonical entities, several lookups keyed on
canonical names needed to learn about aliases:
- `to_branch()` previously returned raw aliases unchanged, so `to_branch("LOC")`
would have silently degraded from "LOCATION" to "LOC" -- projecting into the
wrong branch rather than failing. It now resolves aliases first. Same for
`get_depth()`, which would otherwise raise.
- `add_alias()` accepts an alias as its subject, so add_alias("LOC", ...) works.
- `_find_node()` no longer matches the reserved `_aliases` key, which would
otherwise let `add_alias("_aliases", x)` attach x to whichever branch the
tree walk happened to reach first.
- `add_alias()` raises ValueError instead of silently persisting an alias that
a descendant already claims (branch aliases are applied before the descent
into their own subtree, so a descendant wins). The hierarchy is rolled back.
Updates two docs examples that used map({'LOCATION': 'LOC'}) as a resolution
target; LOC is no longer a valid target, and the mapping is no longer needed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5ceeaa56-7275-46b5-9f57-12e9bb3c20e3
* Address PR #186 review comments
- add_alias: rollback no longer deletes an alias the target already owned.
Appending is now tracked with an `appended` flag, so a failed call leaves
the hierarchy exactly as it found it (negruber1).
- add_alias: remove the duplicated, unreachable rollback+raise block
(negruber1, Copilot).
- definitions: PER moves from an empty leaf under PERSON > NAME to a
branch alias of PERSON, matching LOC/ORG (omri374).
- hierarchy: warn at construction time when a branch alias declared in the
hierarchy definition is shadowed by one of its own descendants. The
add_alias() guard only covered the runtime path (omri374).
- get_depth: error message said "Canonical entity" even for raw aliases.
- docs/entity_hierarchy.md: new "Aliases" section covering leaf vs branch
aliases, add_alias(), the collision ValueError, and alias resolution in
canonicalize/to_branch/get_depth (omri374).
- docs: replace the stale COLLISION_AMBIGUOUS / COLLISION_TRIVIAL issue
types with the actual COLLISION_CROSS_BRANCH / COLLISION_SAME_BRANCH
(Copilot). ADR-003 left as-is; it records a decision at a point in time.
- CHANGELOG: add PER to the breaking-change list.
728 passed, 2 skipped; ruff clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5ceeaa56-7275-46b5-9f57-12e9bb3c20e3
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5ceeaa56-7275-46b5-9f57-12e9bb3c20e3
"PATIENT" was an alias of the PATIENT_ID leaf, i.e. the same leaf as
MEDICALRECORD. In the i2b2/n2c2 2014 de-identification schema, PATIENT and
DOCTOR are both subtypes of the NAME category and MEDICALRECORD is the ID
subtype, so the PATIENT tag marks a person's name ("Yosef Villegas"), not an
identifier.
The hierarchy already treated the neighbouring labels correctly - DOCTOR,
DOCTOR_NAME, PATIENT_NAME and HCW all resolve to NAME - which made bare
PATIENT the odd one out:
PATIENT -> PATIENT_ID (branch PHI) <- wrong
PATIENT_NAME -> NAME (branch PERSON)
DOCTOR -> NAME (branch PERSON)
Because the two disagree at branch level as well as leaf level, a model that
correctly labels a patient name as a NAME was scored wrong at every level, and
a model that reproduced the mislabel was scored right. On a 20-record n2c2
sample this affected 32 of 328 gold spans (~10%).
Moves "PATIENT" to FULL_NAME alongside DOCTOR and PATIENT_NAME. The
record-number aliases (MEDICALRECORD, MEDICAL_RECORD) stay on PATIENT_ID.
Noticed while writing the test, not fixed here: MEDICAL_RECORD_NUMBER is listed
as an alias of both PATIENT_ID and MRN, and MRN currently wins. The new test
asserts that label at branch level only, so it does not silently encode which
leaf shadows the other.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5ceeaa56-7275-46b5-9f57-12e9bb3c20e3
WikiData's API now rejects unauthenticated requests with a 403 and a message asking clients to set a User-Agent per its robot policy. This made HospitalProvider silently fall back to its default hospital list whenever load_wiki_hospitals() ran. Also corrects the log message on the fallback path: it previously claimed to be "returning an empty list" when it actually returns self.default_list. Fixes #173
Removed Python version 3.10 from CI workflow matrix.
Resolve conflicts with the 0.3.2 release and the hierarchy projection work: - CHANGELOG: 0.3.2 shipped after this branch was written, so the two-sided counting entries move from the 0.3.2 section to Unreleased > Behavior Changes, next to the deepest-annotated-ancestor projection entry. - span_evaluator.calculate_score_on_df: keep the allow_generic_entities parameter docstring from main inside this branch's typed docstring. - test_low_iou_descendants_use_the_projected_type: two NAME spans that jointly fail to cover a PERSON annotation now count one false positive per span (num_predicted 2, FP 2), as documented for two-sided counting, while still being attributed to the projected PERSON label. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMHDXc7d43rZLiRrunemYY
omri374
changed the title
omri374
changed the base branch from
main
to
fix/span-evaluator-two-sided-counting
September 10, 2026 06:38
omri374
merged commit Sep 10, 2026
fed4469
into
fix/span-evaluator-two-sided-counting
7 checks passed
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
4 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.
Summary
Merges
maininto the #185 branch and resolves the conflicts. Merging this PR updates #185 so it becomes mergeable again. No evaluator logic changes beyond what #185 already contains.Conflict resolution
Unreleased > Behavior Changes, next to the deepest-annotated-ancestor projection entry from Credit more-specific hierarchy predictions #196. The 0.3.2 section is left as it is onmain.SpanEvaluator.calculate_score_on_dfdocstring: keeps the Two-sided span counting: precision per prediction span, recall per annotation #185 typed docstring and adds theallow_generic_entitiesparameter description introduced onmain.test_low_iou_descendants_use_the_projected_type(from Credit more-specific hierarchy predictions #196): two NAME spans that jointly fail to cover a PERSON annotation are now one FN plus one false positive per span (num_predicted == 2,false_positives == 2,("O", "PERSON") == 2), matching the two-sided counting rule in Two-sided span counting: precision per prediction span, recall per annotation #185. The test still asserts the counts are attributed to the projected PERSON label and not to NAME.Verification
ruff checkandruff format --checkare clean on the touched files.Known limitation (unchanged from #185, candidate for a follow-up)
Same-type coverage of an annotation pools every overlapping span of that type rather than the best subset. A good single-span match can therefore be dragged below the threshold by a neighbouring same-type span that only brushes the annotation, turning one TP into one FN plus two FPs. Taking the coverage as the maximum of the best pairwise IoU and the combined IoU, and crediting only the spans in the winning selection, would remove this.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SMHDXc7d43rZLiRrunemYY