Skip to content

Add a Testing and Validation workstream for the conformance suite - #292

Open
astrogilda wants to merge 2 commits into
GenAI-Security-Project:integrationfrom
astrogilda-forks:docs/testing-validation-workstream
Open

astrogilda wants to merge 2 commits into
GenAI-Security-Project:integrationfrom
astrogilda-forks:docs/testing-validation-workstream

Conversation

@astrogilda

Copy link
Copy Markdown

What changed

GOVERNANCE.md gets a Testing and Validation workstream, with me as first lead and the second seat open. It builds and runs the conformance suite: the #33 requirement map and tests, the #53 negative vectors, their CI job and a public results registry. I brought the ACS-Core vectors in #223 and mapped the v0.2 codes onto section 17.1 on #53, and that work continues here.

@rocklambros, @bar-capsule, @afogel, this brings back the seat #218 dropped, now with a lead and a written scope. The #19 ruling stands: the Spec workstream decides what conformant means and versions the suite with each release.

Which issue does this implement

Part of #33 and part of #53, on the integration base branch since this touches tests.

I tested this

At b8e8fc4, pytest gives 919 passed and the strict mkdocs build passes, and both commits are signed off for the DCO.

GOVERNANCE.md gains a Testing and Validation workstream that builds and
runs the conformance suite in this repository: the requirement ledger and
behavioral tests in GenAI-Security-Project#33, the v0.2.0 negative vectors in GenAI-Security-Project#53, the CI job
that runs them against the reference Guardian, and the registry of
conformance results once the suite exists. The Spec workstream still
decides what conformant means and versions the suite with each release.

CODEOWNERS gains the conformance paths that GenAI-Security-Project#53 asks for, and the landing
page test counts eight workstreams.

Signed-off-by: Sankalp Gilda <23521054+astrogilda@users.noreply.github.com>
The conformance paths carry the reference implementation owners, so a
change to the suite gets the same review whichever directory it lands in.

Signed-off-by: Sankalp Gilda <23521054+astrogilda@users.noreply.github.com>

@drk-ram drk-ram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at b8e8fc440ed3251519bd649aa1f43b72d207e6cb. On Windows / Python 3.11.9 at this SHA, tests/test_render_landing.py gives 27 passed, and tests/test_governance_files.py plus tests/test_roadmap_model_roster.py give 32 passed. Those pass because the branch is behind integration. The merge result is what matters, so I also ran git merge-tree --write-tree origin/integration refs/pull/292/head against bba1b846fbe7511b0b6f876c61b1fe91b27b07da.

I support having a Testing and Validation workstream with a named lead and a written scope, and the paragraph defining its boundary with the Spec workstream is clear. The problems are in the CODEOWNERS half and in staleness.

Blocking

1. Merge conflict, and the test hunk is dead. GitHub reports the PR as CONFLICTING, and merge-tree names tests/test_render_landing.py. On integration, #441 removed the Workstreams section from the landing page and with it parse_workstreams; git grep parse_workstreams origin/integration returns nothing in tools or tests. So the == 7 to == 8 edit in test_parse_workstreams_handles_the_real_file targets a test that no longer exists. The right resolution is to drop that hunk. After the rebase, the "919 passed" figure in the description is for a different tree and should be re-measured.

2. It re-adds an inert CODEOWNERS entry that #414 deliberately removed. #414 (Closes #144) took three owners out for exactly this reason and left the rule in the file header: "Workstream leads without write access stay in GOVERNANCE.md and are not listed here, because an entry without write access does nothing." This PR's own comment says "@astrogilda does not hold write access yet, so his entries stay inert until he does." That is the situation the project just decided not to keep. The roster note #414 added to GOVERNANCE.md ("... hold no write access, so a project lead assigns, labels, and owns review ...") is the pattern to follow here: list the lead in the table, say a project lead owns review until write access exists, and add the CODEOWNERS entry in the PR that grants access.

I also checked the other side of this, because tools/roadmap_model.py::trusted_logins builds the roadmap's trusted-closer set from CODEOWNERS plus the roster. A workstream lead listed in GOVERNANCE.md is already in that set, so the CODEOWNERS lines add no trust. They are pure noise, which makes it hard to justify keeping them.

3. Both new CODEOWNERS paths match nothing. On integration and at this head, conformance/ and reference-implementations/agt/conformance/ do not exist. The conformance package lives at reference-implementations/agt/packages/conformance/ (git ls-tree confirms), and it is already covered by the /reference-implementations/ rule above. The comment "the last matching pattern wins" is correct in general, but here no pattern has anything to win. If the intent is to reserve /conformance/ for the suite described in #33/#53, create it with the PR that adds files there.

Non-blocking

  1. The charter lists "the public registry of conformance results once the suite exists" as in scope. The charter should say who signs off on a results entry and how a result is tied to the spec release it was measured against; a registry entry without a pinned spec version and a runner/operator relationship is not comparable between submitters. #297 (the verdict-evidence predicate) is the natural carrier for that, which is a reason to keep the two PRs coordinated.
  2. Say in GOVERNANCE.md how a Testing and Validation lead interacts with the existing triage authority table, since that lead would be labelling conformance-area issues.
  3. The "seat #218 dropped" and the #19 ruling appear only in the PR description. One line of rationale in GOVERNANCE.md would keep the file self-contained.

Once rebased on integration, with the dead test hunk dropped and the CODEOWNERS lines either removed or moved to the PR that grants write access, I expect to approve.

Raghuvar Karthik Durga (@drk-ram)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants