Skip to content

Detect one node_id carrying several node_types across records (#356) - #366

Merged
realmarcin merged 3 commits into
mainfrom
feat/356-inconsistent-node-type
Aug 13, 2026
Merged

Detect one node_id carrying several node_types across records (#356)#366
realmarcin merged 3 commits into
mainfrom
feat/356-inconsistent-node-type

Conversation

@realmarcin

Copy link
Copy Markdown
Contributor

Detection for #356. Baselined at 294 occurrences across 63 node_ids. No trait data changed — the burn-down is a separate campaign.

The only cross-record check here

Every other check in audit_causal_graphs.py is scoped to one graph. This defect isn't: two records disagree about what a node is, and neither is wrong read alone. That's why nothing caught it.

The issue understates the scale by ~7×

#356 reports proton_motive_force typed four ways across 9 records. Measured:

node_type records
STATE 18
BIOLOGICAL_PROCESS 13
CHEMICAL 2
CAPACITY 2

35 records, one concept — and 63 node_ids disagree with themselves corpus-wide, membrane_potential also at four types.

Why it stopped being cosmetic

#355 minted powers (METPO:2007900) gated to subject_types = BIOLOGICAL_PROCESS|STATE. Two byte-identical assertions now behave differently purely by subject typing:

physiology/carboxydotrophic.yaml   proton_motive_force (STATE)     -> atp_synthase   grounds
physiology/phototrophic.yaml       proton_motive_force (CAPACITY)  -> atp_synthase   blocked_by_node_type

Typing decides groundings now. A disagreement is no longer just untidy.

Two design decisions worth reviewing

Reported per occurrence, not on a presumed-wrong minority. Nothing here knows which type is right — proton_motive_force splits 18/13, and the gradient genuinely is a state while generating it is a process. The majority is an observation, not a verdict. Per-occurrence rows also mean a family clears together the moment it's normalised.

The detail leads with node_id, deliberately not with the type set. _key takes the leading token as discriminator, so leading with the types would re-key every row of a family each time one member is fixed — un-suppressing rows nobody has reached yet, a burn-down that fights itself. That's FRAGMENTED_GRAPH's documented failure from the other direction, and there's a test pinning it.

Not every hit is a defect

terminal electron acceptor is deliberately both CHEMICAL and MOLECULAR_FUNCTIONmappings/node_grounding.tsv carries a row for each, noting one METPO class covers both senses and the MOLECULAR_FUNCTION typing "surfaces the role-of interpretation". Same two-senses shape as reduces (#330/#333) and the CAPACITY table.

So the playbook gets a decision table: does this id mean one thing? — normalise / decide-and-record / split into two node_ids. Where a family really means two things, the fix is two ids, not one type. It also carries #352's lesson forward, since the burn-down will need it: the test is not "is this type defensible in isolation" but "is it compatible with what the record and its predicates already assert".

just qc green · 534 tests pass (4 new) · ruff clean · history record per #325.

Stacked note: #365 (backlog reconcile) is open and its section 11 describes this work; no file overlap.

Every other check in this file is scoped to one graph. This defect is not:
two RECORDS disagree about what a node is, and neither is wrong read alone —
which is exactly why nothing caught it.

Adds node_type_index() and an INCONSISTENT_NODE_TYPE finding, baselined at
294 occurrences across 63 node_ids.

THE ISSUE UNDERSTATES THE SCALE BY ~7x. It reports proton_motive_force typed
four ways across 9 records. Measured, it is 35:

    STATE               18
    BIOLOGICAL_PROCESS  13
    CHEMICAL             2
    CAPACITY             2

and 63 node_ids disagree with themselves corpus-wide, membrane_potential also
at four types.

It stopped being cosmetic when #355 minted `powers` (METPO:2007900) gated to
subject_types = BIOLOGICAL_PROCESS|STATE. Two byte-identical assertions now
behave differently purely by subject typing — carboxydotrophic.yaml's
proton_motive_force (STATE) grounds, phototrophic.yaml's (CAPACITY) is
blocked_by_node_type. Typing decides groundings now.

REPORTED PER OCCURRENCE, not on a presumed-wrong minority: nothing here knows
which type is right. proton_motive_force splits 18 STATE / 13
BIOLOGICAL_PROCESS, and the gradient genuinely IS a state while generating it
IS a process — the majority is an observation, not a verdict. Per-occurrence
rows also mean a family clears together the moment it is normalised.

The detail leads with node_id, so `_key` discriminates by node within a graph.
Deliberately NOT led with the type set: a family part-way through a burn-down
would re-key on every step and un-suppress rows nobody has reached yet, which
is FRAGMENTED_GRAPH's documented failure from the other direction. Pinned by a
test.

NOT EVERY HIT IS A DEFECT, and the baseline is where that gets decided.
`terminal electron acceptor` is deliberately both CHEMICAL and
MOLECULAR_FUNCTION — node_grounding.tsv carries a row for each, saying one
METPO class covers both senses and the MOLECULAR_FUNCTION typing "surfaces the
role-of interpretation". Same two-senses shape as `reduces` (#330/#333). Where
a family really does mean two things the fix is TWO node_ids, not one type;
written into the playbook as a decision table.

Detection only — no trait data changed. The burn-down is the next step, and
the playbook carries #352's lesson for it: the test is not "is this type
defensible in isolation" but "is it compatible with what the record and its
predicates already assert".

534 tests pass (4 new) - ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@culturebot-reviewer

culturebot-reviewer Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

From review of this PR. node_type_index() was documented as counting "records"
and counts node OCCURRENCES.

The two are equal everywhere in the corpus today — checked rather than assumed:
no node_id appears twice in one graph, and none appears in two graphs of one
record (0 and 0). So the docstring was accidentally true, by coincidence rather
than construction, and nothing forbids the shape that breaks it.

It matters because the count is quoted straight into the finding text —
"also STATE×18 elsewhere in the corpus" — whose whole job is to size the
disagreement. A record with one node_id in two graphs would silently inflate it.

Docstring now says occurrences and says why the distinction is currently
invisible. The old test could not see it either (one graph per record, so both
readings pass), so there is now a fixture with one node_id in two graphs of one
record asserting {"STATE": 2} — one record, two occurrences.

47 tests in this file, 535 total.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@culturebot-reviewer

culturebot-reviewer Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

realmarcin added a commit that referenced this pull request Aug 13, 2026
From review of this PR against #366.

The file recommended #356 as next and described its first step — "Detect. Add
an INCONSISTENT_NODE_TYPE check … Baseline it" — as pending. #366 ships exactly
that. Merging both would make the reconcile false on arrival: recommending as
"next" a step already taken, which is precisely the drift a reconcile exists to
remove.

Step 1 is now marked DONE (2026-08-13, PR #366) with what it actually decided
(per-occurrence rows, keyed on node_id not the type set, and why), and the
recommendation moves to step 2 — decide a type per family and normalise —
which is the part still open, and the harder part.

Also records #373 as what #366 left behind (third corpus walk, 7.9s → 12.4s),
and adds a section 7 note that automatic claude-review is off (#371), including
why the trigger was removed from the file rather than toggled in the Actions tab
(#372).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
realmarcin added a commit that referenced this pull request Aug 13, 2026
* Reconcile NEXT_TASKS.md (2026-08-08)

41 PRs merged since the 2026-08-05 reconcile — the file said "everything merged
through #272" and main is at #363.

Seven of the seventeen listed issues are closed (#198, #217, #248, #252, #270,
#275, #283); five are new (#289, #292, #356, #358, #364). Header rewritten to
say what the 41 PRs actually were, which is one thread rather than a list: the
corpus disagreeing with itself about types and predicates, run as
detect -> propose -> migrate -> burn down, three times.

Section 5's measurement was from 2026-07-30 and predates #294/#300/#351/#360.
Re-measured: 218 graphs still split, 1296 UNREACHABLE_FROM_TRAIT — barely moved
in five weeks. Added the number that should replace it now that #363 exists:
353 graphs, 861 components over 4129 wired nodes, 69.7% attached.

New section 11 for #356/#364, and it is the recommended next item. MEASURED
RATHER THAN RELAYED, and the issue understates it by 7x: proton_motive_force
carries four node_types across 35 records, not 9, and 63 node_ids carry more
than one node_type corpus-wide. Recorded the shape of the fix and the trap —
the test is not "is this type defensible in isolation" but "is it compatible
with what the record and its predicates already assert", which #360 took three
review rounds to learn.

Section 7 records #358's ordering constraint where someone will find it:
../culturebotai-claw is checked out locally so the canonical fix is actionable
today, but adding history.yaml to check_vendored_sync.sh still waits on #209.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Do not recommend a step that has already been taken (#375)

From review of this PR against #366.

The file recommended #356 as next and described its first step — "Detect. Add
an INCONSISTENT_NODE_TYPE check … Baseline it" — as pending. #366 ships exactly
that. Merging both would make the reconcile false on arrival: recommending as
"next" a step already taken, which is precisely the drift a reconcile exists to
remove.

Step 1 is now marked DONE (2026-08-13, PR #366) with what it actually decided
(per-occurrence rows, keyed on node_id not the type set, and why), and the
recommendation moves to step 2 — decide a type per family and normalise —
which is the part still open, and the harder part.

Also records #373 as what #366 left behind (third corpus walk, 7.9s → 12.4s),
and adds a section 7 note that automatic claude-review is off (#371), including
why the trigger was removed from the file rather than toggled in the Actions tab
(#372).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@realmarcin
realmarcin merged commit 74c55a9 into main Aug 13, 2026
5 checks passed
@realmarcin
realmarcin deleted the feat/356-inconsistent-node-type branch August 13, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant