Skip to content

Don't report a shared fragment as a fragment cycle - #886

Merged
milessabin merged 1 commit into
typelevel:mainfrom
toddburnside:fix-fragment-diamond-false-cycle
Aug 15, 2026
Merged

Don't report a shared fragment as a fragment cycle#886
milessabin merged 1 commit into
typelevel:mainfrom
toddburnside:fix-fragment-diamond-false-cycle

Conversation

@toddburnside

Copy link
Copy Markdown
Contributor

validateVariablesAndFragments accumulated visited fragments into a single seen set spanning the whole traversal rather than tracking the fragments on the current path. A fragment reachable by more than one path was therefore reported as a cycle, e.g.

fragment a on T { x { ...c } y { ...b } }
fragment b on U { z { ...c } }
fragment c on V { w }

failed with "Fragment cycle starting from 'a'".

Replace the check with a proper DFS: path holds the fragments on the current traversal path and done those already explored in full, a fragment joins done only after its references have been explored, and the path check precedes the done check. Genuine recursion is still detected, and reported against the same fragment as before.

Claude-Session: https://claude.ai/code/session_01Lyt6Er1f7k4DDcNBoZEUGS

`validateVariablesAndFragments` accumulated visited fragments into a single
`seen` set spanning the whole traversal rather than tracking the fragments on
the current path. A fragment reachable by more than one path was therefore
reported as a cycle, e.g.

    fragment a on T { x { ...c } y { ...b } }
    fragment b on U { z { ...c } }
    fragment c on V { w }

failed with "Fragment cycle starting from 'a'".

Replace the check with a proper DFS: `path` holds the fragments on the current
traversal path and `done` those already explored in full, a fragment joins
`done` only after its references have been explored, and the `path` check
precedes the `done` check. Genuine recursion is still detected, and reported
against the same fragment as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lyt6Er1f7k4DDcNBoZEUGS

@tpolecat tpolecat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. 👍

It's a blocking issue for us so it would be good to have this change.

@tpolecat
tpolecat requested a review from milessabin August 14, 2026 17:58

@milessabin milessabin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@milessabin
milessabin merged commit 29b9ddc into typelevel:main Aug 15, 2026
13 checks passed
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.

3 participants