Skip to content

Add rolling-versus-exact parser equivalence tests - #1097

Open
Gudge (MGudgin) wants to merge 3 commits into
user/gudge/version_specific_config_parsers_phase7cfrom
user/gudge/version_specific_config_parsers_phase7d
Open

Add rolling-versus-exact parser equivalence tests#1097
Gudge (MGudgin) wants to merge 3 commits into
user/gudge/version_specific_config_parsers_phase7cfrom
user/gudge/version_specific_config_parsers_phase7d

Conversation

@MGudgin

@MGudginGudge (MGudgin) commented Sep 3, 2026

Copy link
Copy Markdown
Member

📖 Description

This PR adds a private differential harness that classifies
rolling-versus-exact parser behavior while rolling parsing remains
authoritative in production.

Details

  • Compares complete one-shot and state-aware runtime models, diagnostics,
    loader modes, command splicing, every lifecycle phase, and every registered
    contract version.
  • Explicitly inventories 125 exact-stricter corpus files across configs,
    examples, and policy fixtures; new or changed divergences require deliberate
    classification.
  • Fails on every exact-looser acceptance or accepted-model mismatch.

Tests

  • cargo fmt --all -- --check
  • cargo check --workspace --all-targets
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • macOS aarch64-apple-darwin cross-target check for wxc_common
Microsoft Reviewers: Open in CodeFlow

@MGudgin
Gudge (MGudgin) requested a review from a team as a code ownerSeptember 3, 2026 16:46
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

CopilotAI balanced review requested due to automatic review settings September 3, 2026 17:42
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 01f3c01 to 16b8ec3CompareSeptember 3, 2026 17:42
@MGudgin
Gudge (MGudgin) requested a review from a teamSeptember 3, 2026 17:42

CopilotAI 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.

Copilot review overview

🟡 Changes recommended

Rejection pairs bypass diagnostic comparison, allowing unclassified diagnostic drift.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — This branch treats any rolling/exact rejection pair as equivalent without comparing…
What changed in this PR

Adds a private differential harness comparing rolling and exact configuration parsers.

Changes:

  • Compares runtime models across versions and lifecycle phases.
  • Exercises loader modes, command splicing, and diagnostics.
  • Inventories 125 expected corpus divergences.
FileDescription
src/​core/​wxc_common/​src/​config_parser.rsAdds differential parser snapshots, matrices, and corpus validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadsrc/core/wxc_common/src/config_parser.rs Outdated
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 16b8ec3 to 225977fCompareSeptember 3, 2026 18:14
CopilotAI review requested due to automatic review settings September 3, 2026 18:27
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 225977f to 2d4b094CompareSeptember 3, 2026 18:27

CopilotAI 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.

Copilot review overview

🔵 Needs a closer look

Corpus cases rejected by both parsers currently bypass diagnostic-equivalence validation.

Review tier: Balanced
Findings: 1 Medium severity

Pre-existing issues (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — This branch treats any rolling/exact rejection pair as equivalent without comparing… View comment
Suppressed comments (1)

src/core/wxc_common/src/config_parser.rs:3432

  • When both parsers reject a corpus file, this branch accepts any mismatch in route, category, path, source location, or message. A diagnostic regression—or the exact parser beginning to reject for a different reason—will therefore pass without deliberate classification. Compare the two rejection snapshots here and require any intentional mismatch to be listed in an explicit diagnostic-divergence inventory.
 (ParserSnapshot::Rejected(_), ParserSnapshot::Rejected(_)) => {

CopilotAI review requested due to automatic review settings September 4, 2026 17:58
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 2d4b094 to 8965f00CompareSeptember 4, 2026 17:58

CopilotAI 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.

Copilot review overview

🟡 Changes recommended

The harness discards logger diagnostics, leaving the claimed diagnostic equivalence incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — The differential snapshot discards both parsers' Logger state, so it does not actually compare…
Pre-existing issues (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — This branch treats any rolling/exact rejection pair as equivalent without comparing… View comment

Comment threadsrc/core/wxc_common/src/config_parser.rs
CopilotAI review requested due to automatic review settings September 4, 2026 18:56
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 8965f00 to efa59caCompareSeptember 4, 2026 18:56

CopilotAI 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.

Copilot review overview

🟢 Approval recommended

The test-only changes comprehensively enforce the stated parser-equivalence guarantees without identified defects.

Review tier: Balanced
Findings: 1 Medium severity

Pre-existing issues (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — The differential snapshot discards both parsers' Logger state, so it does not actually compare… View comment
Issues resolved since last review (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — This branch treats any rolling/exact rejection pair as equivalent without comparing… View resolved comment

CopilotAI review requested due to automatic review settings September 4, 2026 19:35
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from efa59ca to 9dd5991CompareSeptember 4, 2026 19:35

CopilotAI 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.

Copilot review overview

🟢 Approval recommended

The test-only harness comprehensively detects unintended parser drift while preserving rolling parsing as production authority.

Review tier: Balanced
Findings: 1 Medium severity

Pre-existing issues (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — The differential snapshot discards both parsers' Logger state, so it does not actually compare… View comment

CopilotAI review requested due to automatic review settings September 4, 2026 23:52
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 9dd5991 to 5ccb5c8CompareSeptember 4, 2026 23:52

CopilotAI 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.

Copilot review overview

🔵 Needs a closer look

Classified diagnostic-divergence paths currently omit logger parity checks.

Review tier: Balanced
Findings: 1 Medium severity

Pre-existing issues (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — The differential snapshot discards both parsers' Logger state, so it does not actually compare… View comment
Suppressed comments (2)

src/core/wxc_common/src/config_parser.rs:3061

  • This diagnostic-divergence path compares only the dereferenced DiagnosticSnapshot; it never compares the LoggerSnapshot retained by each rejection. A change that adds/removes primary output or warnings for any of these cases will therefore pass even though logger parity is part of the differential harness. Assert logger equality before allowing the message divergence.
 let rolling_message = case.rolling_message.unwrap();

src/core/wxc_common/src/config_parser.rs:3724

  • Known diagnostic differences bypass RejectedSnapshot equality and validate only the diagnostic fields, so logger-output drift for these corpus fixtures is silently accepted. Include rolling_diagnostic.logger != exact_diagnostic.logger in the blocker condition to preserve the harness's logger-parity guarantee.
 let exact_mismatches =
diagnostic_expectation_mismatches(expected.exact, exact_diagnostic);
if !rolling_mismatches.is_empty() || !exact_mismatches.is_empty() {

@MGudgin

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@MGudgin

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

CopilotAI review requested due to automatic review settings September 5, 2026 05:02
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 5ccb5c8 to 4fb1d17CompareSeptember 5, 2026 05:02

CopilotAI 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.

Copilot review overview

🟡 Changes recommended

The model snapshot omits the telemetry requested_sandbox_kind field, allowing parser drift to go undetected.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rsserde_json::to_value(request) also omits TelemetryConfig::requested_sandbox_kind
Issues resolved since last review (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — The differential snapshot discards both parsers' Logger state, so it does not actually compare… View resolved comment

Comment threadsrc/core/wxc_common/src/config_parser.rs
CopilotAI review requested due to automatic review settings September 5, 2026 05:46
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 4fb1d17 to 7bc55cfCompareSeptember 5, 2026 05:46

CopilotAI 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.

Copilot review overview

🔵 Needs a closer look

Classified diagnostic divergences currently allow logger output to drift undetected.

Review tier: Balanced
Findings: None

Issues resolved since last review (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rsserde_json::to_value(request) also omits TelemetryConfig::requested_sandbox_kindView resolved comment
Suppressed comments (2)

src/core/wxc_common/src/config_parser.rs:3903

  • Expected corpus diagnostic divergences bypass logger parity: the mismatch helper receives the dereferenced DiagnosticSnapshot, while the whole-snapshot inequality is already satisfied by the intentionally different diagnostic. Consequently, logger output can drift for any listed fixture without becoming a blocker. Compare the two logger snapshots explicitly in this branch.
 Some(expected) => {
observed_diagnostics.insert(relative.clone());

src/core/wxc_common/src/config_parser.rs:3258

  • This DiagnosticOnly branch validates only the error messages, so a change to either parser's primary log buffer or warnings still passes as an intentional diagnostic divergence. Since logger divergences are not classified, assert that the logger snapshots remain equal before accepting the diagnostic-only difference.
 let rolling_message = case.rolling_message.unwrap();

Gudgeand others added 3 commits September 5, 2026 11:43
This PR adds a private differential harness that classifies rolling-versus-exact parser behavior while rolling parsing remains authoritative in production.
Details
* Compare complete one-shot and state-aware runtime models, diagnostics, loader modes, command splicing, lifecycle operations, and every registered contract version.
* Explicitly inventory exact-stricter corpus files across configs, examples, and policy fixtures so new or changed divergences require deliberate classification.
* Compare shared-rejection diagnostic snapshots and classify intentional differences by route, category, path, location, and message.
* Fail on exact-looser acceptance, accepted-model mismatch, or unclassified diagnostic drift.
Tests
* `cargo fmt --all -- --check`
* `cargo check --workspace --all-targets`
* `cargo clippy --workspace --all-targets -- -D warnings`
* `cargo test --workspace`
* `cargo check -p wxc_common --all-targets --target aarch64-apple-darwin`
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f04437d-4a05-487a-b402-4ed4c774e8e0
This PR fixes differential coverage of telemetry fields omitted from serialized execution requests.
Details
* Compare the serde-skipped requested_sandbox_kind field in execution snapshots.
* Prove attribution changes and omissions are detected even when serialized models match.
Tests
* cargo fmt --all -- --check
* cargo check -p wxc_common --all-targets
* cargo clippy -p wxc_common --all-targets -- -D warnings
* cargo test -p wxc_common --lib: 1,171 passed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f04437d-4a05-487a-b402-4ed4c774e8e0
This PR fixes the diagnostic coverage gap that allowed the development parser to bypass path-aware error rendering unnoticed.
Details
* Compare typed-error paths, positions, categories, and routes for all four exact one-shot versions.
* Include development state-aware exec in the differential diagnostic matrix rather than checking only the published parser.
Tests
* cargo fmt --all; cargo check -p wxc_common --all-targets; cargo clippy -p wxc_common --all-targets -- -D warnings passed.
* cargo test -p wxc_common --lib passed, including the expanded differential diagnostics and the new exact-root regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f04437d-4a05-487a-b402-4ed4c774e8e0
CopilotAI review requested due to automatic review settings September 5, 2026 19:16
@MGudgin
Gudge (MGudgin)force-pushed the user/gudge/version_specific_config_parsers_phase7d branch from 7bc55cf to e26d5b2CompareSeptember 5, 2026 19:16

CopilotAI 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.

Copilot review overview

🟡 Changes recommended

Divergence fixtures do not validate complete diagnostics, allowing route, location, category, or logger regressions to pass.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitysrc/​core/​wxc_common/​src/​config_parser.rs — This case schema records only message fragments, so assert_classified_divergence does not pin the…

Comment on lines +2310 to +2316
struct DivergenceCase {
name: &'static str,
input: &'static str,
direction: DivergenceDirection,
rolling_message: Option<&'static str>,
exact_message: &'static str,
reason: &'static str,
Sign up for freeto 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.

2 participants

@MGudgin