Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add/Update Action Context variables to main Context for Stepwise Planner #1893

Closed
wants to merge 2 commits into from

Conversation

akshaykokane
Copy link
Contributor

@akshaykokane akshaykokane commented Jul 7, 2023

Add ActionContext variable to main Context

Motivation and Context

Please help reviewers and future users, providing the following information:

  1. Why is this change required?
    In stepwise planner, for every action we create new Action Context. Each action can result into adding new context variables. These context variables are potentially used for executing of next action. Currently, the context variables created from Action are ignored and only the "Result" is added back to context. So the action only adds Result to SKContext and doesn't update the context variables in Kernel context.

  2. What problem does it solve?

  • Chaining of multiple skills using Stepwise Planner and Context Variables (inline with SequentialPlanner)
  • Before implementing Action, reads from main context to check if variable is available if not try to get it from {actionVariables}
  1. What scenario does it contribute to?
    Stepwise Planner uses Context Variables from previous actions to invoke next action

  2. If it fixes an open issue, please link to the issue here. Stepwise Planner doesn't updates Context Variables from actions #1894

Description

Contribution Checklist

Add ActionContext variable to main Context
@akshaykokane akshaykokane requested a review from a team as a code owner July 7, 2023 15:06
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel labels Jul 7, 2023
@akshaykokane akshaykokane changed the title Add Action Context variable to main Context Add Action Context variable to main Context for Stepwise Planner Jul 7, 2023
@akshaykokane akshaykokane changed the title Add Action Context variable to main Context for Stepwise Planner Add/Update Action Context variables to main Context for Stepwise Planner Jul 7, 2023
@shawncal
Copy link
Member

shawncal commented Aug 1, 2023

@akshaykokane Thanks for the contribution! Looking through this, even after a rebase, I see some use of classes that no longer exist (i.e TrustAwareString) which only means it look us too long to get to this PR. I'm going to close it, only because it's clear that the solution is no longer applicable. That said, the Issue #1894 remains open -- if you can confirm the issue is still... an issue, feel free to re-open the PR with an updated solution, and add me (@shawncal and Roger @RogerBarreto) as reviewers.

Thanks! And apologies for the delay in reviewing this. Your contributions are appreciated, and we'd love to see more!

@shawncal shawncal closed this Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants