Correct HI:2.0.2 value definitions to match the DT_HI:1.0.0 decision table - #1254
Open
sandakelum97 wants to merge 1 commit into
Open
sandakelum97 wants to merge 1 commit into
sandakelum97 wants to merge 1 commit into
Conversation
sandakelum97
requested review from
ahouseholder and
sei-vsarvepalli
as code owners
September 19, 2026 04:46
Author
|
Attaching the verification script referenced above. It takes no arguments and no dependencies beyond the repo itself: It parses the four HI:2.0.2 value definition strings and evaluates them against all sixteen cells of DT_HI:1.0.0, exiting non-zero on any disagreement, no-match, or ambiguous cell. On main it reports 5 of 16 failing; on this branch, all 16 agree. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Corrects the
Low,MediumandHighvalue definitions ofssvc:HI:2.0.2insrc/ssvc/decision_points/ssvc/human_impact.pyso that they describe the same mapping as thessvc:DT_HI:1.0.0decision table.Very Highwas already correct and is unchanged.No decision table, mapping or outcome is modified. This is a documentation correction only: the table's logic is authoritative and stays exactly as it is, per @sei-vsarvepalli's comment on #1227.
The regenerated files under
data/are included, produced by the project's doctools regeneration step, as required for fork pull requests by.github/workflows/run_doctools.yml.Why
As set out in #1218 and #1227, the published prose and the published decision table disagreed in four of the sixteen Safety Impact x Mission Impact cells, and a fifth combination (
Critical+MEF Failure) matched none of the four value definitions at all, so it had no defined value in the text.@ahouseholder's root cause note on #1218 describes the sequence: the documentation was written, the decision table was implemented, the table was corrected during implementation, and the documentation was not updated. This pull request completes that last step.
Two secondary points worth recording:
Crippledis not a value name. The previous definitions usedCrippledas shorthand. Thessvc:MI:2.0.0value is namedMEF Support Crippled. The corrected text uses the full value name, so the definitions can be checked mechanically against the value enumerations. Happy to revert to the shorthand if the shorter form is preferred for readability.Nonewas already removed in 2.0.2. The2.0.1to2.0.2bump removed references to the retiredssvc:MIvalueNone. That change did not touch the cells in question, so this discrepancy predates2.0.2and is present in2.0.1as well.2.0.1is left unchanged here as historical record.Verification
I wrote a short script that parses the four value definition strings and evaluates them against all sixteen cells of
ssvc:DT_HI:1.0.0. It does not hand-encode the logic, so it tests the published text rather than a reader's interpretation of it. Happy to share it if useful.Before this change, five cells fail:
After this change, all sixteen cells agree, and no cell satisfies more than one value definition.
Also run locally:
black@25.1.0 --checkon the modified file: cleandefinitionstrings andgeneratedAttimestamps changepytest: full suite passesOne question, not addressed by this PR
Closing these two issues will also close the open question raised at the end of #1227, so recording it here.
Under
DT_HI:1.0.0, Safety ImpactMarginalproduces the same Human Impact asNegligiblefor every Mission Impact value. Safety Impact only changes the Human Impact outcome atCriticalorCatastrophic, so a four-value decision point behaves as three values in the deployer tree. This correction makes that visible in the text rather than changing it.If that collapse is intended, no action is needed. If it is not, it is a separate question about the table rather than the prose, and I am happy to open a new issue for it so it is not lost when these two close.