feat(requirements): add test<->requirement reconciliation layer#482
Open
jkenyon-nvidia wants to merge 1 commit into
Open
feat(requirements): add test<->requirement reconciliation layer#482jkenyon-nvidia wants to merge 1 commit into
jkenyon-nvidia wants to merge 1 commit into
Conversation
Adds a Requirements to Tests Matrix which will allow us to trace from various requirements sources (which we will consolidate more of over time) to our main test-plan.yaml, and then trace through that to the individual tests. docs/requirements/ - offtake-requirements.yaml/.md: in-repo offtake listing - software-reference-requirements.yaml/.md: backfills requirements from the NCP Software Reference Guide (NSRG). - test-requirements-matrix.yaml: the junction index, rendered to a flat (sheet-friendly) AsciiDoc traceability matrix that can be viewed directly in the github UI. - README.md: the reconciliation policy (canonical authority, ID/naming + prefix registry, continue-numbering collision policy, legacy_ids lifecycle, and a new-source onboarding runbook). Tooling - scripts/reqtrace.py: `validate` resolves both endpoints of every mapping edge (test in plan, req in its source listing) with uniqueness checks, plus a `coverage` report of untested requirements. Wired into `make reqcheck` + a unit test; `make plan` renders the listings and traceability matrix. Signed-off-by: John Kenyon <jkenyon@nvidia.com>
📝 WalkthroughWalkthroughThe PR adds requirement source YAMLs and Markdown exports, a test-to-requirement matrix with renderers, a requirements validation CLI, and Makefile targets for rendering and validation. ChangesRequirements traceability pipeline
Sequence Diagram(s)sequenceDiagram
participant MakefileReqcheck as "Makefile reqcheck target"
participant ReqtraceValidate as "scripts/reqtrace.py validate"
participant TestPlan as "docs/test-plan.yaml"
participant MatrixIndex as "docs/requirements/test-requirements-matrix.yaml"
participant RequirementSources as "docs/requirements/*.yaml"
MakefileReqcheck->>ReqtraceValidate: uv run python scripts/reqtrace.py validate
ReqtraceValidate->>TestPlan: load test_id values
ReqtraceValidate->>MatrixIndex: load mappings
ReqtraceValidate->>RequirementSources: discover requirement listings and req_id values
ReqtraceValidate-->>MakefileReqcheck: exit status
🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-06-26 18:10:35 UTC | Commit: 00c7c1f |
abegnoche
approved these changes
Jun 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/requirements_matrix_to_adoc.py (1)
42-46: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize embedded newlines before writing AsciiDoc cells.
Line 46 only escapes
|.notesis explicitly free-form indocs/requirements/test-requirements-matrix.yaml, and test summaries come fromdocs/test-plan.yaml, so a single multi-line value will split a row and corrupt the flat table. Flatten\r?\nhere, like the Markdown renderer already does inscripts/requirements_source_to_md.pyat Lines 42-45.Proposed fix
def esc_adoc(val: Any) -> str: """Stringify `val` and escape the AsciiDoc cell delimiter; '' for None.""" if val is None: return "" - return str(val).replace("|", "\\|") + return str(val).replace("\r", " ").replace("\n", " ").replace("|", "\\|")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/requirements_matrix_to_adoc.py` around lines 42 - 46, The esc_adoc helper currently escapes only the AsciiDoc cell delimiter, so multi-line values can break table rows. Update esc_adoc to also normalize embedded CR/LF newlines into a single-line representation before returning the string, keeping the existing pipe escaping intact. Use the esc_adoc function as the fix point and mirror the newline-flattening behavior already used by the Markdown renderer so notes and test summary fields remain safe for flat AsciiDoc cells.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/requirements/software-reference-requirements.yaml`:
- Around line 343-346: Replace the placeholder TODO requirement descriptions in
the software requirements catalog with concrete acceptance criteria before
treating them as covered. Update the affected requirement entries identified by
their component, description, reference_mapping, and covers_test fields so they
no longer use undefined text, or remove those rows from the catalog until the
requirements are fully defined. Keep the traceability mapping in sync with
docs/requirements/test-requirements-matrix.yaml for entries like VMAAS10-01 and
META01-01 so coverage is only reported for requirements with real descriptions.
In `@scripts/reqtrace.py`:
- Around line 171-189: Reject incomplete mapping rows in reqtrace validation by
making the mapping loop in reqtrace.py treat entries with empty requirements or
missing req_id/source as errors before recording them as mapped. Update the
logic around the mapping iteration and _mapping_edges() so a row only counts
toward mapped_test_ids and mapped_reqs after every requirement endpoint is
validated, and emit explicit errors for missing req_id or source instead of
silently skipping them.
- Around line 130-135: The _mapping_edges function is missing an explicit return
type annotation; update its signature to declare the yielded iterator type for
the tuples it produces. Use the existing tuple contents in _mapping_edges and
annotate the return consistently with the file’s typing style so all functions
in the module have explicit return types.
In `@scripts/requirements_source_to_md.py`:
- Around line 112-117: The render() flow currently defaults any non-offtake YAML
to render_reference, which hides invalid or misspelled source values. Update
render() to validate the parsed doc["source"] explicitly and only dispatch to
render_offtake or render_reference for known values, otherwise fail fast with an
error. Use the existing render() function and the
render_offtake/render_reference symbol names to locate the branching logic and
keep the output from being written when the source is unknown.
---
Nitpick comments:
In `@scripts/requirements_matrix_to_adoc.py`:
- Around line 42-46: The esc_adoc helper currently escapes only the AsciiDoc
cell delimiter, so multi-line values can break table rows. Update esc_adoc to
also normalize embedded CR/LF newlines into a single-line representation before
returning the string, keeping the existing pipe escaping intact. Use the
esc_adoc function as the fix point and mirror the newline-flattening behavior
already used by the Markdown renderer so notes and test summary fields remain
safe for flat AsciiDoc cells.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bd2dee25-7efc-4d00-84a1-93f4e47cb587
📒 Files selected for processing (13)
Makefiledocs/requirements/.gitignoredocs/requirements/README.mddocs/requirements/offtake-requirements.mddocs/requirements/offtake-requirements.yamldocs/requirements/software-reference-requirements.mddocs/requirements/software-reference-requirements.yamldocs/requirements/test-requirements-matrix.adocdocs/requirements/test-requirements-matrix.yamlscripts/reqtrace.pyscripts/requirements_matrix_to_adoc.pyscripts/requirements_source_to_md.pyscripts/tests/test_reqtrace.py
Comment on lines
+343
to
+346
| component: 'Compute Service: VMaaS' | ||
| description: 'TODO: More things from the "NVIDIA Grace I/O Virtualization Guide"' | ||
| reference_mapping: 'NSRG: Performance Requirements / Virtualizing a GPU' | ||
| covers_test: VMAAS10-01 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Replace placeholder requirement descriptions before publishing these as covered requirements.
Line 344 and the other TODO descriptions leave the requirement undefined, but these IDs are already mapped as fully covered in docs/requirements/test-requirements-matrix.yaml (for example VMAAS10-01 at Lines 620-626 and META01-01 at Lines 1013-1019). That makes the new traceability layer report coverage for requirements that still have no acceptance criteria. Fill in concrete requirement text or remove these rows from the catalog until they are defined.
Also applies to: 502-507, 777-800, 939-940
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/requirements/software-reference-requirements.yaml` around lines 343 -
346, Replace the placeholder TODO requirement descriptions in the software
requirements catalog with concrete acceptance criteria before treating them as
covered. Update the affected requirement entries identified by their component,
description, reference_mapping, and covers_test fields so they no longer use
undefined text, or remove those rows from the catalog until the requirements are
fully defined. Keep the traceability mapping in sync with
docs/requirements/test-requirements-matrix.yaml for entries like VMAAS10-01 and
META01-01 so coverage is only reported for requirements with real descriptions.
Comment on lines
+130
to
+135
| def _mapping_edges(mapping: dict[str, Any]): | ||
| """Yield ``(test_id, req_id, source)`` for every edge in the index.""" | ||
| for m in mapping.get("mappings", []): | ||
| tid = m.get("test_id", "") | ||
| for req in m.get("requirements") or []: | ||
| yield tid, req.get("req_id", ""), req.get("source", "") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing return type on _mapping_edges.
This is the only function in the file without an explicit return type. As per coding guidelines, "Return types
must be explicitly specified for all functions".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/reqtrace.py` around lines 130 - 135, The _mapping_edges function is
missing an explicit return type annotation; update its signature to declare the
yielded iterator type for the tuples it produces. Use the existing tuple
contents in _mapping_edges and annotate the return consistently with the file’s
typing style so all functions in the module have explicit return types.
Source: Coding guidelines
Comment on lines
+171
to
+189
| for m in mapping.get("mappings", []): | ||
| tid = m.get("test_id", "") | ||
| if not tid: | ||
| errors.append("mapping: entry with empty test_id") | ||
| continue | ||
| if tid in mapped_test_ids: | ||
| errors.append(f"mapping: duplicate test_id {tid!r}") | ||
| mapped_test_ids.add(tid) | ||
| if tid not in plan_ids: | ||
| errors.append(f"mapping: test_id {tid!r} not in test-plan.yaml") | ||
|
|
||
| for tid, rid, src in _mapping_edges(mapping): | ||
| if not rid: | ||
| continue | ||
| mapped_reqs.add(rid) | ||
| if src not in source_sets: | ||
| errors.append(f"mapping: test {tid!r} cites unknown source {src!r} for req {rid!r}") | ||
| elif rid not in source_sets[src]: | ||
| errors.append(f"mapping: req_id {rid!r} (test {tid!r}) not found in {src!r} listing") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject empty or partial mapping rows.
A mapping entry with requirements: [] or a requirement missing req_id/source currently passes as
"mapped" because Line 178 records the test_id before any endpoint is validated, and _mapping_edges()
silently yields nothing for those cases. That lets a test evade the Line 191 warning while still having no
resolvable requirement edge, which breaks the traceability contract described here and in the matrix renderer's
current schema handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/reqtrace.py` around lines 171 - 189, Reject incomplete mapping rows
in reqtrace validation by making the mapping loop in reqtrace.py treat entries
with empty requirements or missing req_id/source as errors before recording them
as mapped. Update the logic around the mapping iteration and _mapping_edges() so
a row only counts toward mapped_test_ids and mapped_reqs after every requirement
endpoint is validated, and emit explicit errors for missing req_id or source
instead of silently skipping them.
Comment on lines
+112
to
+117
| def render(path: Path) -> None: | ||
| """Render a single source YAML to its sibling `.md`.""" | ||
| doc = yaml.safe_load(path.read_text(encoding="utf-8")) | ||
| renderer = render_offtake if doc.get("source") == "offtake" else render_reference | ||
| out_path = path.with_suffix(".md") | ||
| out_path.write_text(renderer(doc, path.name), encoding="utf-8") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail on unknown source documents instead of silently rendering them as reference.
Line 115 treats every non-offtake YAML as reference. A misspelled or missing source field will therefore generate a valid-looking Markdown file in the wrong schema instead of surfacing the bad input.
Proposed fix
def render(path: Path) -> None:
"""Render a single source YAML to its sibling `.md`."""
doc = yaml.safe_load(path.read_text(encoding="utf-8"))
- renderer = render_offtake if doc.get("source") == "offtake" else render_reference
+ if not isinstance(doc, dict):
+ raise ValueError(f"{path} must contain a single mapping document")
+
+ source = doc.get("source")
+ if source == "offtake":
+ renderer = render_offtake
+ elif source == "reference":
+ renderer = render_reference
+ else:
+ raise ValueError(f"{path} has unsupported source {source!r}")
+
out_path = path.with_suffix(".md")
out_path.write_text(renderer(doc, path.name), encoding="utf-8")
print(f"Wrote {out_path}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def render(path: Path) -> None: | |
| """Render a single source YAML to its sibling `.md`.""" | |
| doc = yaml.safe_load(path.read_text(encoding="utf-8")) | |
| renderer = render_offtake if doc.get("source") == "offtake" else render_reference | |
| out_path = path.with_suffix(".md") | |
| out_path.write_text(renderer(doc, path.name), encoding="utf-8") | |
| def render(path: Path) -> None: | |
| """Render a single source YAML to its sibling `.md`.""" | |
| doc = yaml.safe_load(path.read_text(encoding="utf-8")) | |
| if not isinstance(doc, dict): | |
| raise ValueError(f"{path} must contain a single mapping document") | |
| source = doc.get("source") | |
| if source == "offtake": | |
| renderer = render_offtake | |
| elif source == "reference": | |
| renderer = render_reference | |
| else: | |
| raise ValueError(f"{path} has unsupported source {source!r}") | |
| out_path = path.with_suffix(".md") | |
| out_path.write_text(renderer(doc, path.name), encoding="utf-8") | |
| print(f"Wrote {out_path}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/requirements_source_to_md.py` around lines 112 - 117, The render()
flow currently defaults any non-offtake YAML to render_reference, which hides
invalid or misspelled source values. Update render() to validate the parsed
doc["source"] explicitly and only dispatch to render_offtake or render_reference
for known values, otherwise fail fast with an error. Use the existing render()
function and the render_offtake/render_reference symbol names to locate the
branching logic and keep the output from being written when the source is
unknown.
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
2 participants
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.
Adds a Requirements to Tests Matrix which will allow us to trace from various requirements sources (which we will consolidate more of over time) to our main test-plan.yaml, and then trace through that to the individual tests.
docs/requirements/
Tooling
validateresolves both endpoints of every mapping edge (test in plan, req in its source listing) with uniqueness checks, plus acoveragereport of untested requirements. Wired intomake reqcheck+ a unit test;make planrenders the listings and traceability matrix.Summary by CodeRabbit
New Features
Documentation
Bug Fixes