Skip to content

Move some SM hooks from render_context to SM actions#217

Merged
NejcS merged 1 commit into
mainfrom
impr/state-machine-and-context-cleanup
Jun 30, 2026
Merged

Move some SM hooks from render_context to SM actions#217
NejcS merged 1 commit into
mainfrom
impr/state-machine-and-context-cleanup

Conversation

@NejcS

@NejcS NejcS commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This moves some code from on_enter/on_exit hooks to the relevant actions.

The added value of these changes is:

  • to have less code in the RenderContext
  • to have the associated code together
  • to get rid of machine.dispatch(...) calls in order to get the full picture of the SM just from its config

This PR doesn't get rid of all of the dispatches in the RenderContext, the main chunk is in the start_conformance_tests_for_frid function and its delegated functions but I will try to do that at a later time in a separate PR.

@NejcS NejcS self-assigned this Jun 11, 2026
@pedjaradenkovic pedjaradenkovic requested a review from VitjanZ June 11, 2026 10:24
if ctx.conformance_tests_render_attempts >= MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS:
error_msg = f"The renderer was unable to produce an implementation that passes conformance tests for functionality '{render_context.frid_context.frid}' after many attempts. Please review and rewrite the specification. (Render ID: {render_context.run_state.render_id})"
render_context.last_error_message = error_msg
return self.LIMIT_EXCEEDED_OUTCOME, None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wherever we return SOME_STATE, None and this triggers ExitWithError, then None may be printed for no reason. The first line of ExitWithError is console.error(previous_action_payload) which in this case is console.error(None) which prints "None".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point.

I improved that: 367c772

@VitjanZ VitjanZ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM and passes benchmarks. I left a minor comment regarding error logging.

@NejcS NejcS force-pushed the impr/state-machine-and-context-cleanup branch from b263189 to 8a84195 Compare June 29, 2026 09:06
@NejcS NejcS force-pushed the impr/state-machine-and-context-cleanup branch from 367c772 to ef798af Compare June 30, 2026 07:13
@NejcS NejcS merged commit 6c0af01 into main Jun 30, 2026
10 checks passed
@NejcS NejcS deleted the impr/state-machine-and-context-cleanup branch June 30, 2026 07:15
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