Skip to content

fix(tests,#7225): rewrite MultiConnectorTests for SK 1.78 + PlanJsonHelpers xUnit suite - #66

Open
jsboige wants to merge 4 commits into
upgrade-skfrom
fix/c716-7225-multiconnector-tests-rewrite
Open

jsboige wants to merge 4 commits into
upgrade-skfrom
fix/c716-7225-multiconnector-tests-rewrite

Conversation

@jsboige

@jsboige jsboige commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

fix(tests,#7225): rewrite MultiConnectorTests for SK 1.78 + PlanJsonHelpers xUnit suite

Context

Axe 3+ of #7225 tracks the SK 1.78 migration in the semantic-fleet submodule. The
SK 1.78 release removed Microsoft.SemanticKernel.Planning.Plan, Plan.FromJson,
Plan.State, Plan.Steps, and SequentialPlanner.CreatePlanAsync. The legacy
MultiConnectorTests exercised those APIs directly and was therefore <Compile Remove>'d
from IntegrationTests.csproj in PR #64 (c.699), breaking CI builds on main.

PR c.699 (commit 3c975a0) re-modelled samples/Plans/*.json to the new SK 1.78
ChatHistory-style format and updated the Skip reasons in MultiConnectorTests.cs,
but it did not actually rewrite the test methods themselves. This PR completes that
work.

What this PR does

  1. Rewrites MultiConnectorTests.cs against the new
    Plan-as-KernelFunction pattern. Each test now:

    • Calls PlanJsonHelpers.BuildChatHistoryFromPlanJsonAsync(planPath, input) to get
      a (ChatHistory history, IReadOnlyList<ResolvedInvocation> invocations) tuple.
    • Registers a single runtime KernelFunction (via KernelFunctionFactory.CreateFromMethod)
      that walks the resolved invocations against the configured Kernel and returns a
      Task<string> (SK 1.78 auto-wraps this in FunctionResult).
    • Iterates ChatHistory to find the AuthorRole.System message (SK 1.78 removed
      the ChatHistory.SystemMessage convenience property).
    • Replaces Json.Encode(Json.Serialize(...)) with JsonSerializer.Serialize(...).
  2. Drops <Compile Remove="Connectors\MultiConnector\MultiConnectorTests.cs" /> from
    IntegrationTests.csproj. The file now compiles cleanly under net8.0 + SK 1.78.

  3. Adds PlanJsonHelpersTests.cs — 4 xUnit tests covering the SK 1.78 fixtures
    in samples/Plans/SK178/:

    • LoadPlan_Summarize_ResolvesExpectedShape — asserts plan-level name/description
      • the single invocation's metadata.
    • LoadPlan_SummarizeTopicsElementAt_ResolvesChainedInvocations — asserts the
      3-step chain (Summarize → Topics → ElementAtIndex) including the $RESULT__TOPICS
      argument binding.
    • BuildChatHistoryFromPlan_EmitsSystemAndUserMessages — asserts the system+user
      message split.
    • BuildInvocationPlan_ResolvesVariableSubstitutionForChainedPlan — asserts that
      step 3 receives a <RESULT__TOPICS> sentinel at plan-resolution time (the
      actual value flows through at runtime via FunctionChoiceBehavior.Auto()).
  4. Adds samples/Plans/SK178/Summarize.json + Summarize_Topics_ElementAt.json —
    brought in from commit c627e40 (already implemented in that branch but not yet
    on the test-rewrite branch).

  5. Adds <None Update="Connectors\MultiConnector\Plans\SK178\*.json"> entries to
    IntegrationTests.csproj so the fixtures copy to bin/.../Connectors/MultiConnector/Plans/SK178/.

Build / test verification

cd dotnet/src/IntegrationTests
dotnet build -c Debug
# → 0 errors, 4 warnings (all pre-existing, unrelated to this PR)

dotnet test -c Debug --no-build --filter "FullyQualifiedName~PlanJsonHelpersTests"
# → 4 passed, 0 failed

dotnet test -c Debug --no-build
# → 11 total: 4 passed (PlanJsonHelpersTests) + 7 skipped (Oobabooga / ChatGPT live-server tests)

The skipped tests are the original Oobabooga and ChatGPT integration tests that
require live LLM endpoints (not available in CI); they were already Skip-marked
before this PR. The build itself — which was previously broken — now compiles cleanly.

Acceptance for #7225 (Axe 3+)

  • CI build no longer broken on main.
  • MultiConnectorTests compiles against SK 1.78.
  • PlanJsonHelpers materializes a ChatHistory + IReadOnlyList<ResolvedInvocation>
    that FunctionChoiceBehavior.Auto() can drive at runtime.
  • xUnit coverage for the SK 1.78 plan format shipped.
  • Step 4/4 of the multi-cycle plan: validate the cost-offload behavior end-to-end
    against a real Oobabooga server — see #7225 follow-up. Not blocking the merge
    of this PR because the build/test unblock is the highest-priority item from
    c.714.

Cross-repo plan

This PR targets the upstream MyIntelligenceAgency/semantic-fleet repo. Once
merged, a follow-up PR on jsboige/CoursIA will bump the submodule pointer. The
two-step pattern (upstream first, parent bump second) is the canonical one for
submodule-managed forks — see PR #7583 (c.706) for a prior example.

Co-author

Co-Authored-By: Claude-Code noreply@anthropic.com

Refs

  • See #7225 — full Axe 3+ decision rationale (FunctionChoiceBehavior.Auto() is the
    canonical SK 1.78 path; SequentialPlanner is removed).
  • Built on 3c975a0 (c.699 decision comment + Skip-reason update).
  • Imports PlanJsonHelpers.cs + SK178 fixtures from c627e40.

jsboige and others added 2 commits July 20, 2026 07:42
…del samples/Plans + update Skip reasons

Cycle c.699 (po-2025 worker, jsboigeEpita via jsboige push identity) —
Option B deferred from #6853 (SK 1.78 migration Axe 2). Issue #7225 on
jsboige/CoursIA is the tracker under #1210 Axe 3+; this commit lands the
decision + the re-modeled JSON + the updated Skip reasons on the
semantic-fleet side (jsboige/CoursIA bump-submodule-pointer PR follows
separately after this is merged upstream).

G.1 firsthand (C636/C638/C640) BEFORE editing confirmed the diagnosis:
- `dotnet/src/IntegrationTests/Connectors/MultiConnector/
  MultiConnectorTests.cs` (458 lines) exercises the legacy `Plan`
  planning model that was REMOVED in SK 1.78 (Plan.FromJson / plan.State
  / plan.Steps / SequentialPlanner.CreatePlanAsync returning a Plan).
- The file also references `Microsoft.SemanticKernel.Connectors.AI.
  OpenAI.*` (legacy namespace, split into `.Connectors.OpenAI.*` in
  SK 1.78) AND the case-mismatched `samples/` paths that do not match
  the on-disk `Samples/` directory.
- The 3 affected tests are `[Theory(Skip="manual verification")]` (never
  run in CI). The production MultiConnector path
  (`ExecuteAsync(KernelFunction)`, migrated in T2b `dece740`) already
  exercises the real functionality at the function level — so the *real*
  MultiConnector coverage exists; only the *plan-based* test is obsolete.

Acceptance coverage (issue #7225):
- #1 SK 1.78 planning paradigm chosen + documented → IntegrationTests.csproj
  decision block: hand-rolled `KernelFunction` pipeline (no external planner
  NuGet). Justification: tests are end-to-end integration probes, not unit
  tests of a planner; a hand-rolled chain keeps the assertion target
  (offloading cost, secondary connector vetting) isolated from planner-
  quality noise.
- #2 samples/Plans/*.json re-modeled to the new format → both JSONs
  rewritten to a minimal `KernelFunction`-shaped schema
  (`schema_version`, `description`, `input_variable`, `default_input`,
  `steps[].function`, `steps[].parameters`, `steps[].outputs`). Readable
  by a future hand-rolled KernelFunction pipeline without re-parsing the
  legacy Plan JSON shape.
- #3 Tests rewritten OR Skip with current reason → branch (b) chosen:
  all 3 `[Theory(Skip="This test is for manual verification.")]` updated
  to explicit, post-SK-1.78 reasons that name the blockers
  (Plan/namespace/path) and reference the csproj decision block + the
  re-modeled JSON. `<Compile Remove>` is KEPT because the file still
  references legacy namespaces and broken paths; the Skip annotations
  are dormant until a future PR lifts the Compile Remove and rewrites
  the test bodies. Documented in the csproj comment block + the PR body.
- #4 dotnet build Semantic-Fleet-dotnet.sln stays at 0 errors → verified
  on the worktree: `dotnet build` = 0 Erreur(s), 28 Avertissement(s)
  (all pre-existing, unrelated to this change).
- #5 dotnet test ≥ baseline (no regression) → verified: 4 tests
  discovered (OobaboogaCompletionTests skipped for manual verification,
  pre-existing); 0 regression. The 3 MultiConnectorTests stay out of
  discovery under `<Compile Remove>`, matching prior state.

Scope of this PR is intentionally narrow (modest cross-famille grain):
it lands the *decision* + the *data re-modeling* + the *Skip-reason
refresh* that the issue asks for. The actual test-body rewrite against
the hand-rolled `KernelFunction` pipeline is a follow-up that requires
the legacy namespace + path fixes at the MultiConnector/MultiText
CompletionSettings layer, out of scope for a single cycle.

Conformité: R3 atomicité 4 fichiers / +56 -134 ≪ 3000L; LF-only CR=0;
0 secrets sur les 4 fichiers. Cross-famille: PR upstream on
MyIntelligenceAgency/semantic-fleet; a separate CoursIA-2 PR will bump
the submodule pointer after this merges (cross-repo, two-step).

Refs #7225 (jsboige/CoursIA, Axe 3+ / #1210)
See #6853 (SK 1.78 migration, Axe 2, Option A = exclude + document)

Co-Authored-By: Claude <noreply@anthropic.com>
…elpers xUnit suite

MultiConnectorTests.cs was SKIP'd under <Compile Remove> after the legacy SK Plan.FromJson / SequentialPlanner APIs were removed in SK 1.78. This commit unblocks the integration test suite:

- MultiConnectorTests.cs rewritten against the new Plan-as-KernelFunction pattern: ChatHistory + ResolvedInvocation list built via PlanJsonHelpers.BuildChatHistoryFromPlanJsonAsync; runtime walker registered as a single KernelFunction that drives each invocation through kernel.InvokeAsync.

- PlanJsonHelpers.cs brought in from c627e40 (helper + DTOs already implemented in that branch but not yet wired into the test suite).

- PlanJsonHelpersTests.cs: 4 xUnit tests covering Summarize + Summarize_Topics_ElementAt fixtures — asserts plan shape, ChatHistory system/user split, and <RESULT__TOPICS> sentinel substitution for chained plans.

- IntegrationTests.csproj: <Compile Remove> for MultiConnectorTests dropped; SK178 fixture CopyToOutputDirectory entries added.

All 4 new tests pass; build = 0 errors. Live-server tests stay SKIP'd (Oobabooga / ChatGPT endpoints not available in CI).

See #7225 for the Axe 3+ decision rationale (FunctionChoiceBehavior.Auto() is the canonical SK 1.78 path; SequentialPlanner is removed).

Co-Authored-By: Claude-Code <noreply@anthropic.com>
jsboige added a commit that referenced this pull request Sep 3, 2026
#66) (#67)

The branch upgrade-sk hosts three PRs (#64, #65, #66) that have been
silent for ~6 weeks: the workflow on: filters exclude upgrade-sk (it
does not match the feature* glob, and is not main), so no build, no
format, no merge-gate check ever runs against these PRs. Only label-pr
fires on them, which is consistent with the ai-01 measurement 'no build
has run'.

Measured divergence main vs upgrade-sk: no common merge-base. upgrade-sk
is a fork of stable-from-v0343 with 3 additional commits; main is a
post-cleanup rewrite (264 vs 304 files). A direct merge of upgrade-sk
into main is materially impossible without massive conflict resolution,
so widening the gate temporarily is the only viable path.

This commit adds 'upgrade-sk' to the branches: filter of the four
workflows that gate the .NET PR pipeline. The widening is **temporary**:
the PR body carries the withdrawal clause required by ai-01 (PR body
below). When the SK 1.78 stack lands on main and the three PRs rebase,
upgrade-sk is removed from all four branches: lists in the same
follow-up.

Scope: 4 files, +4/-4. No code change.
jsboige added a commit that referenced this pull request Sep 4, 2026
…ine) (#68)

Mirror #67 verbatim: the four dotnet-* workflows filtered on
branches: ['main', 'feature*'] (or ['main'] for integration), which
never matched upgrade-sk. PRs #64, #65, #66 have base=upgrade-sk and
have slept since 2026-07-20.

Lines A and B are disjoint: merge-base(origin/main, origin/stable-from-v0343)
is empty (verified). #67 landed on A (base=main), so its filter
extension never touched the workflow copy resolved for PRs on B.

Critere de sortie verifiable: apres merge, PR #64 check count
passe de 1 a >1, avec dotnet-build-and-test-check present.

Refs #67, See #64 #65 #66

Co-authored-by: myia-po-2023 <myia-po-2023@myia.fr>
Records the byte-identical re-rooted upgrade-sk base as a second parent. The branch tree remains unchanged; no PR content is rewritten.

Co-Authored-By: Claude-Code <noreply@anthropic.com>
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.

1 participant