You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Keep only formatting skills and verify skill value independently - #985
Keep only two optional report-formatting skills, move general Excel guidance into documentation while preserving website addresses, and adopt pytest-skill-engineering 1.0.3 with independently verified skill-value comparisons.
The completed broad-skill comparison found equal correctness across 48 unique cases, with all 24 treatment cases successfully reading skills, but 23.4% more recorded MCP tokens and 72.4% more CLI tokens. The new narrow formatting skills remain unproven; no new paid calls were made during delivery.
Type of Change
🐛 Bug fix (non-breaking change which fixes an issue)
✨ New feature (non-breaking change which adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
Closes: N/A; user-directed work.
Relates to sbroenne/pytest-skill-engineering#114, the separate upstream case study and reproducible example. It is not merged by this request.
Changeset
Added a changeset (npx changeset) describing this change for end users — see .changeset/README.md
Not applicable (docs/tests/CI/dependency-only) — added skip-changelog label instead
Changes Made
Replace broad skill templates with excel-cli-report-formatting and excel-mcp-report-formatting. Each short entry ships with one selected formatting reference; general workflows and recovery guides live in docs/reference and docs/guides.
Remove obsolete template build tasks and dependencies. No Core command method or CLI/MCP tool/action/parameter contract changes; the advertised surface remains 31 tools and 326 operations.
Remove 24 weak live-test files and obsolete retry/judge/report machinery. Add paired comparisons, no-send discovery, explicit paid-attempt accounting, durable interruption evidence, and independent saved-workbook checks. Publish only sanitized historical evidence; native traces and benchmark workbooks remain private.
Add an unrun 24-case narrow-formatting comparison and checker regressions, including rejection of a misleading literal percentage. Keep skill availability/reading, workbook correctness and nullable usage separate.
Rebase onto current main, preserve its AGENTS.md/shared-rule migration, update moved guidance links, and resolve the lock conflict without retaining an orphan dependency.
Address all three review findings in 32fa7fc: reuse the tracked peer workbook, count only successful skill reads, and require identical verified task/repetition sets before publishing token percentages. Add explicit exclusion reasons and failing-before/passing-after regressions. Regenerated historical measurements remain unchanged.
Testing Performed
If Core/ComInterop/Service/CLI/MCP runtime paths or their generators changed: ran & .\scripts\Test-E2E.ps1 locally and confirmed it completed with no failures or unresolved issues
Excel E2E not applicable because only documentation/configuration/Excel-free tooling changed
Ran the relevant feature-specific integration tests and recorded the exact command and result below
Tested manually with various Excel files
Verified cleanup of this test's owned Excel sessions without terminating unrelated Excel processes (if applicable)
XLSM file format validation tested (if applicable)
VBA trust setup tested (if applicable)
Build produces zero warnings
Test Commands
# Record each exact command and its result (passed, failed, or not run with reason).# Configuration/docs-only work: record the relevant checks; Excel tests are not required.
dotnet restore Sbroenne.ExcelMcp.sln
# Passed.
dotnet build Sbroenne.ExcelMcp.sln -c Release --no-restore
# Passed: zero warnings and errors. Normal review-fix commit hooks rebuilt successfully too.& .\scripts\check-com-leaks.ps1
& .\scripts\check-success-flag.ps1
& .\scripts\check-dynamic-casts.ps1
& .\scripts\check-workbook-package-access.ps1
& .\scripts\check-doc-counts.ps1 -SkipBuild
# All passed.& .\scripts\Invoke-ExcelFreeTests.ps1-Local -Contracts
# Passed: 57 Core, 53 CLI, 9 MCP contract tests; also passed in review-fix commit hooks.& .\scripts\Test-E2E.ps1-SkipBuild
# Passed sequentially with desktop Excel: 16 CLI checks, graceful-save acceptance,# and the MCP all-tools workflow. Normal commit hooks reran these successfully# for both the initial changes and the review-fix commit.
uv lock --project llm-tests --check --offline
# Passed after rebase: 55 packages resolved; lock matches the manifest.
uv run --project llm-tests --frozen pytest llm-tests\test_aggregate_skill_value.py llm-tests\test_eval_harness.py llm-tests\test_skill_value.py llm-tests\test_skill_value_checks.py llm-tests\test_skill_value_preparation.py llm-tests\test_skill_discovery.py llm-tests\test_spreadsheetbench.py llm-tests\test_cli_mcp_server.py llm-tests\test_cli_result_assertions.py llm-tests\test_consent_scenarios.py -q
# Passed after rebase, before review fixes: 88 passed, 24 gated paid cases skipped,# 73 subtests passed; no model sends.
uv run --project llm-tests --frozen pytest llm-tests\test_aggregate_skill_value.py llm-tests\test_owned_workbook_cleanup.py -q
# Passed after review fixes: 12 tests and 3 subtests, including live Excel cleanup# and unique-wrapper tracking. Relevant regressions failed before the fixes.
uv run --project llm-tests --frozen python llm-tests\aggregate_skill_value.py llm-tests\TestResults\skill-value-real-world-1.0.3-20261001-2202 llm-tests\TestResults\skill-value-real-world-1.0.3-20261002-resume llm-tests\TestResults\skill-value-real-world-1.0.3-20261002-journaled --output llm-tests\evidence\real-world-1.0.3.json
# Passed: only exclusion metadata and scope wording changed; all published# historical cases, successful reads and token measurements remain unchanged.# From gh-pages, after rebase:& .\.venv\Scripts\python.exe-m mkdocs build --strict --clean
& .\.venv\Scripts\python.exe audit_site.py
& .\.venv\Scripts\python.exe check_deploy_paths.py
# All passed: 56 audited pages; 48 mirrored sources covered by 24 deploy paths.# From vscode-extension:
npm run package
# Passed: prepared compile, lint, type checks, 16 tests, x64/ARM64 VSIX builds.# Inspected both VSIXs: only the MCP formatting skill and selected reference ship.
Normal commit hooks were never bypassed. Initial delivery also passed 64 skill-generation/packaging/script tests and 367 MCP tests. Earlier formatting checker proofs and four no-send narrow-skill discovery checks passed.
Not run: new paid formatting-skill comparisons. Existing unrelated moderate npm audit findings were not changed.
Screenshots (if applicable)
N/A; optional guidance, test evidence, and packaging rather than a UI change.
Core Commands Coverage Checklist ⚠️
Does this PR add or modify Core Commands methods? [ ] Yes [x] No
If YES, verify all steps completed:
Updated the annotated Core Commands interface and implementation
Built Release so source generators refreshed Service, CLI, and MCP surfaces
Ran scripts\Invoke-ExcelFreeTests.ps1 -Local -Contracts
Verified CLI and MCP names, parameters, defaults, validation, and results match
Updated focused integration tests for the affected entry points
Updated canonical guidance in docs/reference, affected formatting skills, and user documentation when behavior changed
Coverage Impact: +0 methods; no Core command coverage change.
Checklist
Code follows project style guidelines
Self-review of code completed
Code builds with zero warnings
Appropriate error handling added
Updated help text (if adding new commands)
Updated README.md (if needed)
Follows AGENTS.md and the applicable COM safety guide
Uses batch API with proper disposal (using var batch or await using var batch)
The historical comparison consumed 50 reserved attempts for 48 completed unique cases under a separately approved 100-attempt resumed ceiling. Two interrupted attempts have unknown usage; they are excluded from token percentages, not counted as zero. Earlier comparisons are not pooled. The configured live event callback received zero events, so no live journaling benefit is claimed. Fixed checkers independently verified completed workbook outputs.
Skill entries are about 1.6 KB each; prepared packages contain two Markdown files, about 10 KB combined. General documentation is no longer loaded as a default skill. Old standalone skill names require replacement after a separately authorized publication. This PR does not publish a release or marketplace update.
All three original review threads have been answered and resolved. Merge remains gated on checks for the updated head.
Move general Excel guidance into documentation while preserving website URLs. Ship only the MCP and CLI report-formatting skills, update all distribution surfaces, adopt pytest-skill-engineering 1.0.3, and retain paired comparisons with saved-workbook verification and honest historical evidence.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
One or more custom setup steps configured for this repository failed during this Copilot code review run:
Verify setup tools and Excel-free boundary
Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.
Note
You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.
Replaces broad Excel skills with narrowly scoped report-formatting skills, moves general guidance into documentation, and strengthens independent skill-value evaluation.
Changes:
Introduces MCP and CLI report-formatting skills and updates packaging/publication.
Migrates general guidance to docs/reference while preserving website routes.
Upgrades and restructures LLM evaluation, evidence, and workbook verification.
Count only successful skill reads, require matched verified task/repetition sets for token percentages, and record explicit comparison exclusions. Reuse the tracked peer workbook and prove no COM wrapper is tracked twice. Regenerated historical evidence retains all published measurements.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
One or more custom setup steps configured for this repository failed during this Copilot code review run:
Verify setup tools and Excel-free boundary
Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.
Note
You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.
The default suite is formatting, and cases(None, "formatting") generates 24 executions; 60 is the default approved attempt ceiling, not the matrix size. Calling this a 60-execution matrix misstates both the expected paid run and the accounting model. Please describe the 24-execution default matrix separately from the 60-attempt ceiling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keep only two optional report-formatting skills, move general Excel guidance into documentation while preserving website addresses, and adopt pytest-skill-engineering 1.0.3 with independently verified skill-value comparisons.
The completed broad-skill comparison found equal correctness across 48 unique cases, with all 24 treatment cases successfully reading skills, but 23.4% more recorded MCP tokens and 72.4% more CLI tokens. The new narrow formatting skills remain unproven; no new paid calls were made during delivery.
Type of Change
Related Issues
Closes: N/A; user-directed work.
Relates to sbroenne/pytest-skill-engineering#114, the separate upstream case study and reproducible example. It is not merged by this request.
Changeset
npx changeset) describing this change for end users — see.changeset/README.mdskip-changeloglabel insteadChanges Made
excel-cli-report-formattingandexcel-mcp-report-formatting. Each short entry ships with one selected formatting reference; general workflows and recovery guides live indocs/referenceanddocs/guides.Testing Performed
& .\scripts\Test-E2E.ps1locally and confirmed it completed with no failures or unresolved issuesTest Commands
Normal commit hooks were never bypassed. Initial delivery also passed 64 skill-generation/packaging/script tests and 367 MCP tests. Earlier formatting checker proofs and four no-send narrow-skill discovery checks passed.
Not run: new paid formatting-skill comparisons. Existing unrelated moderate npm audit findings were not changed.
Screenshots (if applicable)
N/A; optional guidance, test evidence, and packaging rather than a UI change.
Core Commands Coverage Checklist⚠️
Does this PR add or modify Core Commands methods? [ ] Yes [x] No
If YES, verify all steps completed:
scripts\Invoke-ExcelFreeTests.ps1 -Local -Contractsdocs/reference, affected formatting skills, and user documentation when behavior changedCoverage Impact: +0 methods; no Core command coverage change.
Checklist
using var batchorawait using var batch).EscapeMarkup()Additional Notes
The historical comparison consumed 50 reserved attempts for 48 completed unique cases under a separately approved 100-attempt resumed ceiling. Two interrupted attempts have unknown usage; they are excluded from token percentages, not counted as zero. Earlier comparisons are not pooled. The configured live event callback received zero events, so no live journaling benefit is claimed. Fixed checkers independently verified completed workbook outputs.
Skill entries are about 1.6 KB each; prepared packages contain two Markdown files, about 10 KB combined. General documentation is no longer loaded as a default skill. Old standalone skill names require replacement after a separately authorized publication. This PR does not publish a release or marketplace update.
All three original review threads have been answered and resolved. Merge remains gated on checks for the updated head.