Skip to content

Update the stale cycle-detection comment on topological_sort - #74079

Merged
pierrejeambrun merged 1 commit into
jason/lang-sdk-e2e/03-native-dag-parsefrom
pierrejeambrun/lang-sdk-e2e/cycle-comment-fix
Oct 2, 2026
Merged

pierrejeambrun merged 1 commit into
jason/lang-sdk-e2e/03-native-dag-parsefrom
pierrejeambrun/lang-sdk-e2e/cycle-comment-fix

Conversation

@pierrejeambrun

Copy link
Copy Markdown
Member

Stack: #74034, #74040, #74041, #74042, #74035, this PR

Why

SerializedTaskGroup.topological_sort's docstring says DAG.check_cycle is what keeps a cyclic Dag from ever reaching it. That was true, but only for a Python-authored Dag — a Lang-SDK Dag has no Python DAG object to call check_cycle on, so the comment overstated the actual guarantee for the other authoring path.

#74035 closes that gap: the Dag processor now runs DagSerialization.validate_serialized_dag on every Dag a Lang-SDK runtime returns, rejecting one with a cycle as an import error before it is ever stored (both this and DAG.check_cycle are backed by the shared detect_cycle from #74034). The comment should name both guards.

What

One docstring update, no behavior change.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

…ogical_sort

DAG.check_cycle was the only guard this comment named, but a Lang-SDK Dag
never runs it: it has no Python DAG object to call it on. The Dag processor
now rejects a cyclic Lang-SDK Dag of its own accord, through
DagSerialization.validate_serialized_dag, so the comment should name both
guards rather than the one that does not cover every author.

@jason810496 jason810496 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.

Thanks.

@pierrejeambrun
pierrejeambrun merged commit ea57472 into jason/lang-sdk-e2e/03-native-dag-parse Oct 2, 2026
2 checks passed
@pierrejeambrun
pierrejeambrun deleted the pierrejeambrun/lang-sdk-e2e/cycle-comment-fix branch October 2, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants