feat(e3b): skill-test-scaffold-audit-coverage — test quality advisory suite

[javiAI]

Summary

E3b delivers three skills (writer-scoped + read-only advisory) for test quality scaffolding and audit. Closes Phase E.

• /pos:test-scaffold (writer-scoped strict, main-strict): Detects test pair convention (co-located vs tests/); generates skeleton if ≥80% clear; STOP+propose options if ambiguous. No source mods, no execution.
• /pos:test-audit (read-only advisory, main-strict): Declares candidate signals via static analysis — flaky risk, orphan tests, trivial assertions. No pytest/vitest execution, no file mods, no exhaustivity claim.
• /pos:coverage-explain (read-only advisory, main-strict): Reads existing coverage reports (lcov.json / pytest-cov / NYC / htmlcov), declares parsing strategy, explains gaps, proposes minimum targets. No execution, no threshold mods.

Scope (strict)

• Three frontmatter-minimal SKILL.md files (name / description / allowed-tools).
• All declarative/advisory-only — no enforcement, no execution, no source code mutation.
• skills_allowed: 10 → 13 (test-scaffold, test-audit, coverage-explain added).

Docs-sync (complete)

• ROADMAP.md ✅ — E3 phase row marked done (E3a + E3b).
• HANDOFF.md ✅ — §1 snapshot updated (current=E3b, next=F1 audit-session); §9 rewritten for F1; new §17 Estado E3a + §18 Estado E3b appended.
• MASTER_PLAN.md ✅ — § Rama E3b expanded from 2-line stub to full close (Decisiones A1..A6, Entregables, Contrato fijado, V1→V2 ajustes, Resultado, Criterio de salida).
• .claude/rules/skills-map.md ✅ — Pattern + Test rows for the three E3b skills populated with finalized lifecycle / context / scope / allowed-tools text matching SKILL.md bodies.

Pre-pr-gate baseline (HANDOFF.md) and conditional skills/** rule (.claude/rules/skills-map.md) were missing from the original push and were closed in commit 9312a69.

Critical review (Opus pass over Haiku-authored diff)

• SKILL.md content from prior Haiku-authored commits verified correct on re-read; no bugs introduced. Issues were docs-sync gap + era-bound naming drift in two test functions, both addressed in 9312a69.
• Test renames to drop stale era prefix:
  • hooks/tests/test_lib_policy.py: test_real_skills_allowed_populated_by_e3a → _by_e3b (docstring 10-tuple → 13-tuple).
  • hooks/tests/test_skills_log_contract.py: test_all_ten_e1_e3a_skills_end_to_end → test_all_thirteen_e1_e3b_skills_end_to_end (docstring + f-string updated to E1a+E1b+E2a+E2b+E3a+E3b).

Deliverables

• 3 SKILL.md files with detailed steps 1–6.
• 15 behavior contract tests in .claude/skills/tests/test_e3b_behavior.py (writer-scoped + main-strict + advisory framing locked).
• ALLOWED_SKILLS extended to 13.
• policy.yaml.skills_allowed updated.
• Docs-sync: ROADMAP + HANDOFF + MASTER_PLAN + skills-map all inside this PR.

Tests

• Full suite: 777 passed + 1 skipped on hooks/tests/ + .claude/skills/tests/.
• No regression on D1..D6 + E1a..E3a.
• Behavior tests lock framing + scope boundaries (no execution, no source mods, allowed-tools shape).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds E3b “test quality advisory suite” skills to the meta-repo skill registry, along with contract-style tests and documentation updates so the Stop-hook allowlist recognizes the new skills.

Changes:

• Extend policy.yaml.skills_allowed and the test allowlist to include test-scaffold, test-audit, and coverage-explain.
• Add 3 new skill definitions (SKILL.md) for scaffolding tests, auditing tests, and explaining coverage gaps.
• Add E3b behavior contract tests and mark E3b complete in ROADMAP.md.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
policy.yaml	Adds E3b skills to skills_allowed allowlist for Stop-hook enforcement.
ROADMAP.md	Marks E3b as complete and updates the branch tracking row.
.claude/skills/tests/test_e3b_behavior.py	New behavior contract tests for E3b skill framing/scope boundaries.
.claude/skills/tests/_allowed_skills.py	Extends canonical ALLOWED_SKILLS list with the 3 new skills.
.claude/skills/test-scaffold/SKILL.md	Defines writer-scoped scaffolding skill for creating a test-pair skeleton.
.claude/skills/test-audit/SKILL.md	Defines read-only advisory test audit skill (candidate-signal reporting).
.claude/skills/coverage-explain/SKILL.md	Defines read-only advisory coverage report explanation skill.

---

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

[Copilot]

The skill suggests reading .coverage directly as a report input, but .coverage is typically a coverage.py SQLite database and isn’t human-readable without running tooling to export JSON/XML. Consider prioritizing coverage.json, coverage.xml, or htmlcov/index.html as the readable artifacts, and treating .coverage as a hint that a report can be generated rather than something to parse directly.

[Copilot]

coverage/lcov.json is not a standard lcov artifact (lcov is usually lcov.info text, often under coverage/lcov.info or coverage/lcov-report/lcov.info). Updating the canonical path examples here will make the guidance work with common NYC/Vitest setups.

[Copilot]

The doc refers to "lcov.json" but then describes parsing SF:/LH:/LF: entries, which are part of the lcov text format (usually lcov.info), not JSON. Please align the format name and parsing description (either document lcov.info parsing, or document actual JSON coverage formats).

Suggested change

	- Search canonical paths: `coverage/lcov.json`, `.coverage`, `coverage.json`, `coverage.html/`, `htmlcov/`, `.nyc_output/coverage.json`, `coverage/coverage.json`.
	- If user provides a path arg, read that directly (Bash find to verify existence).
	- If no report found, declare: "No coverage report detected in canonical locations. Run your test suite with coverage enabled first (e.g., `npm run test-coverage`, `pytest --cov`)."
	- Declare which format was found (lcov.json, pytest-cov JSON, NYC, etc.).
	2. **Parse report** (Read + analyze):
	- **lcov.json**: extract file entries (`SF:filename`, `LH:lines_hit`, `LF:lines_found`); compute per-file coverage %.
	- Search canonical paths: `coverage/lcov.info`, `.coverage`, `coverage.json`, `coverage.html/`, `htmlcov/`, `.nyc_output/coverage.json`, `coverage/coverage.json`.
	- If user provides a path arg, read that directly (Bash find to verify existence).
	- If no report found, declare: "No coverage report detected in canonical locations. Run your test suite with coverage enabled first (e.g., `npm run test-coverage`, `pytest --cov`)."
	- Declare which format was found (lcov.info, pytest-cov JSON, NYC, etc.).
	2. **Parse report** (Read + analyze):
	- **lcov.info (lcov text format)**: extract file entries (`SF:filename`, `LH:lines_hit`, `LF:lines_found`); compute per-file coverage %.

[Copilot]

The section header comment above still says the allowlist is "E3a 10 skills", but this PR extends it to 13 (E3b). Please update that comment so the documentation matches the actual list size/phase progression.

[Copilot]

ALLOWED_SKILLS is imported but never used in this test module, and the preceding sys.path manipulation exists only to support that import. Either remove the unused import (and sys.path tweak) or actually use ALLOWED_SKILLS for parametrization.

Suggested change

	import sys
	from pathlib import Path
	import pytest
	REPO_ROOT = Path(__file__).resolve().parents[3]
	SKILLS_DIR = REPO_ROOT / ".claude" / "skills"
	SKILLS_TEST_DIR = REPO_ROOT / ".claude" / "skills" / "tests"
	if str(SKILLS_TEST_DIR) not in sys.path:
	sys.path.insert(0, str(SKILLS_TEST_DIR))
	from _allowed_skills import ALLOWED_SKILLS # noqa: E402
	from pathlib import Path
	import pytest
	REPO_ROOT = Path(__file__).resolve().parents[3]
	SKILLS_DIR = REPO_ROOT / ".claude" / "skills"

[Copilot]

read_skill_body() re-implements the frontmatter split/parsing logic instead of reusing the existing read_skill() helper from test_skill_frontmatter.py. Reusing the shared helper would avoid delimiter drift and gives clearer assertion failures when SKILL.md frontmatter is malformed.

Suggested change

	def read_skill_body(slug: str) -> str:
	"""Return only the body (post-frontmatter) of a SKILL.md."""
	path = SKILLS_DIR / slug / "SKILL.md"
	raw = path.read_text(encoding="utf-8")
	parts = raw.split("---\n", 2)
	return parts[2] if len(parts) >= 3 else ""
	from test_skill_frontmatter import read_skill # noqa: E402
	def read_skill_body(slug: str) -> str:
	"""Return only the body (post-frontmatter) of a SKILL.md."""
	_, body = read_skill(slug)
	return body

[Copilot]

This test claims to verify that Write is present in the allowed-tools frontmatter, but it only checks whether the string "Write" appears anywhere in the raw SKILL.md. That can pass even if Write is missing from frontmatter (e.g., if the body mentions "Write"). Parse the frontmatter and assert membership in the allowed-tools list instead.

[Copilot]

Docstring wording: "when convictions ambiguous" reads like the wrong word choice in English. Consider replacing "convictions" with something like "conclusions"/"confidence" to match the intent.

Suggested change

	- Ensure STOP boundaries are explicit when convictions ambiguous or output uncertain.
	- Ensure STOP boundaries are explicit when confidence is ambiguous or output uncertain.

[javiAI]

Review applied — 8/8 fixed (commit 0113551)

All 8 inline comments from @copilot triaged FIX (no DISCUSS, no SKIP):

Coverage-explain doc corrections (factually correct)

• ✅ Comment 1 — .coverage is coverage.py's SQLite DB; reframed as a hint that JSON/XML can be generated (not parsed directly).
• ✅ Comment 2 — canonical lcov path corrected: coverage/lcov.json → coverage/lcov.info.
• ✅ Comment 3 — Step 2 format/parse mismatch resolved: documented as "lcov.info (lcov text format)" with SF:/LH:/LF: parsing (was claiming JSON parsing on text-format markers).

Era-bound drift carryover

• ✅ Comment 4 — policy.yaml header above skills_allowed: extended from "→ E3a 10 skills" to "→ E3a 10 skills → E3b 13 skills". Same class of drift as the test renames in 9312a69.

Test correctness

• ✅ Comment 5 — unused ALLOWED_SKILLS import replaced with read_skill from test_skill_frontmatter (the sys.path.insert now has a real consumer).
• ✅ Comment 6 — read_skill_body collapsed to one-line reuse of the shared read_skill (no more duplicated frontmatter splitter).
• ✅ Comment 7 — test_scaffold_allowed_tools_includes_write now parses frontmatter and asserts list membership (mirrors simplify's pattern). Previous raw-text check had a false-positive risk.
• ✅ Comment 8 — docstring typo fixed: "convictions ambiguous" → "confidence ambiguous".

Tests: 777 passed + 1 skipped on full hooks/tests/ + .claude/skills/tests/ suite. No regression.

[Copilot]

Review applied — 8/8 fixed (commit 0113551)

All 8 inline comments from @copilot triaged FIX (no DISCUSS, no SKIP):...

Confirmed — all 8 fixes from commit 0113551 verified locally: 777 passed + 1 skipped. The three correctness fixes (coverage/lcov.json → lcov.info, .coverage reframed as SQLite hint, Step 2 format alignment), the policy.yaml comment drift fix, and the three test improvements (unused import replaced with read_skill reuse, frontmatter-parsed allowed-tools assertion, docstring typo) all look correct. No regressions.