Skip to content

Hand on the reporting classification with the infectious-agent code sets - #185

Merged
Brar merged 4 commits into
NeoIPC:mainfrom
Brar:feature/infectious-agent-code-set-classification
Nov 9, 2026
Merged

Brar merged 4 commits into
NeoIPC:mainfrom
Brar:feature/infectious-agent-code-set-classification

Conversation

@Brar

@Brar Brar commented Nov 8, 2026 •

Copy link
Copy Markdown
Member

What This Changes

Get-NeoIPCInfectiousAgentCodeSet now also reads the reporting classification beside the list
(NeoIPC-Infectious-Agent-Reporting-Groups.yaml), or the file -ClassificationPath names, and returns, for every
Id, its reporting group and the value of each of the classification's attributes that the node takes (its Gram
reaction and oxygen requirement), resolved through the very functions the classification's own checks use, with the
classification's SHA-256. Export-NeoIPCInfectiousAgentCodeSet writes them into its JSON.

neoipcr is moving from its legacy tables to the released infectious-agent list and its reporting classification, and
its parity test holds its own reading of both to the toolkit's through this export. The infectious-agents release
0.2.0-alpha follows this change, since its CHANGELOG entry belongs to that release; the list's YAML is not touched.

Refusals

  1. A file that is no reporting classification, such as an empty file, the list itself, or one whose Groups hold no
    group, is refused, with the file named.
  2. An attribute that is no mapping or has no Key is refused, and so is one whose Key is no text with a word in it,
    would take the place of another column of a row, in any letter case, or turns into a name PowerShell reserves,
    such as PSObject or PSTypeName.
  3. The export refuses to write over the classification, by its path or by its bytes, as it refuses the list.
  4. Both files are read through one helper, Read-NeoIPCTextFile, which refuses a byte order mark, invalid UTF-8, and
    a path that names no readable file. A file that is not valid YAML is refused with the line and the column where
    the parser stopped, or the end of the file; neither that refusal nor the refusal of an unreadable file carries
    PowerShell's wrapper around the .NET exception.
  5. A path on a drive of a provider other than the file system, whose provider path .NET would take for a file
    relative to the process's directory, or a path that cannot be resolved, is refused wherever these cmdlets, the
    text-file reader, and the module's write point hand a path to .NET, and a refused path is never handed on.
  6. Both cmdlets, and Add-NeoIPCInfectiousAgentUid, end at any refusal or failure, whatever the caller's error
    preference, so an unparsable or unreadable file leaves no partial export behind, and a caller that silences errors
    can no longer make the export write over the list it has just refused to replace, or make
    Add-NeoIPCInfectiousAgentUid write a sidecar it has refused.

Testing

  1. Tests/InfectiousAgents.Tests.ps1: 106 tests pass on Windows, among them the cases for each refusal, a relative
    -ClassificationPath beside a decoy in the process directory, directory paths, an unreadable classification,
    drives of other providers and drives that do not exist, and child-process runs under Continue and
    SilentlyContinue; Tests/RelativePathWrites.Tests.ps1: 11, among them the module's write point refusing a path
    outside the file system, under SilentlyContinue too. The new tests were shown failing first.
  2. 34 mutants of the first commit and 27 of the second, each caught by a test.
  3. The whole NeoIPC-Tools suite: 2,666 passed, 0 failed, 1 skipped (the check that R is installed).
  4. PSScriptAnalyzer reports nothing new, and the text-hygiene gate passes.

For Reviewers

The documentation changes are the module README's rows for both cmdlets and for the text-file helpers, the
infectious-agent list's README and CHANGELOG, the metadata generation design note's paragraph on the code sets, and
Convert-InfectiousAgentList.ps1's help, which names the classification its directory must hold.

Get-NeoIPCInfectiousAgentCodeSet now also reads the reporting
classification beside the list, or the file -ClassificationPath names,
and returns one row per Id with the reporting group and the value of
each of the classification's attributes that the node takes, resolved
as the classification's checks resolve them, with the classification's
SHA-256. Export-NeoIPCInfectiousAgentCodeSet writes them into its JSON,
so that another language's reading of the classification, such as
neoipcr's, can be held to the toolkit's resolution; an attribute whose
Key would take the place of another column of a row, in any letter
case, is refused.

The export refuses to write over the classification, by its path or by
its bytes, as it refuses the list, and both files are read through one
helper that refuses a byte order mark, invalid UTF-8, and a path that
names no readable file. Both cmdlets now end at any refusal or failure,
whatever the caller's error preference, so that an unparsable or
unreadable file never leaves a partial export behind, and a caller that
silences errors can no longer make the export write over the list it
has just refused to replace.
Copilot AI balanced review requested due to automatic review settings November 8, 2026 18:43

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Copilot AI 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.

🔵 Needs a closer look

The review can verify the PowerShell producer but not neoipcr’s compatibility with the new Classification/ClassificationSha256 schema and null-handling behavior.

No open issues 🎉

Review effort: Lite

@Brar Brar left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review at 065c422

The change does what it says on the committed files: 3,453 rows, one per Id, with no finding from the structure, member, or attribute checks, and an independent nearest-placement walk over the list agrees with every row's group and both attributes. The refusals in the description hold as probed (below). The findings are about what the rows do with a file that is not what it should be, and about tests that pass for the wrong reason.

Findings (10 low, 6 nits)

  1. low: a file that is no classification (an empty file, a stray mapping, the list itself) is taken as one; every row comes back without a group, and without the attribute columns.
  2. low: the rows repeat the checks' resolution rather than sharing it, and no test ties the two together.
  3. low: "cannot be read into rows" also wraps failures from the list, naming the classification.
  4. low: '' rather than $null for a missing Value or group Key; a key-less group's member hands on its own Value.
  5. low: a path on a drive of another provider is checked against the location and read from the process directory.
  6. low: the YAML refusals keep the Exception calling "Load" wrapper and drop the line and column.
  7. low: no test for a clash between two attributes in different letter case; a case-sensitive mutant survives.
  8. low: the two ends at a <Case> child-process runs pass when the child never reaches the cmdlet.
  9. low: no test for -ClassificationPath on a PowerShell drive; a .ProviderPath → .Path mutant survives.
  10. low: the help's list of refusals leaves out the attribute without a Key.
  11. nit: the "cannot be read" refusal keeps the ReadAllBytes wrapper.
  12. nit: a 186-column help line, with a clause attached to the wrong noun.
  13. nit: the Ids 0 and 3561 come out $null only in a classification that passes its checks.
  14. nit: a whitespace or non-text Key is handed on under a made-up name.
  15. nit: a test name promises rows it does not assert.
  16. nit: Convert-InfectiousAgentList.ps1 now needs a classification it does not use.

Checked and holding

  1. The test file. Tests/InfectiousAgents.Tests.ps1 on Linux (Pester 6.2.0, powershell-yaml 0.4.12): 71 tests, 69 passed, 2 skipped as root (the unreadable classification, which root can read, and the Windows-only letter-case test). Run without root's file-access override, as CI's runner is: 70 passed, 1 skipped, and the unreadable-classification test goes red without the try around ReadAllBytes. Against main's production files, every new test fails (34 passed, 35 failed).
  2. The whole NeoIPC-Tools suite, excluding the yaml12 and ruamel tags as CI does: 2,629 tests at this head (2,576 passed, 48 skipped, 2 not run, 3 failed) against 2,597 at the base. The same three fail at both, for reasons of this environment (two need a .git directory, one an R summary table), and the one extra skip is the root-only test. PSScriptAnalyzer gives the same 8 diagnostics at base and head; the 8 changed files are LF, UTF-8 without BOM, and git diff --check is clean.
  3. Refusal 4. Missing, BOM, invalid-UTF-8, directory, keyless-attribute, and invalid-YAML classifications, and the list-path, classification-path, and identical-bytes refusals, each end the call with their message and write nothing, under a global SilentlyContinue or Ignore, under -ErrorAction SilentlyContinue or Ignore on the call, and under $PSDefaultParameterValues['*:ErrorAction']. The comment at lines 213 to 215 holds: without the try around the classification parse, the export writes under the caller's Continue.
  4. Refusal 2. The path check refuses the output as the classification by absolute, relative, sub/.., .//, and drive path; the byte check refuses it through a symbolic link, a symlinked directory, and an identical copy. Directories named w[1], w*x, w?y, and w`z work throughout, and a directory given as list or classification is refused as "no file".
  5. JSON shape. A list without Ids gives "Classification": [], a single Id an array of one, $null stays JSON null, Ids are numbers, and the rows sort numerically.
  6. Mutants. The description's 34 are caught. Of the further mutants tried, those in items 7 and 9 survive and matter; the other survivors are equivalent, or touch only a case the structure check reports.
  7. CI at this head: every check green (release-assets skipped).

Question

The description says neoipcr's parity test holds its reading to this export, but neoipcr's main (c6de50b13254b162484427adeeffe08b3d98331e) has no reader of this JSON yet, so until that test lands, the cmdlets' own refusals are the only guard against item 1. Will it read the committed classification itself, or the file the export was given, and will it run against a released list? infectious-agents-v0.1.0-alpha ships no classification, so Get and Export now refuse that list, as the README says.

This comment is the sign-off; a formal Approve is not available on one's own pull request. No merge approval from me, and I hold no instruction to merge this.

🤖 Reviewed by Claude Code


Generated by Claude Code

…what no row can come from

The code sets' rows now come from the very resolution the classification's
member and attribute checks judge, built by one function for both, so the
two cannot drift apart. Each row takes its Id as the list's walker reads
it, so that a form of an Id the walker accepts is never blamed on the
classification; it hands on null where the classification leaves a group
without a Key or an assignment without a Value; and a member always takes
its group's Key, never a Value of its own.

Get-NeoIPCInfectiousAgentCodeSet refuses a file that is no reporting
classification, such as an empty file, the list itself, or one whose Groups
hold no group, which would have handed on every Id in no group or every row
without the attributes' columns; and an attribute that is no mapping, whose
Key is no text with a word in it, or whose Key turns into a name PowerShell
reserves, PSTypeName among them, which took a row's type name in place of
its column.

A path on a drive of a provider other than the file system, whose provider
path .NET takes for a file relative to the process's directory, is refused
wherever the infectious-agent cmdlets, the text-file reader, and the
module's write point hand a path to .NET: an output path on the Variable
drive had written a file into the process's directory. A path that cannot
be resolved, such as one on a drive that does not exist, is refused with
its reason, and a refused path is never handed on, so that nothing is
written where a caller's SilentlyContinue drops the refusal.
Add-NeoIPCInfectiousAgentUid runs under Stop, as the code-set cmdlets do,
so that its refusals end the call whatever the caller's preference: under
SilentlyContinue it had written the sidecar it refused.

A refusal of a file that is not valid YAML names the line and the column
where the parser stopped, or the end of the file, and neither it nor the
refusal of a file that cannot be read carries PowerShell's wrapper around
the .NET exception. The help lists every refusal, and
Convert-InfectiousAgentList.ps1's help names the classification its
directory must hold.
@Brar

Brar commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

All sixteen threads are addressed in 23a728f and resolved.

Beyond the threads, the same commit:

  1. refuses a path on a drive of another provider wherever these files hand a path to .NET: besides the two readers of item 5, the export's own resolution of the list and of -OutputPath, the UID sidecar of Add-NeoIPCInfectiousAgentUid, and Write-NeoIPCTextFile, the module's write point. At 065c422, Export-NeoIPCInfectiousAgentCodeSet -OutputPath 'Variable:\x' wrote a file x into the process's directory without a word;
  2. runs Add-NeoIPCInfectiousAgentUid under Stop, as the code-set cmdlets run: under a caller's SilentlyContinue it dropped its refusals and wrote the sidecar anyway, the one it refused for a row whose id is no Id (Give every infectious-agent option a random UID, drawn once and kept #179) as well as one on another provider's drive;
  3. answers the one finding Copilot's review of 065c422 stored but did not post (its run log keeps only Comment stored (id=001, type=bug, …, start=708, end=708, severity=moderate), so its text is not recoverable). Line 708 skipped an attribute that is no mapping; read as that skip dropping the attribute's column from every row without a word, it is fixed: such an attribute is now refused like one without a Key, with a test shown red first.

The four other places in the module that hand a path to .NET in the same way (the release check's read of the list's CHANGELOG, the country plan's two, and the onboarding's metadata directory) lie outside this change and are recorded with the module's other path defects.

On the question: neoipcr's parity test reads neither YAML file. Its data-raw/sysdata.R fetches the list and the classification at the pinned commit and, once the release exists, at the tag infectious-agents-v0.2.0-alpha, stops unless both have their pinned SHA-256s, and bundles the tables it derives, with those SHA-256s as their provenance. The test then holds the bundled tables to tests/testthat/fixtures/infectious-agent-code-sets.json, which this export writes, and requires the fixture's Sha256 and ClassificationSha256 to equal the bundle's, so the export must have been given the very files the bundle was built from. It runs against a released list: 0.2.0-alpha, the first whose release bundle carries the classification (build.yml's list bundle, since #184). Nothing pins 0.1.0-alpha; until the tag is cut, neoipcr's branch, not yet pushed, builds from this repository's 8c69747 under --untagged, which its CI never passes. Until that test lands, the cmdlets' refusals are the guard, as you say, and item 1's refusal now covers the case you found.

Tests: InfectiousAgents.Tests.ps1 106 passed, each new test shown red first; RelativePathWrites.Tests.ps1 11; InfectiousAgentReportingGroupValidation.Tests.ps1 125 and MetadataContent.Tests.ps1 17 as before; 27 mutants of this round, each caught; the whole NeoIPC-Tools suite 2,666 passed, 0 failed, 1 skipped (the check that R is installed). PSScriptAnalyzer reports nothing new on the changed files.

Copilot AI lite review requested due to automatic review settings November 9, 2026 06:16

Copilot AI 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.

🔵 Needs a closer look

Human confirmation is appropriate because the change relies on Windows-specific provider resolution, YAML parser exception details, and cross-process error-preference behavior that cannot be independently reproduced here.

No open issues 🎉

Review effort: Lite

The entry for Get-NeoIPCInfectiousAgentCodeSet promised a reporting group,
a Gram reaction, and an oxygen requirement for every Id. It now says, as
the list's README does, that the attributes concern the bacteria, that a
value is null where the classification gives none, and that the cmdlet
reads the classification beside the list unless -ClassificationPath names
another and does not run without one.
@Brar

Brar commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

Copilot's review of 23a728f posted no comment, but its run log (run 42572018829) stored two moderate findings without keeping their text:

  1. Comment stored (id=001, type=bug, file=scripts/modules/NeoIPC-Tools/Private/InfectiousAgentReportingGroupValidation.ps1, start=765, end=768, …): the check that refuses an attribute whose name PowerShell reserves. It was probed with a one-property row and with a whole row, as the rows are built, on names that could trip it: wildcard characters (Gram[x], Gram*, Gram?), member-set and method names (PSStandardMembers, ToString, GetType, Count, Item, Keys, Values), PSComputerName, and names with a backtick, a dot, or a space, each kept as a property, and PsObject, PsAdapted, PsTypeNames, and PsTypeName, each refused. No defect turned up there, so nothing changed; without the finding's text, what it saw cannot be told.
  2. Comment stored (id=001, type=discrepancy_with_pr_description, file=metadata/common/infectious-agents/CHANGELOG.md, start=30, end=32, …): the CHANGELOG's entry for the code sets promised a reporting group, a Gram reaction, and an oxygen requirement for every Id, and said nothing of the classification's path or that the cmdlet needs one, where the description and the list's README do. Fixed in 0acd719: the entry now says that the attributes concern the bacteria, that a value is null where the classification gives none, and that the cmdlet reads the classification beside the list unless -ClassificationPath names another and does not run without one.

All checks passed on 23a728f; MetadataContent.Tests.ps1 (17) and the text-hygiene gate pass on 0acd719.

Copilot AI lite review requested due to automatic review settings November 9, 2026 06:27

Copilot AI 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.

🟢 Approval recommended

The supplied review records no active blocking issues, with the remaining moderate parity-test limitation not treated as an unresolved approval blocker.

No open issues 🎉

Review effort: Lite

The test on the committed files held every node to a leaf group and to
values that exist, which a resolution placing nodes in the wrong ones would
still pass. Six well-known infectious agents, their groups and values read
from the two files by hand, now pin what the code sets hand on:
Staphylococcus aureus, Bacteroides fragilis, Escherichia coli, Pseudomonas
aeruginosa, Candida albicans, and Streptococcus agalactiae.
@Brar

Brar commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

Copilot's review of 0acd719 posted no comment, but its run log (run 42573152709) stored one moderate finding without keeping its text: Comment stored (id=001, type=test_coverage, file=scripts/modules/NeoIPC-Tools/Tests/InfectiousAgents.Tests.ps1, start=789, end=791, …), the test that holds the code sets of the committed files to the two files, which its overview calls "the remaining moderate parity-test limitation".

Read as that test checking only that every node takes a leaf group and values that exist, it is addressed in 4d7ecfb: a mutant that gives every node the first leaf group's Key passed that test, and a new test now pins six well-known infectious agents to the groups and values read from the two files by hand (Staphylococcus aureus, Bacteroides fragilis, Escherichia coli, Pseudomonas aeruginosa, Candida albicans, and Streptococcus agalactiae), which that mutant turns red. InfectiousAgents.Tests.ps1: 107 passed.

Copilot AI lite review requested due to automatic review settings November 9, 2026 06:38

Copilot AI 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.

🟢 Approval recommended

No active blocking issues remain, and the change includes expanded validation and test coverage supporting approval.

No open issues 🎉

Review effort: Lite

@Brar

Brar commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

Copilot's review of 4d7ecfb posted no comment, but its run log (run 42574271430) stored one moderate finding without keeping its text: Comment stored (id=001, type=bug, file=scripts/modules/NeoIPC-Tools/Private/InfectiousAgentReportingGroupValidation.ps1, start=59, end=63, …), the check in Resolve-NeoIPCReportingClassificationPath that the classification's path names a file before it is resolved.

It was probed through Get-NeoIPCInfectiousAgentCodeSet -ClassificationPath with paths that could trip it: a hidden file, a provider-qualified path, and a name with brackets are read; a path with a trailing separator is refused because it cannot be read; Env:\PATH is refused as being on the Environment provider's drive; a registry key and a directory are refused as no file; and a drive that does not exist is refused as not existing. Together with the cases the tests hold (relative paths, a file-system drive, the Alias drive, a missing file), no defect turned up, so nothing changed; without the finding's text, what it saw cannot be told.

@Brar
Brar merged commit cc1638e into NeoIPC:main Nov 9, 2026
9 checks passed
@Brar
Brar deleted the feature/infectious-agent-code-set-classification branch November 9, 2026 07:34
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.

2 participants