Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates repository guidance/docs to reflect the current detection rule totals and strengthens an integration test that builds Kingfisher crates from a temporary “external” Cargo project.
Changes:
- Updated user-facing rule count references to 942 across docs-site templates and configuration.
- Expanded agent documentation (AGENTS.md) with rule-count breakdown, docs-site structure notes, and rebuild guidance; added CLAUDE.md pointer to AGENTS.md.
- Hardened
library_crates_external_projectintegration test by copyingCargo.lock, generating a lockfile offline, and runningcargo run --frozenfor reproducibility.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/library_crates_external_project.rs | Improves external-project build determinism via lockfile handling and --frozen execution. |
| docs-site/overrides/main.html | Updates JSON-LD description to reflect the new rule count. |
| docs-site/overrides/home.html | Updates homepage stat for detection rules. |
| docs-site/mkdocs.yml | Updates site description to reflect the new rule count. |
| CLAUDE.md | Adds instruction to follow AGENTS.md before starting work. |
| AGENTS.md | Updates rule count/breakdown and adds docs-site workflow guidance and structure notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Add a detection rule: follow the workflow below and validate with relevant tests. | ||
| - Add a CLI command: implement under `src/cli/commands/` and register in the CLI command wiring. | ||
| - Add a validator (rare exception path): implement it in `crates/kingfisher-scanner/src/validation/`, prefer `raw.rs` for one-off provider flows, and wire the narrowest feature/dependencies in `crates/kingfisher-scanner/Cargo.toml` only when YAML validation cannot express the required logic. | ||
| - Update docs-site rule counts: use `uv run '/Users/mickg/src/kingfisher/data/default/rule_cleanup/count_rules.py'` and update `docs-site/overrides/` plus `docs-site/mkdocs.yml` to match the reported totals before rebuilding the docs site. |
There was a problem hiding this comment.
This instruction hard-codes a developer-specific absolute path (/Users/mickg/...), which will be wrong for everyone else and in CI. Use a repo-relative path (e.g., data/default/rule_cleanup/count_rules.py) and let the script’s own DEFAULT_RULES_DIR handle locating rules, or document running it from repo root.
| - Update docs-site rule counts: use `uv run '/Users/mickg/src/kingfisher/data/default/rule_cleanup/count_rules.py'` and update `docs-site/overrides/` plus `docs-site/mkdocs.yml` to match the reported totals before rebuilding the docs site. | |
| - Update docs-site rule counts: use `uv run data/default/rule_cleanup/count_rules.py` and update `docs-site/overrides/` plus `docs-site/mkdocs.yml` to match the reported totals before rebuilding the docs site. |
| - If validation commands cannot be run, report exactly what was skipped and why. | ||
| - Prefer `kingfisher scan --format toon` when invoking Kingfisher from an LLM or agent workflow; keep `pretty` for interactive human CLI use unless the task explicitly calls for a different format. | ||
| - After markdown/doc changes, verify local documentation links when practical. | ||
| - After `docs-site/` source changes, rebuild with `docs-site/.venv/bin/mkdocs build -f docs-site/mkdocs.yml` when practical so checked-in generated output stays in sync. |
There was a problem hiding this comment.
The rebuild command references docs-site/.venv/bin/mkdocs, but docs-site/.venv isn’t present in the repo and the docs workflow builds with cd docs-site && mkdocs build --strict (see .github/workflows/docs.yml:44). Recommend documenting the same command (or uv run mkdocs ...) to avoid a non-existent/host-specific venv path.
| - After `docs-site/` source changes, rebuild with `docs-site/.venv/bin/mkdocs build -f docs-site/mkdocs.yml` when practical so checked-in generated output stays in sync. | |
| - After `docs-site/` source changes, rebuild with `cd docs-site && mkdocs build --strict` when practical so checked-in generated output stays in sync. |
This pull request updates documentation and test logic to reflect the increased number of detection rules, improves documentation accuracy and workflows, and strengthens the integration test for external project builds. The most important changes are grouped below.
Documentation and Rule Count Updates:
AGENTS.md,docs-site/mkdocs.yml, anddocs-site/overrides/home.htmlto ensure consistency across user-facing documentation. [1] [2] [3] [4]AGENTS.mdto update rule counts in documentation using a specific script and to sync totals indocs-site/overrides/anddocs-site/mkdocs.ymlbefore rebuilding the docs site.docs/viewer/anddocs-site/directories inAGENTS.md.docs-site/sources.CLAUDE.mdto read and followAGENTS.mdbefore starting any task.Test Improvements:
library_crates_external_project.rs, now copiesCargo.lockfrom the main repo and explicitly generates the lockfile offline before running the test build, then runscargo run --frozento ensure reproducible builds and better test isolation. [1] [2]