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.

Sign up for GitHub

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

Jump to bottom

[Fix] Modify Plan class to update context variables with input #685

Merged
merged 2 commits into from Apr 27, 2023
Merged

[Fix] Modify Plan class to update context variables with input #685

merged 2 commits into from Apr 27, 2023

Conversation

lemillermicrosoft
Copy link
Member

@lemillermicrosoft lemillermicrosoft commented Apr 27, 2023 •
edited

Motivation and Context

Fix issue with invoking plan with both string input and existing context.

Description

This pull request refactors the Plan class to update the context variables with the input string when invoking a plan. This change allows the plan to use the input as part of the context for executing the plan steps. The pull request also adds a unit test to verify that the plan can execute with a given context and input.

Details:

  • In Plan.cs, change the InvokeAsync method to create a new SKContext with empty variables if the context parameter is null, and then update the context variables with the input string.
  • In PlanTests.cs, add a new test method called CanExecutePlanWithContextAsync that creates a mock context with some variables, memory, skills, and log, and then invokes a plan with the context and an input string. The test asserts that the plan returns the input string as the result, and that the context variables are updated with the input. The test also invokes the plan again with a different input string and the same context, and asserts that the result and the context variables are updated accordingly.

Contribution Checklist

@lemillermicrosoft
🧪Add a test for executing a plan with context
ab46436 
This commit adds a new unit test to the PlanTests class, which verifies that a plan can be executed with a given SKContext object. The test also checks that the plan can accept a different input value than the one provided by the context. The test uses mocks for the IKernel, ILogger, ISemanticTextMemory, and ISkillCollection interfaces.
@lemillermicrosoft
🛠️Fix context initialization in Plan.InvokeAsync
cfe5432 
This commit fixes a bug in the Plan.InvokeAsync method, where the context was initialized with the input as the only variable. This could cause problems if the input was not a valid variable name or if it conflicted with other variables in the context. The fix is to initialize the context with an empty set of variables and then update them with the input. This ensures that the input is properly parsed and validated as a variable assignment.
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel.core labels Apr 27, 2023
@lemillermicrosoft lemillermicrosoft added the PR: ready for review All feedback addressed, ready for reviews label Apr 27, 2023
@lemillermicrosoft lemillermicrosoft added the PR: ready to merge PR has been approved by all reviewers, and is ready to merge. label Apr 27, 2023
@lemillermicrosoft lemillermicrosoft enabled auto-merge (squash) April 27, 2023 00:38
@lemillermicrosoft lemillermicrosoft merged commit f8ab5d8 into microsoft:main Apr 27, 2023
11 checks passed
dluc pushed a commit that referenced this pull request Apr 29, 2023
@lemillermicrosoft @dluc
[Fix] Modify Plan class to update context variables with input (#685)
df7d0f9 
### Motivation and Context
Fix issue with invoking plan with both string input and existing
context.

### Description
This pull request refactors the Plan class to update the context
variables with the input string when invoking a plan. This change allows
the plan to use the input as part of the context for executing the plan
steps. The pull request also adds a unit test to verify that the plan
can execute with a given context and input.

Details:
- In Plan.cs, change the InvokeAsync method to create a new SKContext
with empty variables if the context parameter is null, and then update
the context variables with the input string.
- In PlanTests.cs, add a new test method called
CanExecutePlanWithContextAsync that creates a mock context with some
variables, memory, skills, and log, and then invokes a plan with the
context and an input string. The test asserts that the plan returns the
input string as the result, and that the context variables are updated
with the input. The test also invokes the plan again with a different
input string and the same context, and asserts that the result and the
context variables are updated accordingly.
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
@lemillermicrosoft
[Fix] Modify Plan class to update context variables with input (micro…
68e2a76 
…soft#685)

### Motivation and Context
Fix issue with invoking plan with both string input and existing
context.

### Description
This pull request refactors the Plan class to update the context
variables with the input string when invoking a plan. This change allows
the plan to use the input as part of the context for executing the plan
steps. The pull request also adds a unit test to verify that the plan
can execute with a given context and input.

Details:
- In Plan.cs, change the InvokeAsync method to create a new SKContext
with empty variables if the context parameter is null, and then update
the context variables with the input string.
- In PlanTests.cs, add a new test method called
CanExecutePlanWithContextAsync that creates a mock context with some
variables, memory, skills, and log, and then invokes a plan with the
context and an input string. The test asserts that the plan returns the
input string as the result, and that the context variables are updated
with the input. The test also invokes the plan again with a different
input string and the same context, and asserts that the result and the
context variables are updated accordingly.
github-merge-queue bot pushed a commit that referenced this pull request Feb 20, 2024
@dependabot @eavanvalkenburg @moonbox3
Python: Bump psycopg from 3.1.15 to 3.1.18 in /python (#4988)
3b14d7c 
Bumps [psycopg](https://github.com/psycopg/psycopg) from 3.1.15 to
3.1.18.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/psycopg/psycopg/blob/master/docs/news.rst">psycopg's
changelog</a>.</em></p>
<blockquote>
<p>.. currentmodule:: psycopg</p>
<p>.. index::
single: Release notes
single: News</p>
<h1><code>psycopg</code> release notes</h1>
<h2>Future releases</h2>
<p>Psycopg 3.2 (unreleased)
^^^^^^^^^^^^^^^^^^^^^^^^</p>
<ul>
<li>Add support for integer, floating point, boolean <code>NumPy scalar
types</code>__

(:ticket:<code>[#332](https://github.com/psycopg/psycopg/issues/332)</code>).</li>
<li>Add <code>!timeout</code> and <code>!stop_after</code> parameters to
<code>Connection.notifies()</code>
(:ticket:<code>340</code>).</li>
<li>Add :ref:<code>raw-query-cursors</code> to execute queries using
placeholders in
PostgreSQL format (<code>$1</code>, <code>$2</code>...)
(:ticket:<code>[#560](https://github.com/psycopg/psycopg/issues/560)</code>).</li>
<li>Add <code>~rows.scalar_row</code> to return scalar values from a
query
(:ticket:<code>[#723](https://github.com/psycopg/psycopg/issues/723)</code>).</li>
<li>Add <code>~Connection.set_autocommit()</code> on sync connections,
and similar
transaction control methods available on the async connections.</li>
<li>Add support for libpq functions to close prepared statements and
portals
introduced in libpq v17
(:ticket:<code>[#603](https://github.com/psycopg/psycopg/issues/603)</code>).</li>
<li>The <code>!context</code> parameter of <code>sql</code> objects
<code>~sql.Composable.as_string()</code> and
<code>~sql.Composable.as_bytes()</code> methods is now optional
(:ticket:<code>[#716](https://github.com/psycopg/psycopg/issues/716)</code>).</li>
<li>Disable receiving more than one result on the same cursor in
pipeline mode,
to iterate through <code>~Cursor.nextset()</code>. The behaviour was
different than
in non-pipeline mode and not totally reliable
(:ticket:<code>[#604](https://github.com/psycopg/psycopg/issues/604)</code>).
The <code>Cursor</code> now only preserves the results set of the last
<code>~Cursor.execute()</code>, consistently with non-pipeline
mode.</li>
</ul>
<p>.. __: <a
href="https://numpy.org/doc/stable/reference/arrays.scalars.html#built-in-scalar-types">https://numpy.org/doc/stable/reference/arrays.scalars.html#built-in-scalar-types</a></p>
<h2>Current release</h2>
<p>Psycopg 3.1.18
^^^^^^^^^^^^^^</p>
<ul>
<li>Fix possible deadlock on pipeline exit
(:ticket:<code>[#685](https://github.com/psycopg/psycopg/issues/685)</code>).</li>
<li>Fix overflow loading large intervals in C module
(:ticket:<code>[#719](https://github.com/psycopg/psycopg/issues/719)</code>).</li>
<li>Fix compatibility with musl libc distributions affected by
<code>CPython issue
[#65821](https://github.com/psycopg/psycopg/issues/65821)</code>__
(:ticket:<code>[#725](https://github.com/psycopg/psycopg/issues/725)</code>).</li>
</ul>
<p>.. __: <a
href="https://redirect.github.com/python/cpython/issues/65821">python/cpython#65821</a></p>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/psycopg/psycopg/commit/8585a23fcd7bcf75193adbc10d3005752ba8f15f"><code>8585a23</code></a>
chore: bump psycopg package version to 3.1.18</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/ab646b70c82aafe6004064a40a3ba358142999a3"><code>ab646b7</code></a>
fix(c): drop spurious loop break in pipeline_communicate</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/bebfe97f934c9136e4db52709ac0fb4dd9cae64d"><code>bebfe97</code></a>
chore: bump cibuildwheel version</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/89394a6f36d42d308a8e672e9b5deef8e76254ae"><code>89394a6</code></a>
chore: bump checkout action to v4</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/ed579e51ca9b44af148e55d345e312f58ce12a6f"><code>ed579e5</code></a>
docs: fix tickets format</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/d4a4e8e1447de3446f614a29a8274ef7c4d03d64"><code>d4a4e8e</code></a>
Merge branch 'musl-ctypes' into maint-3.1</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/8bc51e6812cfaedebdd7afff7c86be301d5fbf66"><code>8bc51e6</code></a>
docs: mention musl-ctypes workaround in news file</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/afb040a800b2667a07dc441e8cdb94e55a0dcf65"><code>afb040a</code></a>
fix: add <code>libc.so</code> fallback for musl systems to the ctypes
impl</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/06ef0d92109a63fa1a7630804a3a26af0e0a39c9"><code>06ef0d9</code></a>
test: drop ineffective marker on fixture</li>
<li><a
href="https://github.com/psycopg/psycopg/commit/b955118e523c84f5f702d93fd74288ce51ff61db"><code>b955118</code></a>
Merge branch 'fix-interval-overflow' into maint-3.1</li>
<li>Additional commits viewable in <a
href="https://github.com/psycopg/psycopg/compare/3.1.15...3.1.18">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=psycopg&package-manager=pip&previous-version=3.1.15&new-version=3.1.18)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com>
Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrianwyatt adrianwyatt adrianwyatt approved these changes

@shawncal shawncal Awaiting requested review from shawncal

Assignees
No one assigned
Labels
.NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews PR: ready to merge PR has been approved by all reviewers, and is ready to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lemillermicrosoft @adrianwyatt