fix(charmlint): docs and licence rules are monorepo-aware (#65) - #207
tonyandrewmeyer wants to merge 1 commit into
Conversation
DOC002-DOC005 (installation / configuration / usage / troubleshooting docs) looked only in `<charm>/docs`, and STR001 looked only for `<charm>/LICENSE`. A monorepo keeps its charms under `charms/<name>/` with one shared `docs/` tree and one licence at the repository root, so every charm in such a repository was reported as undocumented and unlicensed — five false positives per charm. `CharmContext` now records the enclosing `repo_root` (the nearest ancestor with a `.git` entry, tested with `exists()` so linked worktrees and submodules count) and exposes `search_roots()`: the charm directory first, then the repository root when it differs. The docs and licence rules walk that list, so charm-local assets still win and a charm outside a repository behaves exactly as before. Mirrored in the Rust implementation so the two stay in step. The `_check_doc_topic` keyword test also moves out of the `try` block, per the convention of minimising code inside try/except. Twelve tests per implementation cover the shared-docs and shared-licence layouts, the `.git`-as-a-file worktree case, `search_roots()` itself, and regression guards that a charm outside a repository, and a repository with no shared assets, are still flagged. Closes #65 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018wfGexXpXdSyPkTuxqAXnF
|
The What is failing
Why it is not this PR's
The reason this surfaces here rather than on #203 itself is that the workflow's I have not spent a re-run on it: the error is a deterministic config-parse failure with an identified root cause, so a re-run would reproduce it exactly, and the run history above establishes the "not this PR's" case more strongly than a re-run would. Proposed patch (not applied here) [defaults]
-model = "claude-sonnet-4-20250514"
+model = "anthropic/claude-sonnet-4-20250514"One caveat on that patch, stated plainly because I could not verify it: the error text asks for A separate, pre-existing issue in the same log Warden cannot post its check runs at all:
This PR's own status: everything else is green — Lint, Type Check, Build, Docs, both Rust crates, E2E, zizmor, Dependency Review, Workflow Security. The two Python test matrix jobs were still running when I wrote this. Given Generated by Claude Code |
Closes #65.
What changed
DOC002–DOC005(installation / configuration / usage / troubleshooting docs) looked only in<charm>/docs, andSTR001looked only for<charm>/LICENSEor<charm>/LICENCE. A monorepo keeps its charms undercharms/<name>/with a single shareddocs/tree and one licence at the repository root, so every charm in such a repository was reported as undocumented and unlicensed — five false positives per charm, none of them actionable.CharmContextnow records the enclosingrepo_rootand exposessearch_roots():repo_rootis the nearest ancestor holding a.gitentry. It is tested withexists()rather thanis_dir(), because.gitis a plain file in linked worktrees and submodules, which are repository roots just the same.search_roots()returns the charm directory first, then the repository root when it is a different directory.The docs and licence rules walk that list. Two consequences worth stating explicitly: charm-local assets still win (a charm with its own
docs/installation.mdis unaffected), and a charm outside a repository gets a one-element list, so its behaviour is byte-for-byte what it was before.The same change is mirrored in the Rust implementation (
models.rs,context.rs,rules.rs) so the two do not drift.One incidental tidy-up inside the function I was already editing: the
keyword in contenttest moves out of thetryblock in_check_doc_topic, per the CLAUDE.md convention of minimising code insidetry/except. No behaviour change.Why it is worth doing
It is a false-positive class, which is the expensive kind for a linter the agent reads: five unactionable findings per charm in a monorepo train the agent (and the human) to discount DOC and STR output generally. The repository-root walk is also the zero-config fix — nothing to configure, and it matches where the META
docslink already points.Scope I deliberately left out
docs-dirkey in.charmlint.yamlwould be the right follow-up if someone keeps docs somewhere other than<root>/docs, but adding a config surface speculatively seemed worse than not having one.DOC001(no README) is unchanged, on purpose. A shared rootREADME.mddoes not document a specific charm, so falling back there would turn a true positive into a miss. Same reasoning forSTR002(icon) and thetests/checks — those are genuinely per-charm.cargo fmt. The crate is already 73 diffs away from rustfmt-clean in files I did not touch, so running it would have been a repo-wide reformat. I matched the surrounding hand-formatting instead. Flagging it as pre-existing rather than silently leaving it.Things I am less sure about
_check_doc_topicis called once per topic, so a monorepo's rootdocs/tree is now walked four times instead of zero. The charm-local tree was already walked four times, so this is the existing shape rather than a new one, and on a realistic docs tree it is a few milliseconds. But if the Rust implementation's sub-30 ms budget matters on a large monorepo, the better fix is to read the docs content intoCharmContextonce at build time and have all four rules share it. I did not do that here because it widens the change past the bug..gitas the repository marker. Charms vendored into a non-git tree (an extracted tarball, ahgcheckout) still get the old behaviour. That seemed like the right trade against guessing a root from, say, the presence of a top-levelcharms/directory.How I verified it
make checkis clean on the branch:make checkdoes not cover everything CI does, so I also ran the rest of the CI gates:uv run pytest tests/integration -n auto→193 passed, 14 skippedcargo test --all-targetson both crates → greenmake docs-check-strict→docs: rebuilt HTML matches committed outputCoverage total is unchanged at 93% because
--cov=cantripdoes not measure thecharmlintpackage; the new tests are counted by neither the before nor the after figure.Tests are negative-checked. Reverting
src/charmlint/and re-running the new Python tests fails 8 of 12; the 4 that still pass are the regression guards, which are supposed to hold either way. Twelve equivalent tests exist on the Rust side. Between them they cover the shared-docs layout, partial shared docs (only the documented topic stops being flagged), sharedLICENSEandLICENCE,.git-as-a-file, charm-local docs still winning,search_roots()directly including the collapse when the charm is the repository root, and the two regression guards (charm outside a repository, repository with no shared assets).No user-facing surface changed — no CLI flag, env var, slash command or config key — and the user docs carry no rule catalogue, so there is nothing under
docs/src/to update.CHANGELOG.mdhas an## Unreleasedentry.🤖 Generated with Claude Code
https://claude.ai/code/session_018wfGexXpXdSyPkTuxqAXnF
Generated by Claude Code