Skip to content

fix: referencing symbol arguments.(Fixes #670)#671

Merged
skxeve merged 1 commit intomasterfrom
feature/670-bugfix-symbol-argument
Apr 10, 2026
Merged

fix: referencing symbol arguments.(Fixes #670)#671
skxeve merged 1 commit intomasterfrom
feature/670-bugfix-symbol-argument

Conversation

@skxeve
Copy link
Copy Markdown
Collaborator

@skxeve skxeve commented Apr 8, 2026

Brief

Fixes #670

Points to Check

  • Any unexpected impact
  • The overall quality is trending positively

Test

Confirmed

(Write content to confirm / Reason why not necessary)
Pass ALL CI.

Review Limit

  • Write review limit on pull request title.

@skxeve skxeve requested review from Tsuyoposon and x-chi April 8, 2026 11:46
@skxeve skxeve self-assigned this Apr 8, 2026
@skxeve skxeve added the bug Something isn't working label Apr 8, 2026
@x-chi
Copy link
Copy Markdown
Collaborator

x-chi commented Apr 9, 2026

Thank you for implementing this. I would like to check the following:

Potential breaking change regarding dynamic attribute overwriting
I have a concern regarding the practical impact of this change on existing custom steps.

In the previous implementation, symbol resolution seemed to point to the BaseStep instance at runtime. However, this PR changes the logic to resolve arguments during the build phase using the static _step_model_map.

If a user has a custom step that dynamically modifies its own attributes during the execute method (for example, updating a file path or a memo field based on runtime logic), those changes will no longer be reflected in subsequent steps that reference it via symbol. The subsequent steps will instead receive the initial values defined in the scenario file.

I would like to clarify the following points:

Is this change in behavior an intentional "breaking change" to enforce the use of context (put_to_context/get_from_context) for dynamic data passing?

If this is a recommended architectural shift, should we provide a more explicit warning or documentation note for users who might experience silent failures when their dynamic updates are no longer propagated?

@skxeve
Copy link
Copy Markdown
Collaborator Author

skxeve commented Apr 9, 2026

@x-chi
Thanks for the comment. However, I believe there is a misunderstanding.

The behavior you mentioned was changed in the v2.x to v3.x migration, not by this PR. This PR is strictly for fixing the bug in #670.
Also, dynamically overwriting attributes in execute is not intended in cliboa. We do not need to consider it as a breaking change.

Copy link
Copy Markdown
Collaborator

@x-chi x-chi left a comment

Choose a reason for hiding this comment

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

Thank you for implementing it.

There were no issues with the comments.

@skxeve skxeve merged commit 256f48e into master Apr 10, 2026
10 checks passed
@skxeve skxeve deleted the feature/670-bugfix-symbol-argument branch April 10, 2026 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Incorrect default value retrieval in get_symbol_argument

3 participants