Skip to content

Sonalmahajan15/add guard rail to backprop#4

Open
lizard-boy wants to merge 6 commits into
mainfrom
sonalmahajan15/add-guard-rail-to-backprop
Open

Sonalmahajan15/add guard rail to backprop#4
lizard-boy wants to merge 6 commits into
mainfrom
sonalmahajan15/add-guard-rail-to-backprop

Conversation

@lizard-boy

Copy link
Copy Markdown

No description provided.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Summary

This pull request refactors the backpropagation logic to add guard rails and extra fixpoint tracking, while updating tests to use a centralized comment extraction utility.

• In assertion/function/analyzer.go, analyzeFunc now discards the extra round count returns while still wrapping errors with context.
• In assertion/anonymousfunc/analyzer_test.go, tests were updated to use nilawaytest.FindExpectedValues for closure extraction.
• In assertion/function/assertiontree/backprop.go, BackpropAcrossFunc returns roundCount and stableRoundCount, improving fixpoint diagnostics.
• In assertion/function/assertiontree/parse_expr_producer.go, the index branch now enforces stability using isIndexTrackable checks.
• In assertion/function/assertiontree/root_assertion_node.go, guard rails were added to filter duplicate nodes in LandAtPath.

8 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +180 to +182
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Using 'defer cancel()' inside the loop may delay context release; consider explicit cancel() after usage to reduce resource retention in long loops.

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