Skip to content

refactor(execution)!: align context overloads - #418

Merged
thomhurst merged 5 commits into
mainfrom
issue-392-parent-context-names
Aug 27, 2026
Merged

refactor(execution)!: align context overloads#418
thomhurst merged 5 commits into
mainfrom
issue-392-parent-context-names

Conversation

@thomhurst

@thomhurst thomhurst commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • rename parent-context execution overloads to ExecuteWithContextAsync
  • add synchronous ExecuteWithContext parent-context overloads with sync-pipeline enforcement
  • update analyzer coverage, public API baselines, generated/user documentation, and execution benchmarks

Performance

BenchmarkDotNet 0.15.8, .NET 10.0.11, Windows 11, Intel Core i7-12700K:

Benchmark origin/main PR Change Allocated
Kevlar_NestedEmptyAsync 142.7 ns 144.0 ns +0.9% 0 B
Kevlar_NestedEmptySync n/a 118.6 ns new API 0 B

The async confidence intervals overlap (139.8–145.6 ns before; 142.2–145.8 ns after), showing no material regression.

Validation

  • dotnet build Kevlar.slnx -c Release --no-restore
  • dotnet run --project tests/Kevlar.Tests -c Release --no-build -- --timeout 5m
  • dotnet run --project tests/Kevlar.IntegrationTests -c Release --no-build -- --timeout 5m
  • dotnet run --project tests/Kevlar.Analyzers.Tests -c Release --no-build -- --timeout 5m
  • allocation tests on net8.0 and net10.0
  • dotnet run --project benchmarks/Kevlar.Benchmarks -c Release -- --filter "*Kevlar_NestedEmpty*"
  • pwsh scripts/Verify-ApiDocs.ps1
  • pwsh scripts/Verify-Docs.ps1
  • pwsh scripts/Verify-Repo.ps1
  • pwsh scripts/Verify-DocSnippets.ps1 -NoImplicitUsings
  • npm run build in docs/
  • CI-equivalent Verify-Packages.ps1 with package layout, determinism, symbols, SourceLink, consumers, publishing, and analyzers

Closes #392

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e9dd0ef5-332b-4a1e-bb59-0e547b90a467


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thomhurst
thomhurst force-pushed the issue-392-parent-context-names branch from a3188c6 to 5ebf790 Compare August 27, 2026 21:30

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ebf790c8d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Internal/ShieldEngine.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9dda3743a4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Shield.cs Outdated
Comment thread src/Kevlar/Shield.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8f6a038ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/docs/polly-migration.md
@thomhurst

Copy link
Copy Markdown
Owner Author

Rebased onto current main after #412 and #416. Post-rebase Release build, 1,301 net10 core tests, API verification, and docs verification pass.

@thomhurst
thomhurst force-pushed the issue-392-parent-context-names branch from f8f6a03 to 0501b67 Compare August 27, 2026 22:14

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 456193343a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/docs/executing.md
@thomhurst

Copy link
Copy Markdown
Owner Author

Coordination: current head 45619334 was force-rewritten from a stale branch and dropped the reviewed fixes in 9dda3743/f8f6a038. In particular, parent-context APIs reverted to (action, parentContext), restoring the explicit-default CS0121 ambiguity reported in discussion_r3876152994; analyzer guidance fixes were also lost. The docs should remain parent-first. Please reapply/cherry-pick f8f6a038 (including 9dda3743) onto the current head before addressing the latest docs review.

@thomhurst
thomhurst force-pushed the issue-392-parent-context-names branch from 4561933 to c6ecea2 Compare August 27, 2026 22:24
@thomhurst

Copy link
Copy Markdown
Owner Author

Coordination update: current head c6ecea24 still does not contain the reviewed 9dda3743/f8f6a038 fixes. gh pr diff and generated API docs show ExecuteWithContextAsync(Func<...> action, KevlarContext parentContext) / ExecuteWithContext(... action, KevlarContext parentContext), so the explicit-default CS0121 ambiguity remains and the unresolved docs thread is based on that reverted API. Please restore the parent-first overloads plus dynamic KEV002/KEV012 guidance/regressions before merge; f8f6a038 is preserved in C:\git\Kevlar-worktrees\pr-418-async-guidance.

@thomhurst

Copy link
Copy Markdown
Owner Author

CI confirms the coordination warning: Ubuntu doc snippets fail because current APIs are still action-first while docs are parent-first (docs/docs/executing.md:152-156 and polly-migration.md:169-178; CS1503/CS1660/CS1061). Restoring commit f8f6a03 resolves both the reviewed ambiguity/analyzer regressions and this gate.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d82a21c560

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Internal/ShieldEngine.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

Queue coordination: current head d82a21c is verified correct, all review threads resolved, and every CI gate now passes. Ready for owner to merge via scripts/Merge-Pr.ps1 and release pr-418 lock.

Name nested context delegates consistently with the existing ExecuteWithContext convention and provide synchronous parent-context parity before the public API freezes.

Refs #392
Report the context-aware and outcome async counterpart when a synchronous pipeline is unsupported.

Refs #392
@thomhurst
thomhurst force-pushed the issue-392-parent-context-names branch from d82a21c to 7134625 Compare August 27, 2026 22:58
@thomhurst
thomhurst merged commit 8680d7a into main Aug 27, 2026
9 checks passed
@thomhurst
thomhurst deleted the issue-392-parent-context-names branch August 27, 2026 23:09
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.

refactor(execution)!: rename parent-context ExecuteAsync overloads to ExecuteWithContextAsync; add sync twin

1 participant