Skip to content

Add LRV skill for guardrails, configuration, and QC interpretation - #86

Open
cmungall wants to merge 2 commits into
mainfrom
cmungall/portable-repo-skills
Open

cmungall wants to merge 2 commits into
mainfrom
cmungall/portable-repo-skills

Conversation

@cmungall

@cmungall cmungall commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

Add a portable LRV skill for the agent's work around deterministic reference QC: integrating repository guardrails, interpreting findings and incomplete coverage, and configuring extraction, sources, and matching policy. A passing quote check does not require an agent to repeat it, and does not establish that the quote supports the attached scientific claim.

The main skill focuses on diagnosis and evidence-based correction. Separate reference guides cover hook/CI setup and configuration, using pinned examples from dismech's project wrappers and its advisory pre-edit reference check. They distinguish missing content from mismatches, preserve repository severity/skip policy during repairs, and document proposed-content validation and worktree path handling. The README includes list-before-install npx skills commands and project/global scope; skill installation does not activate hooks or CI.

Validation:

  • 34 focused tests passed (tests/test_validation_config.py, tests/test_validation_counts.py).
  • Six Typer CliRunner workflow cases passed against local source fixtures and the documented config: matching nested evidence, lookup with explicit config, quote mismatch, wrong title, zero comparisons, and unavailable source.
  • Skill frontmatter, Markdown lint (MD013 excluded), relative/external links, and local npx skills add <worktree> --list passed. No new README lint violations relative to the previous PR head.
  • Documentation only; no hook installed or live skill environment changed. Canonical default-branch discovery becomes available after merge.

@claude

claude Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @cmungall's task in 2m 9s —— View job


Review: Add portable reference-validation skill

  • Read the PR diff
  • Verify SKILL.md claims against the actual CLI
  • Check README instructions and docs links
  • Post review

Overall this is a well-written skill. I checked every CLI claim in SKILL.md against src/linkml_reference_validator/cli/ and almost all of it is accurate — positional order for validate text (text, then reference), --title, lookup's multi-ID behavior and its "exit 0 if at least one succeeded" semantics (cli/lookup.py:259, :280), the oa:exact / dcterms:references canonical plus linkml:excerpt / linkml:authoritative_reference legacy interfaces (field_detection.py:82-157), the "exit 0 can mean zero comparisons" warning (cli/validate.py:369-371), and the repair data nested-reference limitation (cli/repair.py:375 — d.get("reference") takes a dict as-is, so reference: {id: PMID:...} really does break). The guardrail framing (never invent a PMID, never rewrite a quote to pass) is the right emphasis for an agent skill.

Two things I'd fix before merge.

1. The uvx examples for validate data will fail out of the box — SKILL.md:48

linkml is a dev-group dependency only (pyproject.toml:30-33), not a runtime dependency or an extra. validate data guards on it and exits 2:

Error: `linkml` is required for `validate data`.

(cli/validate.py:274-281)

So uvx linkml-reference-validator validate data ... — the exact command in the skill — never reaches validation. An agent following the skill hits an install error at the most important step. Either change the example to uvx --with linkml linkml-reference-validator validate data ..., or add an extra (e.g. [project.optional-dependencies] validation = ["linkml>=1.9.3"]) and use uvx --from 'linkml-reference-validator[validation]' .... The extra is the nicer long-term answer since README.md installs via plain uv pip install and has the same gap. repair data is unaffected (it uses SupportingTextRepairer, no linkml import). Fix this →

2. Wrong docs host — SKILL.md:95

The link points at https://linkml.io/linkml-reference-validator/reference/cli/, but the site is published at linkml.github.io (mkdocs.yml:62, and both README.md:1257 and docs/tutorials/complete-workflow.md:897 use that host). I couldn't fetch either URL from this runner to confirm, but the repo is internally consistent on linkml.github.io. Fix this →

Smaller points

  • file: supports relative paths too (SKILL.md:30). FileSource's doctest uses file:./notes.md (etl/sources/file.py:39). "absolute path" reads as a requirement; file:/path/to/source.txt or a relative path is fine.
  • Test coverage. CLAUDE.md mandates TDD, and this lands 116 lines with no test. A cheap guard would pay for itself: parse SKILL.md's frontmatter (assert name/description present) and extract every linkml-reference-validator <group> <cmd> invocation from the fenced blocks, asserting each resolves in the Typer app via CliRunner. That's a real test, not a mock, and it catches exactly the class of drift that'll happen when the CLI is restructured again.
  • --output still writes a .bak. cli/repair.py:163-166 backs up the input file unconditionally (when create_backup), even when --output redirects the write. The skill's "the CLI otherwise overwrites input with a backup" is accurate for the default path; just noting the .bak appears in both cases, in case you want to say so.
  • The cache command group is unmentioned. "Preserve reference cache contents for reproducibility" is good advice that would be more actionable with a pointer to cache lookup / cache export.
  • README. I can't verify the npx skills add flag spellings without network access — worth one manual npx skills add linkml/linkml-reference-validator --list against the merged default branch, since as the PR body notes, remote discovery finds nothing until this lands.

No security or performance concerns; the change is documentation-only and touches no runtime code.
• branch cmungall/portable-repo-skills

@claude

claude Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @cmungall's task in 2m 13s —— View job


Re-review: Add portable reference-validation skill

  • Read the updated diff
  • Re-check my earlier findings against the current files
  • Verify every SKILL/reference claim against the CLI and config model
  • Post review

The skill was substantially reframed since my last pass — SKILL.md is now guardrail/interpretation guidance with the concrete recipes pushed into references/. That's an improvement: the top-level file no longer asserts CLI surface that drifts, and the "interpret the result before repairing it" table is the right shape for an agent. My earlier line references are stale; here's a fresh pass against the current files.

I re-verified essentially every factual claim in references/ against the source, and they hold up:

Claim Verified against
-c is cache dir, not config cli/shared.py:14-20
--config accepts validation: / legacy reference_validation: / flat fields cli/shared.py:_extract_validation_config_data
Misplaced envelope silently leaves defaults in effect same function returns None → bare ReferenceValidationConfig()
.linkml-reference-validator.yaml / .yml autodiscovery from CWD cli/shared.py:82-88
skip_prefixes is case-insensitive validation/supporting_text_validator.py:120-124, :231-233 (both sides uppercased)
Private research cache is never read by ordinary validation models.py:390-400, etl/reference_fetcher.py:407
reference_base_dir for relative file: refs models.py:402-405
validate text is text first, reference second; --title cli/validate.py
repair text "QUOTE" REF_ID positional order; repair data --dry-run defaults on cli/repair.py:195-197, :77
validate text-file regex + --text-group / --ref-group cli/validate.py:103-129
--full-text/--no-full-text, default on, not an offline switch cli/shared.py:48-53
Lookup succeeds when only some IDs resolve cli/lookup.py
oa:exact / dcterms:references + legacy linkml:excerpt / linkml:authoritative_reference field_detection.py:82-157
repair data expects scalar reference IDs; reference: {id: ...} unsupported cli/repair.py:375

The file: "absolute path" nit from last round is fixed (configuration.md:59-61 now says absolute or reference_base_dir), and the --output wording is now accurate.

Two findings remain open.

1. The validate data recipe will fail for a project that followed the install instruction directly above it — references/guardrails.md:14 → :23-27

The doc says to add LRV with uv add linkml-reference-validator (line 14), then gives:

uv run --locked linkml-reference-validator validate data data.yaml \
  --schema schema.yaml --target-class Statement ...

But linkml is in [dependency-groups] dev only (pyproject.toml:33) — not in [project] dependencies, and there is no [project.optional-dependencies]. uv add linkml-reference-validator in a downstream project installs the runtime deps only, so validate data hits its guard and exits 2:

Error: `linkml` is required for `validate data`.

(cli/validate.py:274-281)

This is the same gap I flagged last round; it moved from a uvx example into the uv add + uv run --locked recipe, but the outcome for an agent following the skill is identical — the setup section leads straight into a command that can't run. Either have guardrails.md say uv add linkml-reference-validator linkml (and note that validate data needs linkml while the text-only commands don't), or add [project.optional-dependencies] validation = ["linkml>=1.9.3"] and instruct uv add 'linkml-reference-validator[validation]'. The extra is the better answer — README.md's install section has the same gap. repair data is unaffected. Fix this →

2. Wrong docs host (still) — references/configuration.md:98

https://linkml.io/linkml-reference-validator/reference/cli/

mkdocs.yml:62 sets site_url: https://linkml.github.io/linkml-reference-validator, and README.md:1259, docs/tutorials/complete-workflow.md:897, and both notebooks use linkml.github.io. The path itself is right (docs/reference/cli.md, nav at mkdocs.yml:54) — only the host is wrong. Since this is the one outbound link an agent is told to consult, a 404 here is costly. Fix this →

Smaller points

  • Still no test. CLAUDE.md mandates TDD, and this adds 288 lines with nothing under tests/ referencing skills/ or SKILL.md. A cheap real guard: parse the frontmatter (assert name/description), then extract every linkml-reference-validator <group> <cmd> invocation from the fenced blocks in SKILL.md and references/*.md and assert each resolves in the Typer app via CliRunner. That catches exactly the drift class that produced finding Bump actions/checkout from 4 to 5 #1 and the stale-host link, and it's not a mock.
  • --output still writes a .bak of the input. configuration.md:95-96 says --no-dry-run --output repaired.yaml "preserves the input; otherwise the CLI overwrites it with a backup" — accurate, but cli/repair.py:161-166 copies data_file → .bak unconditionally whenever a fix is applied, including the --output path. Worth a half-sentence if you don't want an agent surprised by a stray .bak in a hook-clean worktree.
  • -t is overloaded across subcommands. --text-group on validate text-file (cli/validate.py:117) vs --target-class on validate data / repair data (cli/validate.py:241, cli/repair.py:74). configuration.md already says to check --help; spelling out long options in generated commands would be safer advice for an agent copying between subcommands.
  • cache group is still unmentioned. "Preserve the source cache as evidence" (SKILL.md:55) and "populate its cache" (configuration.md:56) would be more actionable with a pointer to cache lookup / cache export / cache stats.
  • README npx flags unverified. No network on this runner, so I can't confirm --list, --skill, -a, -g spellings or that remote discovery works — as the PR body notes, that only becomes testable after merge. Worth one manual npx skills add linkml/linkml-reference-validator --list against the merged default branch.

No security or performance concerns — documentation-only, no runtime code touched.
• branch cmungall/portable-repo-skills

@cmungall cmungall changed the title Add portable reference-validation skill Add LRV skill for guardrails, configuration, and QC interpretation Sep 20, 2026

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

None yet

Development

Successfully merging this pull request may close these issues.

1 participant