Skip to content

Include synthetic_key in TargetRelations equality and hashing - #271

Merged
hbarthels merged 1 commit into
mainfrom
hb-fix-target-relations-equality
Jul 27, 2026
Merged

Include synthetic_key in TargetRelations equality and hashing#271
hbarthels merged 1 commit into
mainfrom
hb-fix-target-relations-equality

Conversation

@hbarthels

@hbarthels hbarthels commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

The hand-written Julia equality for TargetRelations in sdks/julia/LogicalQueryProtocol.jl/src/equality.jl compares only keys and body. It was never updated when the synthetic_key field landed in #269.

As a result, two TargetRelations that differ only in whether the shared key is synthetic compared equal and hashed identically — so they collided in Dict/Set keys, and == reported a false positive.

Changes

  • Add synthetic_key to ==, isequal, and hash for TargetRelations.
  • Add a regression test asserting that a synthetic-key value is unequal to an otherwise-identical explicit-keys value.

Verification

The new @test s1 != g3 assertion fails against the previous implementation and passes with the fix. Full Julia suite green (174 test items).

Split out of the load_errors work so the fix can land on its own; #272 is stacked on this branch.

🤖 Generated with Claude Code

The hand-written Julia equality for `TargetRelations` compared only `keys`
and `body`, so it never picked up the `synthetic_key` field added in #269.
Two `TargetRelations` differing only in whether the shared key is synthetic
compared equal and hashed alike — meaning they collided in dictionaries and
sets, and `==` reported a false positive.

Add `synthetic_key` to `==`, `isequal`, and `hash`, with a regression test
that fails against the previous implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hbarthels
hbarthels merged commit 0cea2af into main Jul 27, 2026
5 checks passed
@hbarthels
hbarthels deleted the hb-fix-target-relations-equality branch July 27, 2026 16:36
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.

2 participants