Skip to content

.NET: Preserve table state across declarative EditTable operations  - #7353

Open
peibekwe wants to merge 2 commits into
microsoft:mainfrom
peibekwe:declarative-bugfix
Open

.NET: Preserve table state across declarative EditTable operations #7353
peibekwe wants to merge 2 commits into
microsoft:mainfrom
peibekwe:declarative-bugfix

Conversation

@peibekwe

Copy link
Copy Markdown
Contributor

Motivation & Context

Keeps itemsVariable as a table after Remove, Clear, TakeFirst, and TakeLast so later edits continue to work. Results use resultVariable, and checkpointed tables remain downgrade-compatible and editable after resume.

Fixes #7351

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

@peibekwe peibekwe self-assigned this Jul 28, 2026
Copilot AI review requested due to automatic review settings July 28, 2026 02:34
@peibekwe
peibekwe temporarily deployed to github-app-auth July 28, 2026 02:34 — with GitHub Actions Inactive
@peibekwe
peibekwe temporarily deployed to github-app-auth July 28, 2026 02:34 — with GitHub Actions Inactive
@peibekwe
peibekwe temporarily deployed to github-app-auth July 28, 2026 02:34 — with GitHub Actions Inactive
@agent-framework-automation agent-framework-automation Bot added .NET Usage: [Issues, PRs], Target: .Net workflows Usage: [Issues, PRs], Target: Workflows labels Jul 28, 2026
@github-actions github-actions Bot changed the title Preserve table state across declarative EditTable operations  .NET: Preserve table state across declarative EditTable operations  Jul 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the .NET declarative EditTable / EditTableV2 executors so table-edit operations (Remove, Clear, TakeFirst, TakeLast) preserve the original table variable as a TableValue, enabling subsequent table edits and improving checkpoint/resume compatibility. It also expands unit test coverage to validate table preservation and portable (checkpoint) round-trips.

Changes:

  • Update EditTableExecutor and EditTableV2Executor to keep itemsVariable bound to a (mutated) table after Clear/Remove/TakeFirst/TakeLast, with taken items routed via ResultVariable.
  • Add/adjust unit tests to validate preserved table schema/shape and editability after portable restore and checkpoint/resume.
  • Refine portable table restoration logic for legacy shapes and add tests for legacy portability expectations.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/EditTableExecutor.cs Preserves table variables across edits and routes operation results via ResultVariable.
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/EditTableV2Executor.cs Aligns V2 executor behavior with table preservation and supports ResultVariable for take operations.
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Extensions/PortableValueExtensions.cs Updates portable → formula table reconstruction to better preserve table shapes/types.
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/EditTableExecutorTest.cs Adds/updates tests for table preservation, portable restore, and checkpoint/resume scenarios.
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/EditTableV2ExecutorTest.cs Adds/updates V2 tests ensuring operations preserve table state and set results as expected.
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Extensions/PortableValueExtensionsTests.cs Adds tests asserting legacy portable shapes and append behavior after restoration.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 5 | Confidence: 89%

✓ Correctness

The PR correctly preserves table state across EditTable operations (Remove, Clear, TakeFirst, TakeLast) by keeping the table in itemsVariable and writing taken/removed results to resultVariable. The PortableValueExtensions changes ensure restored tables are mutable (support AppendAsync) by materializing records into arrays. The special-case handling for empty+schemaless tables in the Add operation gracefully handles the schema loss that occurs during checkpoint round-trips of empty tables. AssignAsync safely handles null PropertyPath by short-circuiting. The changes are well-covered by comprehensive tests including checkpoint/restore scenarios.

✓ Security Reliability

The PR preserves table state across declarative EditTable operations by keeping itemsVariable as a table after Remove, Clear, TakeFirst, and TakeLast, and routing extracted values to resultVariable. The changes are well-structured and well-tested. AssignAsync safely handles null PropertyPath? via early return (DeclarativeActionExecutor.cs:129-131), so the nullable this.Model.ResultVariable?.Path calls are safe. The empty-table schema-loss during checkpoint serialization (via RecordType.Empty() in ToTable for 0-length arrays) is correctly compensated in the Add path by inferring schema from the new value. No security, injection, resource leak, or reliability issues found.

✓ Test Coverage

Test coverage for this PR is comprehensive. All key behavioral changes — table preservation across Remove/Clear/TakeFirst/TakeLast, resultVariable assignment, checkpoint/restore roundtrips, and legacy table mutability — are well-covered with meaningful assertions. Both V1 (EditTableExecutor) and V2 (EditTableV2Executor) have thorough test suites, with composite operation tests (e.g., clear→restore→add, takeFirst→add) validating the core fix. Two minor symmetric gaps exist but are low risk.

✓ Failure Modes

The PR correctly preserves table state across EditTable operations by keeping itemsVariable as a table after Remove, Clear, TakeFirst, and TakeLast, routing extracted/result values to resultVariable instead. The PortableValueExtensions.ToTable change (.ToArray()) ensures restored tables are mutable and support AppendAsync. The AssignAsync base method safely handles null ResultVariable?.Path by returning early. The empty-schema-table Add path correctly detects schema-less tables via !recordType.FieldNames.Any() && !tableValue.Rows.Any() and rebuilds from the record value. Both V1 and V2 executors are updated consistently. No concrete new failure modes were identified.

✓ Design Approach

The table-preservation approach looks sound for record tables and most primitive tables, but it still leaves a compatibility gap for date-only primitive tables restored from checkpoints. The new PortableValue[] -> TableValue path infers DateTime for restored date-like columns even though the same portable scalar path round-trips midnight DateTime values as FormulaType.Date, so the PR does not fully satisfy its stated downgrade/editability goal for that input.


Automated review by peibekwe's agents

@peibekwe
peibekwe temporarily deployed to github-app-auth July 28, 2026 05:10 — with GitHub Actions Inactive
@peibekwe
peibekwe deployed to integration July 28, 2026 05:10 — with GitHub Actions Active
@peibekwe
peibekwe marked this pull request as ready for review July 28, 2026 05:23
@peibekwe
peibekwe temporarily deployed to github-app-auth July 28, 2026 05:23 — with GitHub Actions Inactive

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 5 | Confidence: 90% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by peibekwe's agents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Usage: [Issues, PRs], Target: .Net workflows Usage: [Issues, PRs], Target: Workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.NET: [Bug]: Declarative EditTable operations replace itemsVariable with non-table values, preventing subsequent edits

2 participants