Skip to content

Exempt library-helper modules from audit-writers (write_validated.py)#78

Merged
realmarcin merged 2 commits into
mainfrom
investigate-write-validated
May 26, 2026
Merged

Exempt library-helper modules from audit-writers (write_validated.py)#78
realmarcin merged 2 commits into
mainfrom
investigate-write-validated

Conversation

@realmarcin
Copy link
Copy Markdown
Contributor

Summary

Investigation outcome from item 1 in the recent next-tasks list. The newly-surfaced src/traitmech/validation/write_validated.py is the canonical, well-used write-time YAML gate — not an ungated write path. It just had no way to opt out of the CLI-writer audit, so it appeared as a writer missing curation_history + safeguard.

This PR adds a marker-based opt-out for library helpers.

Investigation findings

$ grep -rn "write_validated_trait" --include='*.py' scripts/
scripts/seed_from_metpo.py
scripts/ground_causal_nodes.py
scripts/ground_causal_predicates.py
scripts/rename_predicate_labels.py
scripts/retype_causal_nodes.py

All five mutating CLI scripts route through write_validated_trait. Each wraps the call in try/except ValidationFailedError, logs + skips invalid records, and exits non-zero on any skipped record. audit_writers already credits the callers with validates_before_write: yes (the write_validated_trait( regex was added in the PR #66 review cycle).

write_validated.py is a sanctioned helper. No hardening needed. The fix is the audit exemption.

Change

  1. src/traitmech/validation/write_validated.py — add an audit-writers: library-helper marker to the module docstring, plus prose documenting why curation-history and safeguards belong to callers, not here.

  2. scripts/audit_writers.py::audit() — skip any file whose source contains the marker substring (parallel to the existing self-suppression added in PR Audit backlog cleanup: G02, G03, G04, G05 #64).

  3. tests/test_audit_writers.py — three new tests:

    • test_audit_skips_library_helper_marker — synthetic file with the marker is excluded.
    • test_audit_does_not_skip_without_marker — sanity check that the exemption is strictly opt-in.
    • test_audit_excludes_write_validated_helper — end-to-end: the real write_validated.py is excluded.
  4. reports/pipeline_writers_audit.tsv — refreshed snapshot.

Audit output

Before (6 writers):
  appends curation_history:   5 / 6
  has write safeguard:        5 / 6
  validates before write:     6 / 6
  wired into justfile:        5 / 6

After (5 writers, all 5/5):
  appends curation_history:   5 / 5
  has write safeguard:        5 / 5
  validates before write:     5 / 5
  wired into justfile:        5 / 5

Note on PR_E from the original list: I'd planned to wire rename_predicate_labels.py and retype_causal_nodes.py into the justfile in a separate PR. They're already wired — the justfile has rename-predicates / rename-predicates-apply / retype-causal-nodes / retype-causal-nodes-apply recipes. So PR_E is a no-op; the writer-audit cleanup happens in this PR via the helper exemption alone.

Verified locally

  • just audit-writers → 5/5 across every column
  • uv run pytest tests/test_audit_writers.py → 15 pass (was 12; +3 for the exemption coverage)
  • uv run pytest tests/ → 59 pass total (was 56; +3)

Test plan

  • Audit reports 5/5 across the matrix
  • All 59 tests green
  • CI runs pytest (added in follow-up PR_B)

🤖 Generated with Claude Code

`src/traitmech/validation/write_validated.py` is the canonical
write-time YAML gate that all five CLI writers
(seed_from_metpo, ground_causal_predicates, ground_causal_nodes,
rename_predicate_labels, retype_causal_nodes) route through. After
PR #75 tightened the audit's YAML-writer heuristic, write_validated
correctly surfaced as a writer — but it was then incorrectly flagged
as missing curation_history and write-safeguard, because those
responsibilities live in callers, not in a library helper.

Adds a marker-based opt-out:

1. write_validated.py declares `audit-writers: library-helper` in
   its module docstring (documenting WHY callers are responsible
   for safeguards).
2. audit_writers.audit() skips any file containing the marker
   substring `audit-writers: library-helper` (parallel to the
   existing self-suppression).
3. Three new tests in test_audit_writers.py:
   - test_audit_skips_library_helper_marker — synthetic file with
     the marker is excluded.
   - test_audit_does_not_skip_without_marker — sanity check that
     the exemption is strictly opt-in.
   - test_audit_excludes_write_validated_helper — end-to-end:
     the real write_validated.py is excluded.

Investigation findings (for the PR description):
- 5 scripts import `write_validated_trait` + `validate_trait` +
  `ValidationFailedError`. All 5 wrap calls in try/except
  ValidationFailedError, log + skip invalid records, and exit
  non-zero on any skipped record.
- audit_writers already credits callers with
  `validates_before_write: yes` based on the
  `write_validated_trait(` regex (added in PR #66 review cycle).
- Net effect: write_validated.py is a sanctioned, well-used helper;
  no hardening needed. The fix is the audit exemption, not changes
  to the helper.

Audit output before:
  6 writers, 5/6 curation_history, 5/6 safeguard, 6/6 validates,
  5/6 wired

Audit output after:
  5 writers, 5/5 across every column

Refreshed snapshot at reports/pipeline_writers_audit.tsv.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 26, 2026 02:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an explicit opt-out mechanism so that library helper modules (not CLI writers) don’t get flagged by the audit-writers report, with accompanying tests and an updated audit snapshot.

Changes:

  • Add an audit-writers: library-helper marker to write_validated.py to indicate it should be excluded from the CLI-writer audit.
  • Update scripts/audit_writers.py::audit() to skip files containing the marker.
  • Add tests covering marker-based exclusion behavior and refresh the TSV audit snapshot.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/traitmech/validation/write_validated.py Documents and applies the library-helper opt-out marker in the canonical validated write helper.
scripts/audit_writers.py Implements marker-based suppression so helper modules don’t appear as “writers” needing CLI safeguards.
tests/test_audit_writers.py Adds unit tests ensuring the exemption is opt-in and that write_validated.py is excluded end-to-end.
reports/pipeline_writers_audit.tsv Updates the audit snapshot to reflect the helper exclusion (writer count reduced by 1).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/audit_writers.py Outdated
Tighten the library-helper exemption per Copilot. Previously a raw
substring match anywhere in the file would silently suppress a CLI
writer that happened to mention the phrase in narrative text or in
a docstring.

Two-part fix:

1. PATH constraint — only files under `src/traitmech/` can opt out.
   CLI writers under `scripts/` can never suppress themselves, even
   if they include the phrase in a docstring/comment. Enforced via
   `src/traitmech/` substring check on the file's posix path.

2. LINE-MARKER constraint — the marker `audit-writers: library-helper`
   must appear on its own line (modulo leading whitespace for
   docstring indentation), not embedded in surrounding prose.
   Enforced via re.MULTILINE regex `^\\s*audit-writers:\\s*library-helper\\s*$`.

Both constraints must hold; either alone is insufficient.

Three new tests added (17 total in test_audit_writers.py):

  test_audit_does_not_skip_cli_writer_with_marker
    A scripts/ file with the same body as a sanctioned library
    helper is still audited — the path-constraint catches it.

  test_audit_does_not_skip_marker_in_narrative_text
    A src/traitmech/ file that mentions the phrase mid-sentence in
    a docstring is still audited — the line-anchor catches it.

  test_audit_skips_library_helper_marker (existing, updated)
    Now creates the synthetic file under tmp_path/src/traitmech/
    so the path-constraint passes.

Verified:
  - just audit-writers → still 5/5 across the matrix (write_validated.py
    correctly excluded; the standalone-line marker in its docstring
    matches).
  - pytest tests/test_audit_writers.py → 17 passed (was 15; +2 for
    the new constraint tests).
  - pytest tests/ → 58 passed (was 56).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@realmarcin realmarcin merged commit 781e5c2 into main May 26, 2026
1 check passed
@realmarcin realmarcin deleted the investigate-write-validated branch May 26, 2026 04:58
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