Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ Issue #354 (stale-app-config-binding-redirects): the atomic-planner's `durable-s
**Why:** The mandatory coverage rule in the feature-review SKILL has no carve-out for "tooling scripts committed to a feature folder rather than production `src/`." Treating it as exempt would be an unauthorized scope-narrowing.

**How to apply:** When a feature-folder diff includes a new `.py`/`.ps1`/`.ts`/`.cs` file under `scripts/` (not just under conventional production paths), still classify it as a changed-file language and require the coverage row. Independently verify the script's *correctness* (re-run it, diff its output, run Black/Ruff/Pyright directly) as strong corroborating evidence for the code-review, but do not let that substitute for the missing coverage artifact in the policy-audit verdict. See [[project_coverage-hook-label-substring-false-positive]] for the adjacent Test-LanguageCoverageRow line-matching mechanics used to write a compliant FAIL row.

**Confirmed instance under `evidence/` (not just `scripts/`)**: #394 (utilitiescs-test-cs2002-duplicate-compile-entry) — an executor retained a `.ps1` sweep helper under `<FEATURE>/evidence/baseline/duplicate-sweep.ps1` "for reproducibility" even though the plan only required capturing the script's *output* in a `.md` artifact. `Get-ChangedLanguageSet` matches any `.ps1`/`.py` bullet regardless of path, so the same mandatory-coverage-gate logic applies to committed evidence-tree scripts, not just `<FEATURE>/scripts/`. Recommended remediation in that case: delete the retained script (its logic/output is already durably captured in the sibling timestamped `.md` narrative) rather than build out Pester/PoshQC/coverage tooling for a one-off audit helper — this is the proportionate fix for a scope-locked bugfix and avoids the unfunded toolchain obligation entirely.
1 change: 0 additions & 1 deletion UtilitiesCS.Test/UtilitiesCS.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@
<Compile Include="OutlookObjects\Folder\FolderScorerRegressionTests.cs" />
<Compile Include="OutlookObjects\Folder\FolderScoreTests.cs" />
<Compile Include="OutlookObjects\Folder\FolderScorerTests.cs" />
<Compile Include="OutlookObjects\Folder\PercentageFormatterTests.cs" />
<Compile Include="OutlookObjects\Folder\FolderNodeViewModelTests.cs" />
<Compile Include="OutlookObjects\Folder\FolderHierarchyBuilderTests.cs" />
<Compile Include="OutlookObjects\Folder\FolderTreeStateModelTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Code Review — utilitiescs-test-cs2002-duplicate-compile-entry-394

- **Issue:** #394
- **Branch:** `bug/utilitiescs-test-cs2002-duplicate-compile-entry-394`
- **Base:** `origin/epic/build-ci-coverage-gate-fidelity-integration` (merge-base `a5e336e5`)
- **Timestamp:** 2026-08-10T23-45

## Executive Summary

The substantive code change — deletion of one duplicate `<Compile Include>` item from `UtilitiesCS.Test/UtilitiesCS.Test.csproj` — is minimal, correctly targeted, and free of quality issues. No production `.cs` code, class, or method is added or modified. Review findings below are limited to (a) the newly committed evidence-tree helper script, which does not meet the repository's PowerShell coding/toolchain standards, and (b) minor documentation-accuracy discrepancies in `spec.md`'s duplicate-sweep table versus the actual captured sweep evidence. Neither the deleted line nor the surrounding `.csproj` content shows any stylistic or correctness issue.

## Findings Table

| Severity | File | Location | Finding | Recommendation | Rationale | Evidence |
|---|---|---|---|---|---|---|
| Blocking | `docs/.../evidence/baseline/duplicate-sweep.ps1` | whole file | Committed PowerShell file with no PoshQC format/analyze run, no Pester test, and no coverage artifact; also cross-references the mandatory Coverage Verification policy finding recorded in `policy-audit.2026-08-10T23-45.md` | Remove the script from the committed tree (its logic and full output are already durably captured in `evidence/baseline/duplicate-sweep.2026-08-10T22-31.md`); if retained, run PoshQC format/analyze, add a Pester test, and produce `artifacts/pester/powershell-coverage.xml` | `.claude/rules/powershell.md` mandates format -> analyze -> test for every `**/*.ps1` file with no evidence-tree carve-out; `general-unit-test.md` requires >=85%/>=75% coverage per language with changed files | `git diff --numstat` shows the file as newly added (27 insertions); `artifacts/pester/` does not exist in the worktree |
| Minor | `docs/.../evidence/baseline/duplicate-sweep.ps1` | whole file (lines 1-27) | Script is a bare top-level script with no `[CmdletBinding()]`, no advanced function, and no parameter validation; also has no `try`/`catch` around `Get-Content -Raw`/`[xml]` casts, so a missing or malformed input file throws an unhandled terminating error rather than a clear diagnostic | Wrap the sweep logic in an advanced function (or, per the Blocking finding above, remove the script entirely) | `.claude/rules/powershell.md` "Coding Standards": prefer advanced functions with `CmdletBinding()`; general-code-change.md "Fail fast and explicitly" | Direct read of the script contents |
| Minor | `docs/.../spec.md` | `## Root Cause Analysis` -> "Duplicate Sweep Result" table | Table states `Analyzer` count = 9, `Reference` count ~= 114, and `packages.config` count ~= 99; the actual captured sweep evidence (`evidence/baseline/duplicate-sweep.2026-08-10T22-31.md`) reports `Analyzer Total=11`, `Reference Total=126`, `packages.config Total=105` | Update the table's precise `Analyzer` figure (stated without an approximation qualifier, unlike `Reference`/`packages.config`) to match the captured evidence, or add a `~` qualifier consistently | Does not change the duplicate-finding conclusion (zero duplicates in either count for these item types), but a precise, unqualified count that does not match its own cited evidence is a documentation-accuracy defect | Side-by-side comparison of `spec.md` lines 158-169 against `evidence/baseline/duplicate-sweep.2026-08-10T22-31.md` |
| Informational | `UtilitiesCS.Test/UtilitiesCS.Test.csproj` | line 356 (removed) | The deletion is exactly one line, correctly identifies the second (redundant) of the two identical `<Compile Include>` items, and leaves the first occurrence (line 304) and all surrounding lines untouched | None — this is the correct fix | Matches spec.md's Proposed Fix design exactly; confirmed no reordering/line-ending churn via `git diff` | `git diff origin/epic/build-ci-coverage-gate-fidelity-integration...HEAD -- UtilitiesCS.Test/UtilitiesCS.Test.csproj` |
| Informational | Evidence artifacts (all) | `evidence/baseline/`, `evidence/qa-gates/`, `evidence/regression-testing/` | All command-bearing artifacts carry `Timestamp:`, `Command:`, `EXIT_CODE:`, and `Output Summary:` fields; no `EXIT_CODE: SKIPPED` occurrences; fail-before and post-fix builds use the identical `/t:Rebuild` command | None | Meets `evidence-and-timestamp-conventions` schema requirements | Direct read of each artifact |

## Design and API Impact

Not applicable — no class, method, interface, or public API is added, removed, or changed. This is a build-configuration item-list edit.

## Overall Assessment

The bugfix itself is correctly scoped, minimal, and well-evidenced. The one blocking finding is a policy/toolchain-compliance issue caused by an unplanned decision to commit a helper script into the evidence tree, not a defect in the production fix. See `policy-audit.2026-08-10T23-45.md` Section 4 and `remediation-inputs.2026-08-10T23-45.md` for the required remediation.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Code Review — utilitiescs-test-cs2002-duplicate-compile-entry-394 (Reaudit — Remediation Cycle 1 Exit Gate)

- **Issue:** #394
- **Branch:** `bug/utilitiescs-test-cs2002-duplicate-compile-entry-394` (HEAD `f39c6fc9`)
- **Base:** `origin/epic/build-ci-coverage-gate-fidelity-integration`
- **Timestamp:** 2026-08-11T04-05
- **Entry review (unmodified, retained):** `code-review.2026-08-10T23-45.md`

## Executive Summary

This reaudit evaluates only the delta introduced by remediation cycle 1 (commits `2a2116eb`, `f39c6fc9`), since the entry review already found the underlying CS2002 fix itself free of quality issues (confirmed unregressed this cycle — see policy-audit reaudit Section 2). The remediation cycle: (1) removes the previously-flagged committed PowerShell helper script, resolving both blocking findings the entry review raised against it; (2) corrects three stale numeric figures in `spec.md`'s duplicate-sweep table; (3) adds an `analyzer-not-applicable` evidence artifact for parity with sibling determinations; and (4) revises one remediation-plan task's acceptance wording mid-execution to accommodate a legitimate subagent memory-index byproduct. No new quality defect is introduced.

## Findings Table

| Severity | File | Location | Finding | Recommendation | Rationale | Evidence |
|---|---|---|---|---|---|---|
| Informational | `docs/.../evidence/baseline/duplicate-sweep.ps1` | (deleted) | The entry review's two findings against this file (Blocking: no coverage artifact/toolchain run; Minor: no `[CmdletBinding()]`, no error handling) are both resolved by deletion rather than remediation of the script itself | None — deletion is the correct proportionate response for a one-off audit helper whose logic and full output are already durably captured verbatim in `evidence/baseline/duplicate-sweep.2026-08-10T22-31.md` | `general-code-change.md`'s "temporary throwaway scripts" allowance; avoids an unfunded Pester/PoshQC/coverage-tooling obligation for a script with no lasting reuse value | `git diff --name-only` shows zero `.ps1` files in the current diff; file confirmed absent from disk |
| Resolved | `docs/.../spec.md` | `## Root Cause Analysis` -> "Duplicate Sweep Result" table | The entry review's Minor finding (Analyzer=9 vs actual 11, Reference=~114 vs actual 126, packages.config=~99 vs actual 105) is corrected | None — figures now read `Analyzer`=11, `Reference`=126, `packages.config`=105, matching `evidence/baseline/duplicate-sweep.2026-08-10T22-31.md` exactly | Documentation accuracy; independently re-verified numerically in this reaudit's policy-audit Section 5 | `evidence/qa-gates/post-remediation-spec-table.2026-08-10T23-45.md`; direct read of current `spec.md` lines 160-169 |
| Resolved | `docs/.../evidence/qa-gates/analyzer-not-applicable.2026-08-10T23-45.md` | new file | The feature-audit's non-blocking observation (no dedicated analyzer-not-applicable artifact for evidentiary parity with the CSharpier/nullable determinations) is addressed | None — artifact added, states the zero-`.cs`-files rationale, cross-references the two sibling determinations | Evidentiary parity | Direct read of the artifact |
| Informational | `remediation-plan.2026-08-10T23-45.md` | `[P2-T2]` | Acceptance clause revised mid-execution from a blanket "no path outside `<FEATURE>/` other than the csproj" to four explicit clauses (a)-(d), carving out `.claude/agent-memory/**` while re-affirming (and making explicit) the `CLAUDE.md`/`.claude/rules/`/`scripts/`/`.cs`/other-`.csproj`/coverage-threshold exclusions and, critically, stating the `.ps1`/`.psm1`/`.psd1` check is exempt from no carve-out | None — the revision is judged legitimate; it does not weaken any check this remediation cycle exists to enforce | Trigger was a genuine, verified two-line append to a pre-existing feature-review subagent memory file — a delegation byproduct, not a scope violation | Diff of `2a2116eb` vs `f39c6fc9` versions of the plan file; `git diff` of the memory-index file itself (2 insertions, 0 deletions) — see policy-audit reaudit Section 3 |
| Informational | `UtilitiesCS.Test/UtilitiesCS.Test.csproj` | (unchanged this cycle) | No line of this file is touched by the remediation cycle; the single-line deletion from the original fix (`f58f8474`) remains intact and unregressed | None | Confirms the remediation cycle did not reopen or perturb the underlying fix | `git diff origin/epic/build-ci-coverage-gate-fidelity-integration...HEAD -- UtilitiesCS.Test/UtilitiesCS.Test.csproj` shows `1 -`, `0 +` |
| Informational | Remediation-cycle evidence artifacts (all) | `evidence/other/`, `evidence/qa-gates/`, `evidence/remediation-baseline/` | All 15 remediation-plan tasks' artifacts carry `Timestamp:`, `Command:`, `EXIT_CODE:`, and `Output Summary:` fields; no `EXIT_CODE: SKIPPED` occurrences; `[P2-T2]`'s evidence file explicitly documents its own superseding re-run after the mid-execution revision rather than silently replacing the earlier capture | None | Meets `evidence-and-timestamp-conventions` schema requirements; the explicit "supersedes" framing is good practice for auditability | Direct read of each artifact |

## Design and API Impact

Not applicable — no class, method, interface, or public API is added, removed, or changed by this remediation cycle. The cycle is documentation/evidence-tree maintenance plus one file deletion (a non-production PowerShell helper).

## Overall Assessment

The remediation cycle correctly and proportionately closes the entry review's blocking finding by removing the unplanned PowerShell helper script rather than building out unfunded toolchain/coverage infrastructure for it. The three stale-figure corrections in `spec.md` are numerically accurate. The one mid-execution acceptance-wording revision is judged legitimate on inspection of both the original and revised text: it narrows the exclusion to exactly the byproduct that triggered it while explicitly re-stating (and strengthening, via explicit non-exemption) the substantive checks the original clause protected. No new code-quality or policy-compliance defect is introduced by this cycle.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Timestamp: 2026-08-10T22-31

Command: `pwsh -NoProfile -Command "& 'C:\Program Files\Microsoft Visual Studio\18\Community\Common7\IDE\Extensions\TestPlatform\vstest.console.exe' UtilitiesCS.Test\bin\Debug\UtilitiesCS.Test.dll /InIsolation '/TestCaseFilter:FullyQualifiedName~PercentageFormatterTests'"`

EXIT_CODE: 0

Output Summary:
```
Passed FormatPercent_Zero_ReturnsZeroPercent [46 ms]
Passed FormatPercent_One_ReturnsHundredPercent [< 1 ms]
Passed FormatPercent_TypicalValue_RoundsToWholePercent [< 1 ms]
Passed FormatPercent_RoundsDownBelowMidpoint [< 1 ms]
Passed FormatPercent_AtMidpoint_RoundsAwayFromZero [< 1 ms]
Passed FormatPercent_SmallMidpoint_RoundsAwayFromZero [< 1 ms]
Passed FormatPercent_Null_ReturnsEmptyString [< 1 ms]

Test Run Successful.
Total tests: 7
Passed: 7
```

Baseline scoped test count against the (still-duplicated) pre-fix rebuilt assembly (`UtilitiesCS.Test\bin\Debug\UtilitiesCS.Test.dll`, produced by the P0-T9 `/t:Rebuild`) is exactly 7 tests, all passed, matching the spec's documented count of 7 `[TestMethod]` members in `PercentageFormatterTests.cs`. This confirms the duplicate `<Compile>` item does not change the number of discoverable tests (it only causes the file to be passed to `csc.exe` twice).
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Timestamp: 2026-08-10T22-31

Command: `pwsh -NoProfile -File "docs\features\active\2026-08-10-utilitiescs-test-cs2002-duplicate-compile-entry-394\evidence\baseline\duplicate-sweep.ps1"`

The sweep script (`duplicate-sweep.ps1`, stored alongside this artifact for reproducibility) XML-parses `UtilitiesCS.Test\UtilitiesCS.Test.csproj` and groups `Include` attribute values by item type (`Compile`, `EmbeddedResource`, `None`, `Reference`, `ProjectReference`, `BootstrapperPackage`, `Analyzer`, `AdditionalFiles`), and separately parses `UtilitiesCS.Test\packages.config` and groups `<package>` `id` attribute values.

EXIT_CODE: 0

Raw output:
```
ItemType=Compile Total=452 DuplicateIncludeValues=2
DUPLICATE: 'OutlookObjects\Folder\PercentageFormatterTests.cs' x2
ItemType=EmbeddedResource Total=1 DuplicateIncludeValues=0
ItemType=None Total=7 DuplicateIncludeValues=0
ItemType=Reference Total=126 DuplicateIncludeValues=0
ItemType=ProjectReference Total=2 DuplicateIncludeValues=0
ItemType=BootstrapperPackage Total=2 DuplicateIncludeValues=0
ItemType=Analyzer Total=11 DuplicateIncludeValues=0
ItemType=AdditionalFiles Total=1 DuplicateIncludeValues=0
packages.config Total=105 DuplicateIds=0
```

Output Summary: The sweep confirms exactly one duplicate `Include` value in the entire project file: `Compile` / `OutlookObjects\Folder\PercentageFormatterTests.cs`, appearing twice (lines 304 and 356, per P0-T7). Zero duplicates were found in every other item type (`EmbeddedResource`, `None`, `Reference`, `ProjectReference`, `BootstrapperPackage`, `Analyzer`, `AdditionalFiles`) and zero duplicate `id` values in `packages.config`.

Note on the `DuplicateIncludeValues=2` label for `Compile`: this is a PowerShell array-unwrapping artifact, not a second duplicate group. When `Where-Object` returns exactly one matching `Group-Object` result, PowerShell unwraps the single-element array to a scalar `GroupInfo` object, so `.Count` on the (would-be) array instead returns that group's own `.Count` property — the occurrence count of the one duplicate value (2), not the number of distinct duplicate values (1). The enumerated `DUPLICATE:` lines are the authoritative per-group finding and confirm exactly one duplicate `Include` value in `Compile` and none elsewhere; `Total=452` for `Compile` matches the plan's stated baseline `<Compile Include=` count. Verified independently: for item types with zero duplicates, PowerShell's `$null.Count` evaluates to `0`, which is why those lines correctly read `DuplicateIncludeValues=0`.

This confirms the plan's acceptance criterion for P0-T8: exactly one duplicate (`Compile` / `PercentageFormatterTests.cs`) and zero duplicates in every other item type and in `packages.config`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Timestamp: 2026-08-10T22-31

Command: `pwsh -NoProfile -Command "& 'C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe' UtilitiesCS.Test\UtilitiesCS.Test.csproj /t:Rebuild /p:Configuration=Debug /p:Platform=AnyCPU"`

EXIT_CODE: 0 (Build succeeded — CS2002 is a warning, not a build failure)

Output Summary: `/t:Rebuild` forced a genuine `CoreCompile` for `UtilitiesCS.Test.csproj` and its project-reference dependency chain (`TaskMaster`, `QuickFiler`, `TaskVisualization`, `Tags`, `UtilitiesCS`, `ToDoModel`). Build output contains the literal warning:

```
CSC : warning CS2002: Source file 'C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a267ee5c24c8a630d\UtilitiesCS.Test\OutlookObjects\Folder\PercentageFormatterTests.cs' specified multiple times [C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a267ee5c24c8a630d\UtilitiesCS.Test\UtilitiesCS.Test.csproj]
```

Final summary line: "6 Warning(s), 0 Error(s)". Total of 12 lines in the captured output contain the word "warning" (5 duplicate `System.Reactive.PackagesConfigCheck.targets` packages.config-migration warnings emitted once per dependent project in the chain, plus their MSBuild-log restatement lines, plus the one CS2002 line and its restatement). The CS2002 substring appears exactly 2 times in the raw output (once under the `CoreCompile` target block, once implicitly reiterated as part of the same block's full-path text match) — confirming the pre-change baseline reproduces the CS2002 warning for `PercentageFormatterTests.cs` as required. This satisfies the fail-before evidence requirement: a `/t:Build` capture would not be acceptable per spec.md; this is a genuine `/t:Rebuild`.

Full raw MSBuild output is not persisted verbatim in this artifact (large, includes hundreds of resource-DLL copy lines from the Rebuild of dependency projects); the CS2002 warning line and the build summary line above are the load-bearing excerpts and were verified directly against the captured command output.
Loading