Skip to content

fix(audit): state observations, not conclusions, in detector evidence - #168

Merged
AlexanderMakarov merged 10 commits into
mainfrom
fix/156-evidence-observations
Aug 3, 2026
Merged

fix(audit): state observations, not conclusions, in detector evidence#168
AlexanderMakarov merged 10 commits into
mainfrom
fix/156-evidence-observations

Conversation

@AlexanderMakarov

@AlexanderMakarov AlexanderMakarov commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Closes #156.

The problem

Detector evidence strings asserted the definition-level conclusion instead of the observation actually made. The reported case was AI-07:

no run mechanism found — no Makefile, docker-compose, package.json start/dev script, build-tool wrapper (mvnw/gradlew), manage.py, or Procfile; Claude Code cannot run the application without human involvement

The detector had only established that a fixed list of root files was absent. The bolded clause is a capability claim, and it is false for a Maven repo whose CLAUDE.md documents scripts/backend-start.sh — the case raised in Slack. Because these strings ship with reliability_default = "maximal", every divergence between a detector's proxy and the capability it stands for became a confidently stated falsehood in a client-facing report.

The change

Evidence now states what was checked and what was found: the patterns and globs searched, the paths matched, counts and denominators. The capability claim stays in references/standards.toml's definition field, which the report already surfaces as the hint and tooltip beside the evidence — so "why it matters" is still there, stated as what the standard requires rather than what is true of this repo. That is the correct owner of the sentence, precisely because the proxy can diverge from the definition.

AI-07 now reads:

no run mechanism found at repo root — checked for Makefile, docker-compose.yml, docker-compose.yaml, run.sh, start.sh, justfile, Justfile, Taskfile.yml, Taskfile.yaml, mvnw, gradlew, manage.py, Procfile, and package.json start/dev script; none present

Interpolated from ROOT_RUN_FILES, so the sentence cannot drift from the list the code actually searches. The same treatment is applied to MCP config paths, README names, TLS config globs, and env-example globs, and the two .env pattern lists now share one constant — that drift is what produced an enumeration missing .env*.local, the Next.js default, while the regex accepted it.

Statements about the engine's own measurement are deliberately kept: incomputability ("cannot compute a pass rate"), applicability and SKIP-gate reasons, scoring-rule notes, and threshold citations. Those describe the measurement, not the audited project.

Claims the code never checked

The sweep surfaced several strings asserting things no branch had established. These are corrected in wording only — no detection logic was added, so no score moves:

  • documentation.ts reported "missing setup instructions" on a branch that only tested content length.
  • security.ts claimed a Dockerfile COPY . would leak a file, having only checked that a Dockerfile exists.
  • supply_chain_security.ts reported that dependency updates required review without checking branch protection or CODEOWNERS.
  • application_security.ts named pbkdf2/sha256/sha512 as found on a branch reachable when only a CSPRNG signal is present — a repo with no password-hashing code at all was told weaker hashing algorithms were found. That branch is now split, with a regression test pinning it.
  • A coverage gate claimed a threshold "is enforced" when only a pattern in CI or config had been found.
  • QA-01 rendered one ratio two ways, so adjacent evidence lines read 59.5% and 60%, the second next to "below the 60% pass threshold".

Verification

No audit number moves. The engine was run from the pre-change and post-change sources over two repositories and three synthetic fixtures, and every field except evidence was compared: per-check status, value, score, confidence and weight; per-dimension totals; audit totals and coverage ratios. All identical, to full float precision. The fixtures were built specifically to reach the two branches that were split, since neither real repository exercises them.

npm test is green across all four layers (1568 tests). dist/ is rebuilt and committed. Prettier is clean.

Regression protection

tests/evidence-phrasing.test.ts scans every string literal under detectors/, metrics/ and audit_core.ts for banned conclusion phrasings. Its positive controls are derived mechanically — every string literal in the changed detectors is tokenized before and after the sweep and set-diffed — so the contract is "this diff cannot be reverted silently" rather than "these sentences won't come back". The scanner tracks comment, string, template and regex state rather than grepping raw source, and merges both + concatenation and template interpolation so a banned phrase split across a seam is still caught. The escape hatch requires a stated reason and is pinned at zero uses, so introducing the first one is a visible review decision.

Nine co-located regression tests pin the individual fixes — constant interpolation for AI-07/AI-04/DOC-01/AS-01, one-decimal rendering for QA-01/SBP-03, threshold-citation ordering for AS-05, code-point ranges for AIS-01, and the AS-07 branch split. Each was verified by reproducing the defect it guards and confirming the test fails.

Known limit, stated plainly: a lexical lint cannot judge whether a string is true of the branch it sits on. Every falsehood found during review of this work was banned-word-free and was caught by reading code, not by the lint. One residue class — inferring functional status from the presence of a directory — is documented in the test as out of reach.

Also here

  • detectors/README.md documents the evidence-phrasing contract, cross-referenced from metrics/README.md.
  • dimensions/application-security.md drops advice and verdict wording its bullets had mirrored from the old evidence.
  • Plugin version 2.4.3 → 2.4.4, since report output changes.
  • CLAUDE.md and a lint assertion message are corrected to the four-file version-bump discipline the lint actually enforces — plugins/awos/commands/flow.md carries a generator-version constant that the three-file description omitted. Note this means the bump marks previously generated flow artifacts as stale on the next re-run; that is the pre-existing design, not new behavior.

Summary by CodeRabbit

  • New Features

    • Updated the AWOS plugin to version 2.4.4.
    • Improved AI readiness audit results with clearer evidence, thresholds, searched locations, and measured percentages.
    • Added more precise reporting across security, documentation, testing, delivery, and development checks.
  • Documentation

    • Documented guidelines for consistent, fact-based audit evidence.
  • Bug Fixes

    • Corrected misleading or incomplete audit messages and improved reporting consistency.
  • Tests

    • Added regression coverage for evidence accuracy, thresholds, and detector output.

…#156)

Detector evidence strings asserted the definition-level conclusion instead of
the observation actually made. The clearest example was AI-07, which reported
"no run mechanism found — ... Claude Code cannot run the application without
human involvement" when all it had established was that none of a fixed list of
root files existed. That claim is false for a Maven repo whose CLAUDE.md
documents a run script, and because these strings ship with a maximal
reliability tag, every proxy-vs-definition divergence became a confidently
stated falsehood in a client-facing report.

Evidence now states what was checked and what was found: the patterns and globs
searched, the paths matched, counts and denominators. The capability-level claim
stays where it already lived — the `definition` field in standards.toml, which
the report surfaces as the hint and tooltip beside the evidence. Where a string
enumerates what was searched it interpolates the constant the code actually uses
(AI-07 renders ROOT_RUN_FILES), so the text cannot drift from the check.

Statements about the engine's own measurement are deliberately kept:
incomputability ("cannot compute a pass rate"), applicability and SKIP-gate
reasons, scoring-rule notes, and threshold citations. Those describe the
measurement rather than the audited project.

The sweep also removed several claims the code never checked, among them
documentation.ts asserting missing setup instructions on a branch that only
tested content length, security.ts asserting a Dockerfile COPY . would leak a
file without ever reading the Dockerfile, and supply_chain_security.ts reporting
that dependency updates required review without checking branch protection or
CODEOWNERS. Two strings that named specific findings on branches where those
findings were not established are now split so each branch states what actually
fired.

Verification: an engine run against the pre-change and post-change sources over
two repositories plus two synthetic fixtures produces byte-identical status,
value, score, confidence, weight, per-dimension totals and audit totals — only
evidence text differs. This change moves no audit number.

Also in this change:

- tests/evidence-phrasing.test.ts scans every string literal under detectors/
  and metrics/ for banned conclusion phrasings, with 20 positive controls drawn
  from real pre-change strings and negative controls pinning the allowed
  engine-self idioms. It parses comment, string, template and regex state rather
  than grepping raw source, and merges +-concatenated literals so a banned
  phrase split across a seam is still caught.
- detectors/application_security_as07.test.ts pins the AS-07 branch split: a
  repository with no password-hashing code at all must not be told that weaker
  hashing algorithms were found.
- detectors/README.md documents the evidence-phrasing contract, with a
  cross-reference from metrics/README.md.
- dimensions/application-security.md drops the advice and verdict wording its
  bullets had mirrored from the old evidence strings.
- Measured percentages that cite a threshold now render with one decimal, so a
  value just under a threshold no longer prints as equal to it.
- Plugin version 2.4.3 -> 2.4.4, since report output changes. CLAUDE.md and the
  lint comment are corrected to the four-file bump discipline that the lint
  actually enforces.
…eview gaps

Follow-up to the evidence sweep, from two independent reviews of the previous
commit. The sweep itself was sound — no audit number moved and the five
factually wrong strings found during review were corrected — but the tests
protecting it were thin and several strings on lines adjacent to the ones fixed
carried the same defect.

Strings corrected, all of the same class the issue describes — asserting more
than the check established:

- The SQL FAIL line named string concatenation as the mechanism found, though
  three of the five patterns match interpolation and the WARN branch beside it
  already hedged.
- The hardcoded-secret FAIL line said "committed files", but the scan walks the
  filesystem and never consults git, so an untracked file counted as committed.
- The Python type-safety lines read as whole-repo, while the ratio samples at
  most twenty files and only matches single-line signatures. The denominator is
  now stated, so a reader can reproduce the number.
- The weak-hash line said "near password hashing", but the proximity term
  includes a bare "hash", so an unrelated cache hash satisfies it.
- The coverage-gate WARN line still claimed a threshold "is enforced" when only
  a pattern in CI or config had been found. Its PASS sibling had been corrected;
  this one was missed, and the widened lint caught it.
- The QA-01 check rendered one ratio two ways, so adjacent evidence lines read
  59.5% and 60%. It now renders once.

Lists that were retyped by hand are interpolated from the constant the code
actually searches — MCP config paths, env-example globs, TLS config globs — and
the two .env pattern lists now share one constant, which is the drift that
produced the missing .env*.local form in the first place.

Tests now pin the fixes rather than only the matcher. Each was verified by
reproducing the defect it guards and confirming the test fails:

- Constant-interpolation pins for AI-07, AI-04, DOC-01 and AS-01 iterate the
  imported constant, so a hand-typed list that drifts from the check fails.
- One-decimal rendering is pinned for QA-01 and SBP-03 with fixtures whose ratio
  rounds to the threshold at integer precision.
- AS-05 asserts its threshold citation is not inverted.
- AIS-01 ties the advertised code-point ranges to the detection predicate in
  both directions.

The lint's recall was the main finding: twelve of eighteen strings the previous
commit removed still passed it, so most of that diff could have been reverted
invisibly. Its positive controls are now derived mechanically, by tokenizing
every string literal in the changed detectors before and after the sweep and
set-diffing them, which grew the corpus from nineteen hand-picked strings to
forty-one and surfaced thirteen further banned families. Fixing the tokenizer to
treat a template's static parts as one literal both repaired that extraction and
closed the gap where a banned phrase split across an interpolation escaped the
scan. The scan now also covers audit_core.ts, which authors report-visible skip
reasons; the escape hatch requires a reason and is pinned at zero uses; and a
keyword fix removes a case where a regex literal containing a quote would
silently end literal extraction for the rest of a file.

What the lint cannot do is judge whether a string is true of the branch it sits
on. Every falsehood found in review was banned-word-free, and the residue is
documented in the test: claims that infer functional status from the presence of
a directory are real, already fixed, and beyond the reach of a lexical check.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AlexanderMakarov, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b93adc5-84d6-4fb1-b07a-9fbd668f9a63

📥 Commits

Reviewing files that changed from the base of the PR and between 2829dd3 and 5b66e2b.

⛔ Files ignored due to path filters (1)
  • plugins/awos/skills/ai-readiness-audit/dist/cli.js is excluded by !**/dist/**
📒 Files selected for processing (10)
  • plugins/awos/skills/ai-readiness-audit/detectors/_base.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/application_security.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/documentation.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/quality_assurance.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/security.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/software_best_practices.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/software_best_practices_sbp03.test.ts
  • plugins/awos/skills/ai-readiness-audit/tests/det-quality-assurance.test.ts
  • plugins/awos/skills/ai-readiness-audit/tests/det-security.test.ts
  • plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts
📝 Walkthrough

Walkthrough

The PR aligns AWOS plugin versions at 2.4.4, standardizes detector evidence wording, exports shared detector constants, improves unreadable supply-chain configuration handling, and adds documentation plus regression and repository-wide enforcement tests.

Changes

Evidence phrasing and version alignment

Layer / File(s) Summary
Plugin version contract
.claude-plugin/marketplace.json, plugins/awos/.claude-plugin/plugin.json, plugins/awos/commands/flow.md, tests/lint-prompts.test.js, CLAUDE.md
Plugin metadata, generator stamping, lint expectations, and version-bump documentation now use version 2.4.4 and a four-file update contract.
Evidence phrasing enforcement
plugins/awos/skills/ai-readiness-audit/detectors/README.md, plugins/awos/skills/ai-readiness-audit/metrics/README.md, plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts
Evidence rules are documented and enforced through source scanning, tokenization, banned-pattern matching, controls, and allow-marker handling.
Detector evidence and supply-chain updates
plugins/awos/skills/ai-readiness-audit/detectors/*.ts, plugins/awos/skills/ai-readiness-audit/dimensions/application-security.md
Detector messages identify checked paths, patterns, counts, thresholds, and applicability details. SCS-05 reports unreadable automation files and returns WARN when automerge state is unknown.
Detector regression coverage
plugins/awos/skills/ai-readiness-audit/detectors/*test.ts, plugins/awos/skills/ai-readiness-audit/tests/det-quality-assurance.test.ts, plugins/awos/skills/ai-readiness-audit/tests/det-supply-chain-security.test.ts
Tests verify shared constants, formatted threshold evidence, AS-07 evidence separation, Unicode-range detection, QA wording, and unreadable configuration handling.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

  • provectus/awos#115 — Shares detector implementation and messaging changes for supply-chain security and prompt-agent integrity.
  • provectus/awos#145 — Shares the AWOS plugin metadata and version-pin contract.
  • provectus/awos#164 — Directly overlaps the generator, pinned version, and plugin metadata contract.

Suggested labels: patch

Suggested reviewers: kmakarychev-dev, workshur

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: updating detector evidence to state observations instead of unsupported conclusions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/156-evidence-observations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🧹 Nitpick comments (3)
plugins/awos/skills/ai-readiness-audit/detectors/documentation.ts (1)

64-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hand-typed 200 in the evidence can drift from the condition.

Same class of drift this PR guards elsewhere — derive the printed threshold from a named constant used by the comparison.

♻️ Interpolate the threshold
-  if (content.length <= 200) {
+  if (content.length <= README_MIN_CHARS) {
     return makeResult('WARN', content.length, [
-      `${relPath} is ${content.length} characters (<= 200 character threshold)`,
+      `${relPath} is ${content.length} characters (<= ${README_MIN_CHARS} character threshold)`,
     ]);
   }

Declare near the other module constants:

const README_MIN_CHARS = 200;
As per coding guidelines: detector evidence should be templated off "the constant the code actually uses rather than hand-typing a parallel list".
🤖 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 `@plugins/awos/skills/ai-readiness-audit/detectors/documentation.ts` around
lines 64 - 66, Define a module-level README_MIN_CHARS constant alongside the
other constants and use it in the content.length comparison within the
documentation detector. Interpolate README_MIN_CHARS into the warning evidence
message instead of hard-coding 200, keeping the existing threshold behavior
unchanged.

Source: Coding guidelines

plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts (2)

844-847: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Escape-hatch instruction omits that the ratchet test must be updated too.

Adding // evidence-phrasing:allow <reason> as instructed here immediately fails the "escape hatch is unused" test at line 801. That's deliberate, but the message should say so, otherwise the next maintainer follows the advice and lands in a second red test with no pointer.

🤖 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 `@plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts`
around lines 844 - 847, Update the escape-hatch guidance string in the
evidence-phrasing test to state that adding a valid evidence-phrasing:allow
marker also requires updating the ratchet test asserting the escape hatch is
unused. Reference the ratchet test near line 801 so maintainers know to adjust
that expectation when the exemption is intentionally introduced.

524-538: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Scan recursively or assert the flat-directory contract.

readdirSync only scans the top level, so any future nested source files under detectors/ or metrics/ would be excluded from the wording guard without an error. Either walk recursive and keep only non-test .ts files outside *.d.ts, or assert these directories are flat.

🤖 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 `@plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts`
around lines 524 - 538, Update nonTestTsFiles, used by scanTargets, to
recursively discover in-scope non-test TypeScript files under each configured
directory while excluding *.test.ts and declaration files (*.d.ts); preserve the
existing relative-path output format. Alternatively, explicitly validate that
SCAN_DIRS directories are flat and fail when nested source files are present.
🤖 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 `@plugins/awos/skills/ai-readiness-audit/detectors/prevention_coverage.ts`:
- Line 589: Update the PASS evidence text in the prevention coverage detector to
change “and a coverage-threshold pattern found” to “and a coverage-threshold
pattern was found,” preserving the rest of the message.

In `@plugins/awos/skills/ai-readiness-audit/detectors/software_best_practices.ts`:
- Around line 241-255: Update the threshold display calculations near
pctDisplay, passAtPct, and warnAtPct to format passAt and warnAt as percentages
with one decimal place, matching pctDisplay while leaving the exact ratio
comparisons unchanged. Use the resulting values in all PASS, WARN, and FAIL
evidence messages.

In `@plugins/awos/skills/ai-readiness-audit/detectors/supply_chain_security.ts`:
- Around line 585-586: Update the detector’s result handling around readTextSafe
and the PASS branch so unreadable automation configuration files are tracked
separately instead of being treated as inspected configs without automerge.
Return an unknown/WARN result when any file cannot be read, or at minimum
include the unreadable paths and state that no automerge: true pattern was
detected rather than asserting automerge is disabled.

---

Nitpick comments:
In `@plugins/awos/skills/ai-readiness-audit/detectors/documentation.ts`:
- Around line 64-66: Define a module-level README_MIN_CHARS constant alongside
the other constants and use it in the content.length comparison within the
documentation detector. Interpolate README_MIN_CHARS into the warning evidence
message instead of hard-coding 200, keeping the existing threshold behavior
unchanged.

In `@plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts`:
- Around line 844-847: Update the escape-hatch guidance string in the
evidence-phrasing test to state that adding a valid evidence-phrasing:allow
marker also requires updating the ratchet test asserting the escape hatch is
unused. Reference the ratchet test near line 801 so maintainers know to adjust
that expectation when the exemption is intentionally introduced.
- Around line 524-538: Update nonTestTsFiles, used by scanTargets, to
recursively discover in-scope non-test TypeScript files under each configured
directory while excluding *.test.ts and declaration files (*.d.ts); preserve the
existing relative-path output format. Alternatively, explicitly validate that
SCAN_DIRS directories are flat and fail when nested source files are present.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b20cad4-7708-47b8-a145-e3d26c80c1a0

📥 Commits

Reviewing files that changed from the base of the PR and between f7293e3 and 726df92.

⛔ Files ignored due to path filters (1)
  • plugins/awos/skills/ai-readiness-audit/dist/cli.js is excluded by !**/dist/**
📒 Files selected for processing (30)
  • .claude-plugin/marketplace.json
  • CLAUDE.md
  • plugins/awos/.claude-plugin/plugin.json
  • plugins/awos/commands/flow.md
  • plugins/awos/skills/ai-readiness-audit/detectors/README.md
  • plugins/awos/skills/ai-readiness-audit/detectors/ai_development_tooling.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/ai_development_tooling_ai04.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/ai_development_tooling_ai07.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/application_security.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/application_security_as01.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/application_security_as05.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/application_security_as07.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/documentation.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/documentation_doc01.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/end_to_end_delivery.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/prevention_coverage.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/prompt_agent_integrity.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/prompt_agent_integrity_ais01.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/quality_assurance.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/quality_assurance_qa01.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/security.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/software_best_practices.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/software_best_practices_sbp03.test.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/spec_driven_development.ts
  • plugins/awos/skills/ai-readiness-audit/detectors/supply_chain_security.ts
  • plugins/awos/skills/ai-readiness-audit/dimensions/application-security.md
  • plugins/awos/skills/ai-readiness-audit/metrics/README.md
  • plugins/awos/skills/ai-readiness-audit/tests/det-quality-assurance.test.ts
  • plugins/awos/skills/ai-readiness-audit/tests/evidence-phrasing.test.ts
  • tests/lint-prompts.test.js

Comment thread plugins/awos/skills/ai-readiness-audit/detectors/prevention_coverage.ts Outdated
Comment thread plugins/awos/skills/ai-readiness-audit/detectors/software_best_practices.ts Outdated
Comment thread plugins/awos/skills/ai-readiness-audit/detectors/supply_chain_security.ts Outdated

@AndreyNenashev AndreyNenashev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ran everything locally on the PR head before reading in depth: all 8 evidence-phrasing tests and the 60 co-located detector tests pass, npm run build:audit-engine gives an empty diff against the committed dist/, prettier is clean, and the version says 2.4.4 in all four places the lint looks. I also traced the AS-07 split by hand — the fallthrough WARN really is only reachable when CSPRNG is the lone signal, and both WARN paths keep the same status and score.

Approving. I have four comments — all small, and honestly three of them are just your own patterns from this PR applied to spots it didn't reach. Nothing here touches detection logic or moves a score.


// Human-readable form of the patterns checked in ENV_GITIGNORE_RX, shared by
// the PASS and FAIL evidence lines so they can't drift apart.
const ENV_GITIGNORE_PATTERNS =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ENV_GITIGNORE_PATTERNS is still a hand-typed twin of ENV_GITIGNORE_RX. I expanded the regex and the seven patterns do line up today, but nothing keeps them lined up — det-security.test.ts exercises the regex, not the sentence. That's the exact seam that produced the .env*.local omission you describe in the PR body, and this ended up as the one hand-written rendering without a pin (AIS-01 got its range test, everything else interpolates the constant it searches). What I'd do: flip it so the list is the source of truth — ['.env', '.env.*', '.env*', '.env*.local', '*.env', '**/.env', '/.env'] — join it for the evidence string, and add a small test that each entry, written as a .gitignore line, matches the regex (plus something like .envrc that shouldn't). Then the sentence can't lie.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in aca7b1e, taking the flip you describe.

ENV_GITIGNORE_PATTERNS is now the array, the evidence joins it, and the old comment claiming the two lines "can't drift apart" is gone — it was true of the PASS/FAIL pair and false of the thing that matters, since neither was tied to the regex that decides.

The test walks every entry as the literal .gitignore line and asserts the detector accepts it, with .envrc as the negative so it cannot pass by the regex matching everything.

// 70% pass threshold". Using one decimal consistently, rather than only
// in the threshold-comparison lines, keeps every evidence line for the
// same ratio agreeing with the others.
const pctDisplay = (ratio * 100).toFixed(1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One decimal shrinks the collision window from 0.5 pp to 0.05 pp, but it's still there (same story in detectTypeSafety and detectAuthOnMutations). Take 149 test files over 497 source modules: that's 29.98%, which renders as "30.0% — below the 30% warn threshold" (line 159). And the guard regex /(\d+)% — below the \1%/ can't see it, since 30.030 as text — so the "must never render as the same number" assertion holds for the fixture, not in general. Cheapest fix is to admit the residual in the comment and widen the guard to /(\d+)(?:\.\d+)?% — below the \1(?:\.\d+)?%/ with a numeric compare. If you want to actually close it, bump precision only when the rendered value collides with the threshold that branch cites as "below":

let pctDisplay = (ratio * 100).toFixed(1);
if (Number(pctDisplay) === citedBelowThresholdPct)
  pctDisplay = (ratio * 100).toFixed(2);

Either way it's evidence-only — status and score don't move.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right, and this also corrects a reply I posted earlier on this PR. CodeRabbit raised the threshold-rendering question from the other direction (a fractional pass_at), I pushed back because nothing in standards.toml configures one — and then framed the collision as closed. It was not. The residual comes from the measurement side, which I did not consider.

Verified your case directly before fixing: 149/497 is 29.9799%, renders "30.0% — below the 30% warn threshold", and /(\d+)% — below the \1%/ returns false on that string. So the guard was holding for its fixture, exactly as you said.

Took the second option, in 945242c. formatMeasuredPct in _base.ts renders at one decimal and adds precision only when the result would read as equal to a threshold the same sentence calls it "below". One refinement on your snippet: it only guards against thresholds the value is genuinely below, so a ratio sitting exactly on a threshold still renders 30.0% rather than escalating to 30.000% — "at or above the 30%" is accurate there, so there is nothing to disambiguate.

Applied to all three sites you named — QA-01, SBP-03, AS-06 — since it was the third copy of the shape. The guard in software_best_practices_sbp03.test.ts now extracts both numbers and compares them numerically instead of by backreference. New test pins your exact ratio, and I confirmed it reproduces "30.0% — below the 30%" when the formatting is reverted.

Evidence-only, as you said: status and score do not move.


return makeResult('PASS', content.length, [
`${relPath} present with headings and setup instructions (${content.length} bytes)`,
`${relPath} present with headings and setup instructions (${content.length} characters)`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line and the WARN at line 72 are two survivors of the class you fixed one branch up in this same function. SETUP_CONTENT_RX matches ten keywords, but both strings claim something stronger — that setup instructions exist or don't. A README that says "Bootstrapping: ./bootstrap.sh" has setup instructions and matches nothing, so it gets told it "contains no setup/install/usage instructions." They slipped past the lint because there's no banned token in them. One-word-ish fixes:

  • line 72: `${relPath} has no setup/install/usage/run/build keyword`
  • line 83: `${relPath} present with heading(s) and setup-keyword match (${content.length} characters)`

Could also pull the keywords into a constant and interpolate, same as ROOT_RUN_FILES.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3ba6202, with your wording.

  • line 72 → ${relPath} has no setup/install/usage/run/build keyword
  • line 83 → ${relPath} present with heading(s) and setup/install/usage/run/build keyword match (N characters)

Also pulled the keywords into a SETUP_KEYWORDS constant the sentences interpolate, mirroring ROOT_RUN_FILES, so the wording stays tied to what is searched rather than being retyped next to it — same failure mode as the AS-12 thread.

* fragments at each `${...}`.
*
* The positive-control corpus below is derived mechanically from
* `git diff f7293e3 4f619bb` — every evidence string issue #156 actually

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tiny one: this cites git diff f7293e3 4f619bb (also at lines 546 and 574), but 4f619bb is a branch commit — after the squash-merge it won't resolve, and the "mechanically derived" claim becomes unverifiable. Point at issue #156 / this PR instead, or say "this PR's base..head".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 43a724c — all three sites (35, 546, 574) now cite issue #156 and this PR's base..head instead of 4f619bb.

…ver read

`detectDependencyAutomationReview` pushed every existing config into
`foundFiles` and then `continue`d past the content check when `readTextSafe`
returned null. An unreadable config therefore counted toward the PASS while
its contents were never inspected, and the evidence asserted "dependency
automation configured without automerge enabled" — a conclusion about a file
nobody opened. That is the same observation-vs-conclusion problem this branch
is closing, one level down.

Unreadable paths are now tracked apart from read ones. When some configs were
readable and none matched, the evidence scopes itself to what was inspected
and lists the unreadable paths separately instead of folding them into the
claim. When no config could be read at all the result is WARN, because
automerge is unknown rather than absent.

Adds the two cases, which had no coverage: a mixed readable/unreadable pair,
and a wholly unreadable config. Both use a directory at the config path, the
portable way to make `readTextSafe` return null.
'a coverage-threshold pattern found' was missing its verb. Fixed on the PASS
branch, and on the paired WARN branch on the next line so the two read alike.
Picks up the SCS-05 and PRV-06 evidence changes; dist/cli.js is committed, so
without this the shipped engine keeps emitting the old strings.
`ENV_GITIGNORE_PATTERNS` was a prose twin of `ENV_GITIGNORE_RX`, hand-typed
beside it. The comment claimed the two evidence lines "can't drift apart",
which was true of the lines and false of the thing that matters: neither is
tied to the regex that actually decides. That seam is what let `.env*.local`
be accepted by the detector while going unlisted in the sentence.

The list is now the source of truth and the wording is joined from it, so the
sentence cannot name a pattern the detector does not accept. A test walks
every entry as the literal `.gitignore` line a reader would write and asserts
the detector passes on it, with `.envrc` as the negative so the assertion
cannot pass by the regex matching everything.
…setup

Two survivors of the class fixed one branch up in the same function.
`SETUP_CONTENT_RX` matches ten keywords, but the WARN and PASS strings claimed
setup instructions were absent or present. A README reading
"Bootstrapping: ./bootstrap.sh" has them and matches nothing, and was told it
"contains no setup/install/usage instructions". Neither string carried a
banned token, so the phrasing lint let them through.

Both now state what was searched for and what matched. The keywords come from
a named constant the sentence interpolates, so the wording stays tied to the
check the way ROOT_RUN_FILES already is.
…owing it

Moving from integer to one-decimal rendering shrank the window in which a
measured ratio prints as the same number as the threshold it is called
"below" — from 0.5pp to 0.05pp — but did not close it. 149 test files over
497 source modules is 29.9799%, which rendered "30.0% — below the 30% warn
threshold": still self-contradictory, and still unverifiable to a reader.

The guard could not see it. `/(\d+)% — below the \1%/` compares by textual
backreference, and "30.0" is not the string "30", so the assertion that the
two "must never render as the same number" held for its own fixture rather
than in general. It now extracts both numbers and compares them numerically.

`formatMeasuredPct` renders at one decimal and adds precision only when the
result would read as equal to a threshold the same sentence calls it "below" —
so the extra digits appear exactly where they carry information, and a value
sitting legitimately *at* a threshold is not escalated, because "at or above"
is accurate there. Applied to the three checks that share the shape: QA-01,
SBP-03, and AS-06. Evidence-only; no status or score moves.

Pinned with the ratio from the review (149/497 against a 30% threshold),
verified to reproduce the reported sentence when the formatting is reverted.
evidence-phrasing.test.ts derived its corpus from `git diff f7293e3 4f619bb`
in three places. `4f619bb` is a commit on this branch: after the squash-merge
it stops resolving and the "mechanically derived" claim becomes unverifiable.
Points at issue #156 and this PR's base..head instead.
Picks up the AS-12, DOC-01 and QA-01/SBP-03/AS-06 evidence changes.
@AlexanderMakarov
AlexanderMakarov merged commit 46746bc into main Aug 3, 2026
8 checks passed
@AlexanderMakarov
AlexanderMakarov deleted the fix/156-evidence-observations branch August 3, 2026 15:33
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.

audit: detector evidence strings assert conclusions instead of observations

2 participants