diff --git a/.claude/knowledge/architecture-boundaries.md b/.claude/knowledge/architecture-boundaries.md index e18361c..427f786 100644 --- a/.claude/knowledge/architecture-boundaries.md +++ b/.claude/knowledge/architecture-boundaries.md @@ -30,3 +30,6 @@ Key architectural decisions, service boundaries, data flow, integration points, - **The usage menu makes one outbound call (opt-out).** Running `codeguard` with no args or `help` renders a "What's New" banner (`internal/whatsnew` + `internal/cli/whatsnew.go`) that queries the GitHub releases API for the latest version. It is disabled when `CODEGUARD_NO_UPDATE_CHECK` is truthy OR `$CI` is non-empty (so CI/automated runs make no surprise call), caches the result for 24h under `os.UserCacheDir()/codeguard/update-check.json`, uses the SSRF-guarded `ai/safehttp.Client` with a 1.5s timeout, and fails silent (falls back to stale cache, then to version-only). Changelog bullets come from `CHANGELOG.md` embedded via a **module-root package** (`/changelog.go`, `package codeguard`, imported as `root "github.com/devr-tools/codeguard"`) — `go:embed` cannot reference `../CHANGELOG.md`, so the embed directive must live at the repo root, not in `internal/`. - **Lint is enforced in CI.** `make lint` is `go vet`; `make lint-strict` / the CI `golangci-lint-action` step (now blocking — no `continue-on-error`) runs the 16-linter `.golangci.yml`. `golangci-lint run` must report 0 before pushing. The config excludes revive's documentation/naming style rules (`exported`, `package-comments`, `unused-parameter`) and excludes `tests/` from gosec/errcheck/prealloc/bodyclose (intentional fixtures). Cache/identity keys use sha256 (not sha1). Documenting the public `pkg/codeguard` API with doc comments is a known follow-up (revive `exported` is currently excluded rather than satisfied). - **Adding a check section touches a fixed wiring list; two are silent-failure traps.** Beyond the obvious (check package with `Run(ctx, env) core.SectionResult`, a `sectionDef` in `runner/checks/registry.go`, a `CheckConfig` bool + rules struct in `core/config_types.go`/`config_rule_types.go`, catalog + fix-template maps merged in `rules/catalog.go`/`catalog_fix_templates.go`, defaults in `config/defaults*.go` + `example*.go`, docs): (1) `runner/support/cache_helpers.go` — add the section to `SectionConfigHashes` AND `sectionConfigFamily`, else the section's per-file cache keys fall back to the all-checks fingerprint (correct but invalidates on any config change); (2) top-level `CheckConfig` bools get NO defaulting in `applyCheckDefaults` — an omitted `checks.
` key means **off**, which is how the opt-in `performance` and `supply_chain` sections work. The performance section (v0.9.0) is the reference move: rules migrated from quality with renamed ids (`quality.*` → `performance.*`), no runtime aliasing, `rules.RetiredPerformanceRuleIDs()` powers a `codeguard doctor` waiver migration hint. +- **PERF: the quality AI checks read through the corpus via `checks/support.Context.ListTargetFiles`/`ReadTargetFile`** (wired in `runner/checks/checks.go`, backed by `runnersupport.ListTargetFiles`/`ReadTargetFile`; `checks/quality/quality_ai_corpus.go` holds the nil-safe fallbacks for unit-test contexts). GOTCHA when routing more reads through the corpus: `corpusRead` caps at 32 MiB (`readCappedFile`) while `os.ReadFile` does not — walk-enumerated files are already under the cap so behavior is identical, but fixed-filename reads that never went through the walk (`go.mod`, root `package.json`, python manifests) deliberately stay direct `os.ReadFile` to preserve semantics. The Go repo-dominant style signals (test framework, error style, naming) are computed in ONE corpus pass (`goRepoStyleProfile` in `quality_ai_target_go.go`); each signal is an independent per-file sum, which is what makes the fold behavior-identical. +- **GOTCHA: `goFuncEligibleForUnusedCheck` must not consult `fn.Doc`.** The AI dead-code pass historically parsed with `parser.ParseFile(..., 0)`, so `fn.Doc` was always nil and the "go: directive implies external use" exemption NEVER fired. The pass now reuses the shared ParseComments corpus AST (`support.ParseGoSource`), where `fn.Doc` IS populated — keeping that check would have silently exempted unused functions carrying `//go:generate`-style comments and changed findings. It was removed to keep output identical (see the NOTE in `quality_ai_dead_code.go`); activating it is a candidate behavior fix, to be made deliberately with tests. +- **PERF: clone-detector hashing is not behavior-bearing.** `cloneToken` now stores the original-case source slice plus a per-token FNV-1a hash of the ASCII-lowercased text (tokens are ASCII-only by the regex); window grouping uses a rolling polynomial hash (`cloneWindowIndex`) and equality is verified token-by-token in `sharedCloneLength` (hash short-circuit + ASCII case-fold compare). Because every candidate is value-verified, ANY deterministic hash yields identical clone candidates — collisions only cost time. `tests/checks/quality_clone_differential_test.go` pins findings captured from the pre-rewrite algorithm; don't regenerate its expectations with the current code. diff --git a/.codeguard/codeguard.yaml b/.codeguard/codeguard.yaml index 628d37e..6a9bec5 100644 --- a/.codeguard/codeguard.yaml +++ b/.codeguard/codeguard.yaml @@ -4,6 +4,7 @@ exclude: - .codeguard/cache.json - .codeguard/cache.slop-history.json - .codeguard/cache.perf-history.json + - .codeguard/cache.legibility-history.json - .gomodcache/** - tests/**/.codeguard/cache.json # Detection-precision fixtures: intentionally vulnerable-looking sample @@ -77,6 +78,10 @@ checks: kind: file-size path: README.md max_bytes: 65536 + context_rules: + # Enforceable AI-readiness gate: warn if the repo_legibility score drops + # below 85 (the repo currently scores 100; see docs/checks.md). + legibility_warn_threshold: 85 ci_rules: require_workflow_dir: true required_workflow_files: diff --git a/.gitignore b/.gitignore index acc328f..d7eeaa8 100644 --- a/.gitignore +++ b/.gitignore @@ -39,3 +39,4 @@ go.work.sum **/.codeguard/cache.slop-history.json **/.codeguard/cache.perf-history.json **/.codeguard/cache.rule-stats-history.json +**/.codeguard/cache.legibility-history.json diff --git a/CLAUDE.md b/CLAUDE.md index 9f300ac..ca4559b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ agent context — and reports pass/warn/fail findings per rule. ## Build, test, verify -- `make build` → `dist/codeguard`; `make test`; `make lint` (vet) and `make lint-strict` (golangci-lint, CI-blocking, must be 0 issues) +- `make build` → `dist/codeguard`; `make test`; `make fmt` (gofmt the tree); `make lint` (vet) and `make lint-strict` (golangci-lint, CI-blocking, must be 0 issues) - `make codeguard-ci` — validate + self-scan with `.codeguard/codeguard.yaml` - `make check` — the full CI gate (fmt-check, lint, test, codeguard-ci) - Scope gofmt to `gofmt -l cmd internal pkg tests changelog.go` (repo root picks up worktrees/module caches) diff --git a/README.md b/README.md index 1d685a7..5407414 100644 --- a/README.md +++ b/README.md @@ -95,13 +95,13 @@ codeguard baseline -config codeguard.yaml -output codeguard-baseline.json `codeguard rules` prints each rule's level, execution model, language coverage, section, and title. `codeguard explain ` includes the same metadata for a single rule. -By default, `codeguard` looks for `codeguard.yaml`, `codeguard.yml`, or `codeguard.json` in the repository root. If those are missing, it also checks `.codeguard/codeguard.yaml`, `.codeguard/codeguard.yml`, and `.codeguard/codeguard.json`. +By default, `codeguard` looks for `codeguard.yaml`, `codeguard.yml`, or `codeguard.json` in the repository root. If those are missing, it also checks for the same file names inside a `.codeguard/` directory. If you point `-config` at a directory such as `.codeguard`, `codeguard` will look inside it for `codeguard.*` or `config.*` files. Text output includes ANSI color and emoji markers by default. Set `NO_COLOR=1` if you want plain terminal output. -If you want a JSON starting point instead, use [examples/codeguard.json](/Users/alex/Documents/GitHub/codeguard/examples/codeguard.json:1). +If you want a JSON starting point instead, use [examples/codeguard.json](examples/codeguard.json). ## SDK @@ -129,16 +129,16 @@ func main() { ## Docs -- [Getting started](/Users/alex/Documents/GitHub/codeguard/docs/getting-started.md:1) -- [Features](/Users/alex/Documents/GitHub/codeguard/docs/features.md:1) -- [Security & OWASP](/Users/alex/Documents/GitHub/codeguard/docs/security.md:1) -- [AI-generated code quality](/Users/alex/Documents/GitHub/codeguard/docs/ai-quality.md:1) -- [Agent-native features](/Users/alex/Documents/GitHub/codeguard/docs/agent-native.md:1) -- [Integrations](/Users/alex/Documents/GitHub/codeguard/docs/integrations.md:1) -- [Hook-pack examples](/Users/alex/Documents/GitHub/codeguard/examples/hooks/README.md:1) -- [SDK guide](/Users/alex/Documents/GitHub/codeguard/docs/sdk.md:1) -- [Release automation](/Users/alex/Documents/GitHub/codeguard/docs/release-automation.md:1) -- [Homebrew packaging](/Users/alex/Documents/GitHub/codeguard/docs/homebrew.md:1) -- [Checks reference](/Users/alex/Documents/GitHub/codeguard/docs/checks.md:1) -- [Architecture](/Users/alex/Documents/GitHub/codeguard/docs/architecture.md:1) -- [Competitive roadmap](/Users/alex/Documents/GitHub/codeguard/docs/competitive-roadmap.md:1) +- [Getting started](docs/getting-started.md) +- [Features](docs/features.md) +- [Security & OWASP](docs/security.md) +- [AI-generated code quality](docs/ai-quality.md) +- [Agent-native features](docs/agent-native.md) +- [Integrations](docs/integrations.md) +- [Hook-pack examples](examples/hooks/README.md) +- [SDK guide](docs/sdk.md) +- [Release automation](docs/release-automation.md) +- [Homebrew packaging](docs/homebrew.md) +- [Checks reference](docs/checks.md) +- [Architecture](docs/architecture.md) +- [Competitive roadmap](docs/competitive-roadmap.md) diff --git a/docs/architecture.md b/docs/architecture.md index effee19..783d716 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -52,5 +52,6 @@ This keeps the runner tree organized with the same directory-first style as `int - waivers and inline suppressions allow time-bounded exceptions - baselines suppress known findings so new regressions are the only gate failures - cached file findings are keyed by file hash plus config hash so repeat scans skip unchanged files +- a per-scan file corpus (`internal/codeguard/runner/support/corpus.go`) memoizes the target directory walk, individual file reads, and Go/tree-sitter parses so each file is walked, read, and parsed at most once per scan no matter how many sections inspect it; the quality AI checks share it through the `ListTargetFiles`/`ReadTargetFile`/`ParseGoFile` context hooks rather than re-reading the tree - diff mode can scope file findings down to changed lines derived from `git diff` - report serialization supports plain text, structured JSON, SARIF, and GitHub workflow annotations diff --git a/docs/checks.md b/docs/checks.md index 2a3d1cb..684e891 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -940,10 +940,11 @@ Rules: Purpose: - Agent instruction file presence (CLAUDE.md, AGENTS.md, .cursorrules, .github/copilot-instructions.md) -- Drift between agent docs / README commands and the actual repository -- Agent context budget for individual source files +- Drift between agent docs / README references and the actual repository +- Canonical dev commands the docs never mention, and markdown links that rot +- Agent context budget for individual source files and for the agent docs themselves - Basename ambiguity that defeats filename-based navigation -- A `repo_legibility` artifact scoring how legible the repository is to AI agents +- A `repo_legibility` artifact scoring how legible the repository is to AI agents, with an enforceable score threshold and a persisted per-scan trend Config keys: @@ -957,8 +958,17 @@ Config keys: "detect_readme_drift": true, "detect_oversized_files": true, "detect_ambiguous_symbols": true, + "detect_undocumented_commands": true, + "detect_oversized_agent_docs": true, + "detect_doc_link_rot": true, "max_file_lines": 1500, - "ambiguous_symbol_threshold": 4 + "ambiguous_symbol_threshold": 4, + "max_agent_doc_lines": 600, + "ambiguous_symbol_ignore": ["index.ts", "__init__.py"], + "legibility_warn_threshold": 0, + "legibility_fail_threshold": 0, + "legibility_history": true, + "legibility_history_limit": 100 } } } @@ -969,9 +979,13 @@ When `checks.context` is omitted the family runs in full scans and is skipped in Current behavior: - `context.agent-docs-missing` warns once at repo level when none of the recognized agent instruction files exist at the target root - `context.agent-docs-drift` warns when an agent instruction file references a file or directory path, a `make` target, or an npm/pnpm/yarn `run` script that provably does not exist -- `context.readme-drift` applies the same resolution to fenced `bash`/`sh`/`shell` blocks in the root README.md: `./`-prefixed paths, make targets, and run scripts that resolve nowhere +- `context.readme-drift` applies the same full extraction to the root README.md as agent docs get: prose and inline-code references as well as fenced `bash`/`sh`/`shell` blocks — the README is the doc agents and humans read first, so it earns the same truthfulness bar - `context.oversized-context-unit` warns when a source file exceeds `context_rules.max_file_lines` (default 1500); the message is framed as agent context cost, distinct from `quality.max-file-lines` maintainability thresholds; generated and vendored files are skipped -- `context.ambiguous-symbol` warns once per source-file basename shared by at least `context_rules.ambiguous_symbol_threshold` files (default 4), listing up to five locations +- `context.ambiguous-symbol` warns once per source-file basename shared by at least `context_rules.ambiguous_symbol_threshold` files (default 4), listing up to five locations; conventional basenames in the ignore list (below) never fire +- `context.undocumented-commands` is the inverse of drift: it warns (up to 10 findings) when a high-signal Makefile target or package.json script — exactly `build`, `check`, `dev`, `fmt`, `lint`, `run`, `start`, `test`; prefixed variants like `fmt-check` are not implied — is mentioned by no agent instruction file and not even the root README. Any plausible mention counts as documentation: a structured reference (inline code, shell fence) or a plain-text `make ` / `npm|pnpm|yarn [run] ` invocation anywhere in the doc. The rule stays silent when the repo has no agent docs at all (`context.agent-docs-missing` already covers that), and when the Makefile or package.json cannot be parsed reliably (includes, pattern rules, workspaces) +- `context.oversized-agent-doc` warns when an agent instruction file exceeds `context_rules.max_agent_doc_lines` (default 600): agent docs are loaded into every session verbatim, so an oversized one consumes the context window it exists to save; the README and linked reference docs are free to be long +- `context.doc-link-rot` warns (up to 20 findings per doc) when a markdown link in an agent instruction file or the root README points at a repository file or directory that does not exist. Relative targets resolve against the doc's own directory and the repo root; absolute `/path` targets resolve against the repo root only (the hosted-viewer convention — an absolute filesystem path baked into a doc is exactly the rot this rule catches). External URLs (any scheme) are never fetched, pure `#anchor` links are skipped, `path.md#anchor` checks only the path part, editor-style `:line` suffixes are stripped, and templated or placeholder targets (``, `$VAR`, globs, `..` traversals, query strings) are exempt. Markdown links are owned by this rule; the drift rules no longer extract them, so one broken link is never reported twice +- `context.legibility-threshold` enforces the `repo_legibility` score: because legibility is good-high (the inverse of the slop score), the finding fires when the computed score falls **below** a configured floor — `warn` below `context_rules.legibility_warn_threshold`, `fail` below `context_rules.legibility_fail_threshold` (the fail level takes precedence when both match). `0` (the default) disables each threshold, and when both are set the fail threshold must be less than or equal to the warn threshold. The finding message carries the full component breakdown (e.g. `agent_docs 10/25, readme 10/10, ...`) so the weakest signal is immediately visible Drift resolution is deliberately conservative — precision over recall. It only flags references it can positively prove broken, and skips: - URLs, module/domain paths (`github.com/...`), absolute paths, and `..` traversals @@ -981,16 +995,37 @@ Drift resolution is deliberately conservative — precision over recall. It only - make targets when no root Makefile exists or the Makefile uses `include` or pattern rules - npm scripts when there is no root package.json or it declares workspaces +Conventional-basename ignore list: + +Basenames imposed by a language or framework convention are expected to repeat, so they neither fire `context.ambiguous-symbol` findings nor count against the `navigability` score component. The default set: +- JS/TS module entrypoints: `index.ts`, `index.tsx`, `index.js`, `index.jsx`, `index.mjs`, `index.cjs` +- file-system routers: `route.ts`, `routes.ts`, `page.tsx`, `layout.tsx` +- Python package markers: `__init__.py`, `__main__.py` +- Rust module layout: `mod.rs`, `lib.rs`, `main.rs` +- Go idioms: `main.go`, `doc.go`, `types.go` + +Setting `context_rules.ambiguous_symbol_ignore` **replaces** the default list entirely (matching is case-insensitive): re-list the defaults plus your own names to extend it, or set it to `[]` to disable ignoring altogether. + `repo_legibility` artifact: Every context run publishes one `repo_legibility` artifact per target with a 0-100 score (higher is more legible) and an explainable component breakdown: -- `agent_docs` (25): any agent instruction file present +- `agent_docs` (25): agent instruction files must exist *and* have substance — credit is `25 x min(non-blank lines, 10) / 10` measured on the largest agent doc (an empty CLAUDE.md scores 0, full credit from 10 non-blank lines), minus 2 points per unresolvable reference inside the agent docs (capped at 10); the component detail spells out both terms - `readme` (10): root README.md present -- `doc_accuracy` (20): minus 4 points per unresolvable doc/README reference -- `context_economy` (25): scaled down by the share of source files over the context budget (10% oversized zeroes it) -- `navigability` (20): scaled down by the share of source files caught in ambiguous basename groups (20% affected zeroes it) +- `doc_accuracy` (20): scaled by the share of doc/README references that resolve — penalty `round(20 x broken/total)`, so 2 broken of 10 costs 4 points while a doc set that is mostly wrong loses all 20; with no references the component stays at 20 +- `context_economy` (25): scaled by the share of source files over the context budget, ramping linearly to zero at 25% oversized (penalty `round(25 x share x 4)`, capped at 25) +- `navigability` (20): scaled down by the share of source files caught in ambiguous basename groups (20% affected zeroes it), computed after removing conventional basenames per the ignore list above + +The artifact is emitted even when individual rules are toggled off, so the score always reports reality. The `context.legibility-threshold` rule (above) turns the score into a warn/fail gate. + +Score history: + +The legibility score trend is persisted per target next to the scan cache (`.legibility-history.`, e.g. `cache.legibility-history.json`) whenever the cache is enabled; subsequent scans annotate the artifact with `previous_score` and `delta`. `context_rules.legibility_history: false` disables persistence and `context_rules.legibility_history_limit` caps retained entries per target (default 100). Print the recorded trend with: + +```bash +codeguard report -legibility-history [-config path] [-limit n] +``` -The artifact is emitted even when individual rules are toggled off, so the score always reports reality. +(mirroring `codeguard report -slop-history` and `codeguard report -perf-history`). ## Output diff --git a/examples/codeguard.json b/examples/codeguard.json index a806341..7299195 100644 --- a/examples/codeguard.json +++ b/examples/codeguard.json @@ -137,8 +137,12 @@ "detect_readme_drift": true, "detect_oversized_files": true, "detect_ambiguous_symbols": true, + "detect_undocumented_commands": true, + "detect_oversized_agent_docs": true, + "detect_doc_link_rot": true, "max_file_lines": 1500, - "ambiguous_symbol_threshold": 4 + "ambiguous_symbol_threshold": 4, + "max_agent_doc_lines": 600 } }, "exclude": ["vendor/**", "**/testdata/**"], diff --git a/internal/cli/report.go b/internal/cli/report.go index 1b4a9d1..77f99c0 100644 --- a/internal/cli/report.go +++ b/internal/cli/report.go @@ -17,16 +17,23 @@ func runReport(args []string, stdout io.Writer, stderr io.Writer) int { profile := fs.String("profile", "", "optional policy profile override") slopHistory := fs.Bool("slop-history", false, "print the persisted slop-score trend per target") perfHistory := fs.Bool("perf-history", false, "print the persisted performance-score trend per target") + legibilityHistory := fs.Bool("legibility-history", false, "print the persisted repo-legibility score trend per target") limit := fs.Int("limit", 0, "maximum history entries to print per target (0 = all)") if err := fs.Parse(args); err != nil { return exitError } - if !*slopHistory && !*perfHistory { - _, _ = fmt.Fprintln(stderr, "report requires a mode flag: -slop-history or -perf-history") + modes := 0 + for _, enabled := range []bool{*slopHistory, *perfHistory, *legibilityHistory} { + if enabled { + modes++ + } + } + if modes == 0 { + _, _ = fmt.Fprintln(stderr, "report requires a mode flag: -slop-history, -perf-history, or -legibility-history") return exitError } - if *slopHistory && *perfHistory { - _, _ = fmt.Fprintln(stderr, "report accepts only one mode flag: -slop-history or -perf-history") + if modes > 1 { + _, _ = fmt.Fprintln(stderr, "report accepts only one mode flag: -slop-history, -perf-history, or -legibility-history") return exitError } @@ -37,6 +44,9 @@ func runReport(args []string, stdout io.Writer, stderr io.Writer) int { if *perfHistory { return writePerfHistoryReport(stdout, cfg, *limit) } + if *legibilityHistory { + return writeLegibilityHistoryReport(stdout, cfg, *limit) + } return writeSlopHistoryReport(stdout, cfg, *limit) } diff --git a/internal/cli/report_legibility.go b/internal/cli/report_legibility.go new file mode 100644 index 0000000..17c00f9 --- /dev/null +++ b/internal/cli/report_legibility.go @@ -0,0 +1,52 @@ +package cli + +import ( + "fmt" + "io" + "sort" + "strings" + + service "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// writeLegibilityHistoryReport mirrors writeSlopHistoryReport for the +// repo_legibility trend. Legibility components are score/max slices rather +// than weighted finding counts, so the breakdown renders as label=score/max. +func writeLegibilityHistoryReport(stdout io.Writer, cfg service.Config, limit int) int { + path := service.LegibilityHistoryPath(cfg) + history := service.LoadLegibilityHistory(path) + if len(history) == 0 { + _, _ = fmt.Fprintf(stdout, "no legibility-score history recorded at %s\n", path) + return exitOK + } + keys := make([]string, 0, len(history)) + for key := range history { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + entries := history[key] + if limit > 0 && len(entries) > limit { + entries = entries[len(entries)-limit:] + } + _, _ = fmt.Fprintf(stdout, "%s\n", key) + previousScore := 0 + hasPrevious := false + for _, entry := range entries { + _, _ = fmt.Fprintf(stdout, " %s score %3d%s %s\n", + entry.Timestamp, entry.Score, formatSlopDelta(entry.Score, previousScore, hasPrevious), + formatLegibilityHistoryComponents(entry)) + previousScore = entry.Score + hasPrevious = true + } + } + return 0 +} + +func formatLegibilityHistoryComponents(entry service.LegibilityHistoryEntry) string { + parts := make([]string, 0, len(entry.Components)) + for _, component := range entry.Components { + parts = append(parts, fmt.Sprintf("%s=%d/%d", component.Label, component.Score, component.Max)) + } + return strings.Join(parts, " ") +} diff --git a/internal/codeguard/checks/agentcontext/agentcontext.go b/internal/codeguard/checks/agentcontext/agentcontext.go index c695315..66db004 100644 --- a/internal/codeguard/checks/agentcontext/agentcontext.go +++ b/internal/codeguard/checks/agentcontext/agentcontext.go @@ -27,40 +27,60 @@ func Run(ctx context.Context, env support.Context) core.SectionResult { // even when individual rules are muted. func targetFindings(env support.Context, target core.TargetConfig) []core.Finding { rules := env.Config.Checks.ContextRules - assessment, driftFound := assessTarget(env, target) + assessment, driftFound, readiness := assessTarget(env, target) findings := make([]core.Finding, 0) if ruleEnabled(rules.DetectMissingAgentDocs) && len(assessment.agentDocs) == 0 { findings = append(findings, missingAgentDocsFinding(env)) } if ruleEnabled(rules.DetectAgentDocsDrift) { - findings = append(findings, driftFound.agentDocs...) + findings = append(findings, driftFound.agentDocs.findings...) } if ruleEnabled(rules.DetectReadmeDrift) { - findings = append(findings, driftFound.readme...) + findings = append(findings, driftFound.readme.findings...) } if ruleEnabled(rules.DetectOversizedFiles) { findings = append(findings, oversizedFindings(env, assessment.inventory, assessment.maxFileLines)...) } if ruleEnabled(rules.DetectAmbiguousSymbols) { - findings = append(findings, ambiguousBasenameFindings(env, assessment.inventory, ambiguousThreshold(rules))...) + findings = append(findings, ambiguousBasenameFindings(env, assessment.ambiguousGroups)...) } + if ruleEnabled(rules.DetectUndocumentedCommands) { + findings = append(findings, readiness.undocumentedCommands...) + } + if ruleEnabled(rules.DetectOversizedAgentDocs) { + findings = append(findings, readiness.oversizedAgentDocs...) + } + if ruleEnabled(rules.DetectDocLinkRot) { + findings = append(findings, readiness.docLinkRot...) + } + artifact := legibilityArtifact(target, assessment) + findings = append(findings, legibilityThresholdFindings(env, artifact.RepoLegibility)...) if env.PutArtifact != nil { - env.PutArtifact(legibilityArtifact(target, assessment)) + recordLegibilityHistory(env, &artifact) + env.PutArtifact(artifact) } return findings } -// driftResults keeps the two drift rules' findings separate so toggles gate -// them independently while the artifact counts both. +// driftResults keeps the two drift rules' resolutions separate so toggles +// gate their findings independently while the artifact counts both. type driftResults struct { - agentDocs []core.Finding - readme []core.Finding + agentDocs docResolution + readme docResolution } -// assessTarget performs every measurement once: doc presence, drift -// resolution, and the source inventory walk shared by the size and basename -// rules and the legibility score. -func assessTarget(env support.Context, target core.TargetConfig) (targetAssessment, driftResults) { +// readinessResults carries the AI-and-human-readiness rules' findings, each +// gated by its own toggle in targetFindings. +type readinessResults struct { + undocumentedCommands []core.Finding + oversizedAgentDocs []core.Finding + docLinkRot []core.Finding +} + +// assessTarget performs every measurement once: doc presence, drift and +// readiness resolution, and the source inventory walk shared by the size and +// basename rules and the legibility score. +func assessTarget(env support.Context, target core.TargetConfig) (targetAssessment, driftResults, readinessResults) { rules := env.Config.Checks.ContextRules resolver := newRepoResolver(target.Path) assessment := targetAssessment{ @@ -68,14 +88,22 @@ func assessTarget(env support.Context, target core.TargetConfig) (targetAssessme readmePresent: resolver.pathExists("README.md"), maxFileLines: contextBudgetLines(rules), } + assessment.agentDocLines = agentDocSubstance(target.Path, assessment.agentDocs) drift := driftResults{ - agentDocs: agentDocsDriftFindings(env, resolver, assessment.agentDocs), - readme: readmeDriftFindings(env, resolver), + agentDocs: agentDocsDrift(env, resolver, assessment.agentDocs), + readme: readmeDrift(env, resolver), + } + readiness := readinessResults{ + undocumentedCommands: undocumentedCommandFindings(env, resolver, assessment.agentDocs), + oversizedAgentDocs: oversizedAgentDocFindings(env, resolver.root, assessment.agentDocs, maxAgentDocLinesBudget(rules)), + docLinkRot: docLinkRotFindings(env, resolver, assessment.agentDocs), } - assessment.driftReferences = len(drift.agentDocs) + len(drift.readme) + assessment.agentDocBrokenRefs = drift.agentDocs.broken + assessment.brokenReferences = drift.agentDocs.broken + drift.readme.broken + assessment.totalReferences = drift.agentDocs.total + drift.readme.total assessment.inventory = collectSourceInventory(env, target, assessment.maxFileLines) - assessment.ambiguousGroups = ambiguousBasenameGroups(assessment.inventory, ambiguousThreshold(rules)) - return assessment, drift + assessment.ambiguousGroups = ambiguousBasenameGroups(assessment.inventory, ambiguousThreshold(rules), ambiguousIgnoreSet(rules)) + return assessment, drift, readiness } // ruleEnabled treats a nil toggle as enabled: the family's rules are opt-out. diff --git a/internal/codeguard/checks/agentcontext/artifact.go b/internal/codeguard/checks/agentcontext/artifact.go index 313f0b6..539ea37 100644 --- a/internal/codeguard/checks/agentcontext/artifact.go +++ b/internal/codeguard/checks/agentcontext/artifact.go @@ -11,12 +11,15 @@ import ( // targetAssessment carries every measurement the legibility score aggregates // for one target, independent of which rules are toggled on. type targetAssessment struct { - agentDocs []string - readmePresent bool - driftReferences int - inventory sourceInventory - ambiguousGroups [][]string - maxFileLines int + agentDocs []string + agentDocLines int + agentDocBrokenRefs int + readmePresent bool + brokenReferences int + totalReferences int + inventory sourceInventory + ambiguousGroups [][]string + maxFileLines int } // legibilityArtifact converts a target assessment into the repo_legibility @@ -41,15 +44,42 @@ func legibilityArtifact(target core.TargetConfig, assessment targetAssessment) c ) } -// agentDocsComponent grants 25 points when any agent instruction file exists. +// agentDocSubstanceLines is the non-blank line count at which an agent +// instruction file earns full substance credit. Ten non-blank lines is +// roughly the minimum for build/test commands plus a layout note; below that +// credit scales linearly, so an empty or one-line CLAUDE.md no longer banks +// the full component. +const agentDocSubstanceLines = 10 + +// agentDocDriftPenaltyPerRef and agentDocDriftPenaltyCap deduct points from +// the agent_docs component for unresolvable references inside the agent docs +// themselves: a doc full of stale instructions is worse than a short accurate +// one. Capped at 10 so presence plus substance always retains some credit. +const ( + agentDocDriftPenaltyPerRef = 2 + agentDocDriftPenaltyCap = 10 +) + +// agentDocsComponent grants up to 25 points for agent instruction files, +// gated on substance and accuracy. Formula (spelled out in Detail): +// substance = 25 x min(non-blank lines, 10)/10 across the largest doc, minus +// 2 points per unresolvable reference in the agent docs (capped at 10), +// floored at 0. func agentDocsComponent(a targetAssessment) core.RepoLegibilityComponent { component := core.RepoLegibilityComponent{Label: "agent_docs", Max: 25} - if len(a.agentDocs) > 0 { - component.Score = component.Max - component.Detail = "found " + strings.Join(a.agentDocs, ", ") + if len(a.agentDocs) == 0 { + component.Detail = "no agent instruction files (CLAUDE.md, AGENTS.md, .cursorrules, .github/copilot-instructions.md)" return component } - component.Detail = "no agent instruction files (CLAUDE.md, AGENTS.md, .cursorrules, .github/copilot-instructions.md)" + substance := component.Max * minInt(a.agentDocLines, agentDocSubstanceLines) / agentDocSubstanceLines + driftPenalty := minInt(agentDocDriftPenaltyCap, agentDocDriftPenaltyPerRef*a.agentDocBrokenRefs) + driftPenalty = minInt(driftPenalty, substance) + component.Score = substance - driftPenalty + component.Detail = fmt.Sprintf( + "found %s; substance %d/%d (%d non-blank lines, full credit at %d), drift -%d (%d unresolvable references, -%d each capped at %d)", + strings.Join(a.agentDocs, ", "), substance, component.Max, + a.agentDocLines, agentDocSubstanceLines, + driftPenalty, a.agentDocBrokenRefs, agentDocDriftPenaltyPerRef, agentDocDriftPenaltyCap) return component } @@ -63,35 +93,50 @@ func readmeComponent(a targetAssessment) core.RepoLegibilityComponent { return component } -// docAccuracyComponent starts at 20 and loses 4 points per unresolvable doc -// reference across agent docs and the README. +// docAccuracyComponent scales 20 points by the share of doc references that +// resolve: penalty = round(20 x broken/total). The previous flat -4 per +// broken reference saturated at 5, making 5 broken references indistinguishable +// from 50; the proportional form keeps severity scaling with how much of the +// documentation is wrong, still capped at the full 20 points. func docAccuracyComponent(a targetAssessment) core.RepoLegibilityComponent { - penalty := minInt(20, 4*a.driftReferences) - return core.RepoLegibilityComponent{ - Label: "doc_accuracy", - Score: 20 - penalty, - Max: 20, - Detail: fmt.Sprintf("%d unresolvable doc references", a.driftReferences), + component := core.RepoLegibilityComponent{Label: "doc_accuracy", Max: 20} + if a.totalReferences == 0 { + component.Score = component.Max + component.Detail = "no resolvable doc references found" + return component } + penalty := (component.Max*a.brokenReferences + a.totalReferences/2) / a.totalReferences + component.Score = component.Max - penalty + component.Detail = fmt.Sprintf("%d of %d doc references unresolvable (penalty 20 x broken/total)", + a.brokenReferences, a.totalReferences) + return component } // contextEconomyComponent scales 25 points by the share of source files that -// blow the context budget; 10% oversized zeroes the component. +// blow the context budget, ramping linearly to zero at 25% oversized: +// penalty = round(25 x share x 4), capped at 25. The previous x10 multiplier +// zeroed the whole component at 10% oversized, which turned it binary for any +// mature repo carrying a tail of large files; a repo only forfeits the full +// component once one in four files exceeds the budget, while each additional +// oversized file still costs measurably. func contextEconomyComponent(a targetAssessment) core.RepoLegibilityComponent { penalty := 0 if a.inventory.files > 0 { - penalty = minInt(25, 25*len(a.inventory.oversized)*10/a.inventory.files) + penalty = minInt(25, (100*len(a.inventory.oversized)+a.inventory.files/2)/a.inventory.files) } return core.RepoLegibilityComponent{ Label: "context_economy", Score: 25 - penalty, Max: 25, - Detail: fmt.Sprintf("%d of %d source files exceed %d lines", len(a.inventory.oversized), a.inventory.files, a.maxFileLines), + Detail: fmt.Sprintf("%d of %d source files exceed %d lines (zero credit at 25%% oversized)", len(a.inventory.oversized), a.inventory.files, a.maxFileLines), } } // navigabilityComponent scales 20 points by the share of source files caught -// in ambiguous basename groups; 20% affected zeroes the component. +// in ambiguous basename groups; 20% affected zeroes the component. Groups are +// computed after removing conventional basenames (see +// defaultAmbiguousBasenameIgnore / context_rules.ambiguous_symbol_ignore), so +// language-imposed repeats like index.ts or __init__.py cost nothing. func navigabilityComponent(a targetAssessment) core.RepoLegibilityComponent { affected := 0 for _, group := range a.ambiguousGroups { diff --git a/internal/codeguard/checks/agentcontext/docs.go b/internal/codeguard/checks/agentcontext/docs.go index 6009379..2e1ccb9 100644 --- a/internal/codeguard/checks/agentcontext/docs.go +++ b/internal/codeguard/checks/agentcontext/docs.go @@ -3,6 +3,7 @@ package agentcontext import ( "fmt" "path" + "strings" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" @@ -42,45 +43,61 @@ func missingAgentDocsFinding(env support.Context) core.Finding { }) } -// agentDocsDriftFindings resolves every reference in the present agent docs -// and reports the ones that provably point at nothing. -func agentDocsDriftFindings(env support.Context, resolver *repoResolver, docs []string) []core.Finding { - findings := make([]core.Finding, 0) +// docResolution is the outcome of resolving one document set's references: +// the drift findings (capped per doc) plus the uncapped broken/total counts +// the legibility score's proportional components are built from. +type docResolution struct { + findings []core.Finding + broken int + total int +} + +// agentDocsDrift resolves every reference in the present agent docs and +// reports the ones that provably point at nothing. +func agentDocsDrift(env support.Context, resolver *repoResolver, docs []string) docResolution { + result := docResolution{findings: make([]core.Finding, 0)} for _, rel := range docs { data, ok := readCappedDocFile(resolver.root, rel) if !ok { continue } - refs := extractDocReferences(string(data), extractOptions{}) - findings = append(findings, driftFindings(env, resolver, rel, refs, "context.agent-docs-drift")...) + refs := extractDocReferences(string(data)) + docResult := driftFindings(env, resolver, rel, refs, "context.agent-docs-drift") + result.findings = append(result.findings, docResult.findings...) + result.broken += docResult.broken + result.total += docResult.total } - return findings + return result } -// readmeDriftFindings applies the same resolver to the root README, scoped to -// fenced shell blocks: the commands a README tells contributors (and agents) -// to run. -func readmeDriftFindings(env support.Context, resolver *repoResolver) []core.Finding { +// readmeDrift applies the same full extraction as agent docs to the root +// README: prose and inline-code references as well as fenced shell blocks. +// The README is the doc agents (and humans) read first, so it earns the same +// truthfulness bar. +func readmeDrift(env support.Context, resolver *repoResolver) docResolution { data, ok := readCappedDocFile(resolver.root, "README.md") if !ok { - return nil + return docResolution{} } - refs := extractDocReferences(string(data), extractOptions{commandFencesOnly: true}) + refs := extractDocReferences(string(data)) return driftFindings(env, resolver, "README.md", refs, "context.readme-drift") } // driftFindings turns the unresolvable subset of refs into findings for one -// document. -func driftFindings(env support.Context, resolver *repoResolver, docRel string, refs []docReference, ruleID string) []core.Finding { - findings := make([]core.Finding, 0) +// document. Findings are capped at maxDriftFindingsPerDoc, but the returned +// broken/total counts always cover every reference so score components stay +// proportional even for badly rotted docs. +func driftFindings(env support.Context, resolver *repoResolver, docRel string, refs []docReference, ruleID string) docResolution { + result := docResolution{findings: make([]core.Finding, 0), total: len(refs)} for _, ref := range refs { - if len(findings) >= maxDriftFindingsPerDoc { - break - } if resolvable(resolver, docRel, ref) { continue } - findings = append(findings, env.NewFinding(support.FindingInput{ + result.broken++ + if len(result.findings) >= maxDriftFindingsPerDoc { + continue + } + result.findings = append(result.findings, env.NewFinding(support.FindingInput{ RuleID: ruleID, Level: "warn", Path: docRel, @@ -89,7 +106,30 @@ func driftFindings(env support.Context, resolver *repoResolver, docRel string, r Message: driftMessage(docRel, ref), })) } - return findings + return result +} + +// agentDocSubstance returns the largest non-blank line count across the +// present agent docs: one substantial CLAUDE.md earns full substance credit +// even when a stub .cursorrules sits next to it. +func agentDocSubstance(root string, docs []string) int { + best := 0 + for _, rel := range docs { + data, ok := readCappedDocFile(root, rel) + if !ok { + continue + } + lines := 0 + for _, line := range strings.Split(string(data), "\n") { + if strings.TrimSpace(line) != "" { + lines++ + } + } + if lines > best { + best = lines + } + } + return best } // resolvable reports whether a reference resolves. Paths are tried against diff --git a/internal/codeguard/checks/agentcontext/ignore.go b/internal/codeguard/checks/agentcontext/ignore.go new file mode 100644 index 0000000..5be39e5 --- /dev/null +++ b/internal/codeguard/checks/agentcontext/ignore.go @@ -0,0 +1,49 @@ +package agentcontext + +import ( + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// defaultAmbiguousBasenameIgnore lists source-file basenames whose repetition +// is imposed by a language or framework convention rather than chosen by the +// author: agents already know that every package has an __init__.py and every +// module directory an index.ts, so repeats of these names carry no +// navigational ambiguity. The set is deliberately limited to names a +// toolchain or dominant framework prescribes: +// - JS/TS module entrypoints: index.{ts,tsx,js,jsx,mjs,cjs} +// - file-system routers (Next.js, SvelteKit, Remix-style): route.ts, +// routes.ts, page.tsx, layout.tsx +// - Python package markers: __init__.py, __main__.py +// - Rust module layout: mod.rs, lib.rs, main.rs +// - Go idioms: main.go (package main), doc.go (package docs), types.go +// (per-package type declarations) +// +// context_rules.ambiguous_symbol_ignore REPLACES this list when set. +var defaultAmbiguousBasenameIgnore = []string{ + "index.ts", "index.tsx", "index.js", "index.jsx", "index.mjs", "index.cjs", + "route.ts", "routes.ts", "page.tsx", "layout.tsx", + "__init__.py", "__main__.py", + "mod.rs", "lib.rs", "main.rs", + "main.go", "doc.go", "types.go", +} + +// ambiguousIgnoreSet resolves the effective conventional-basename ignore set. +// Replace semantics: a non-nil config list fully replaces the defaults, so +// users can both extend the set (re-list defaults plus their own names) and +// disable it entirely (set it to []). Matching is case-insensitive. +func ambiguousIgnoreSet(rules core.ContextRulesConfig) map[string]struct{} { + list := defaultAmbiguousBasenameIgnore + if rules.AmbiguousSymbolIgnore != nil { + list = rules.AmbiguousSymbolIgnore + } + set := make(map[string]struct{}, len(list)) + for _, name := range list { + name = strings.ToLower(strings.TrimSpace(name)) + if name != "" { + set[name] = struct{}{} + } + } + return set +} diff --git a/internal/codeguard/checks/agentcontext/inventory.go b/internal/codeguard/checks/agentcontext/inventory.go index f0b56bd..ff7eedf 100644 --- a/internal/codeguard/checks/agentcontext/inventory.go +++ b/internal/codeguard/checks/agentcontext/inventory.go @@ -114,9 +114,15 @@ func oversizedFindings(env support.Context, inv sourceInventory, maxLines int) [ // ambiguousBasenameGroups returns basenames shared by at least threshold // source files, with sorted locations, in deterministic basename order. -func ambiguousBasenameGroups(inv sourceInventory, threshold int) [][]string { +// Conventional basenames in the ignore set (index.ts, __init__.py, ...) are +// skipped entirely: they neither fire context.ambiguous-symbol findings nor +// count against the legibility navigability component. +func ambiguousBasenameGroups(inv sourceInventory, threshold int, ignore map[string]struct{}) [][]string { names := make([]string, 0, len(inv.basenames)) for name, locations := range inv.basenames { + if _, skip := ignore[strings.ToLower(name)]; skip { + continue + } if len(locations) >= threshold { names = append(names, name) } @@ -133,8 +139,7 @@ func ambiguousBasenameGroups(inv sourceInventory, threshold int) [][]string { // ambiguousBasenameFindings emits one finding per over-shared basename, // listing up to five locations. -func ambiguousBasenameFindings(env support.Context, inv sourceInventory, threshold int) []core.Finding { - groups := ambiguousBasenameGroups(inv, threshold) +func ambiguousBasenameFindings(env support.Context, groups [][]string) []core.Finding { findings := make([]core.Finding, 0, len(groups)) for _, locations := range groups { shown := locations diff --git a/internal/codeguard/checks/agentcontext/legibility_history.go b/internal/codeguard/checks/agentcontext/legibility_history.go new file mode 100644 index 0000000..0ade5e2 --- /dev/null +++ b/internal/codeguard/checks/agentcontext/legibility_history.go @@ -0,0 +1,51 @@ +package agentcontext + +import ( + "time" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +// recordLegibilityHistory persists the artifact's score to the per-scan trend +// file next to the cache (.legibility-history.) and annotates the +// artifact with the previous score and delta when prior scans exist, +// mirroring recordSlopHistory in checks/quality and +// recordPerformanceScoreHistory in checks/performance. +func recordLegibilityHistory(env support.Context, artifact *core.Artifact) { + if artifact == nil || artifact.RepoLegibility == nil { + return + } + rules := env.Config.Checks.ContextRules + if rules.LegibilityHistory != nil && !*rules.LegibilityHistory { + return + } + if env.Config.Cache.Enabled != nil && !*env.Config.Cache.Enabled { + return + } + path := runnersupport.LegibilityHistoryPathForBase(env.Config.Cache.Path) + if path == "" { + return + } + entry := core.LegibilityHistoryEntry{ + Timestamp: legibilityScanTimestamp(env), + Score: artifact.RepoLegibility.Score, + Components: append([]core.RepoLegibilityComponent(nil), artifact.RepoLegibility.Components...), + } + previous, hasPrevious := runnersupport.AppendLegibilityHistory(path, artifact.ID, entry, rules.LegibilityHistoryLimit) + if !hasPrevious { + return + } + previousScore := previous.Score + delta := artifact.RepoLegibility.Score - previousScore + artifact.RepoLegibility.PreviousScore = &previousScore + artifact.RepoLegibility.Delta = &delta +} + +func legibilityScanTimestamp(env support.Context) string { + if !env.ScanTime.IsZero() { + return env.ScanTime.UTC().Format(time.RFC3339) + } + return time.Now().UTC().Format(time.RFC3339) +} diff --git a/internal/codeguard/checks/agentcontext/linkrot.go b/internal/codeguard/checks/agentcontext/linkrot.go new file mode 100644 index 0000000..c148da3 --- /dev/null +++ b/internal/codeguard/checks/agentcontext/linkrot.go @@ -0,0 +1,122 @@ +package agentcontext + +import ( + "fmt" + "path" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// linkSchemePattern recognizes URI schemes (https:, mailto:, vscode:) before +// any path separator; such targets are external and never checked — this rule +// does no network I/O. +var linkSchemePattern = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9+.-]*:`) + +// markdownLink is one link target found outside code fences, with the line it +// appears on. +type markdownLink struct { + target string + line int +} + +// docLinkRotFindings checks markdown link targets in the agent docs and the +// root README against the repository, reporting links that resolve to +// nothing. Findings per doc share the drift rules' cap. +func docLinkRotFindings(env support.Context, resolver *repoResolver, agentDocs []string) []core.Finding { + findings := make([]core.Finding, 0) + for _, rel := range append(append([]string{}, agentDocs...), "README.md") { + data, ok := readCappedDocFile(resolver.root, rel) + if !ok { + continue + } + findings = append(findings, linkRotFindingsForDoc(env, resolver, rel, string(data))...) + } + return findings +} + +func linkRotFindingsForDoc(env support.Context, resolver *repoResolver, docRel string, content string) []core.Finding { + findings := make([]core.Finding, 0) + for _, link := range extractMarkdownLinks(content) { + if len(findings) >= maxDriftFindingsPerDoc { + break + } + target, ok := linkPathToCheck(link.target) + if !ok || linkResolves(resolver, docRel, target) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "context.doc-link-rot", + Level: "warn", + Path: docRel, + Line: link.line, + Column: 1, + Message: fmt.Sprintf("%s links to %q, which does not resolve to a file or directory in the repository; "+ + "broken links send agents and readers to dead ends — fix the link target or remove the link", docRel, link.target), + })) + } + return findings +} + +// extractMarkdownLinks collects markdown link targets outside code fences, +// deduplicated per document. Fenced blocks are code samples, where link +// syntax is content, not navigation. +func extractMarkdownLinks(content string) []markdownLink { + var links []markdownLink + seen := map[string]struct{}{} + fence := fenceState{} + for idx, line := range strings.Split(content, "\n") { + if fence.observe(line) || fence.inFence() { + continue + } + for _, match := range markdownLinkPattern.FindAllStringSubmatch(line, -1) { + target := match[1] + if _, dup := seen[target]; dup { + continue + } + seen[target] = struct{}{} + links = append(links, markdownLink{target: target, line: idx + 1}) + } + } + return links +} + +// linkPathToCheck reduces a raw link target to the repository path this rule +// is willing to judge, preserving precision over recall: +// - external URLs (any scheme) and protocol-relative //host links: skipped +// - pure #anchor links: skipped; path.md#anchor checks only the path part +// - editor-style :line suffixes are stripped before resolution +// - .. traversals, query strings, and templated or placeholder targets +// (, $VAR, %20, globs): skipped — they cannot be proven broken +func linkPathToCheck(target string) (string, bool) { + if idx := strings.IndexByte(target, '#'); idx >= 0 { + target = target[:idx] + } + target = lineSuffixPattern.ReplaceAllString(target, "") + if target == "" || strings.HasPrefix(target, "//") || linkSchemePattern.MatchString(target) { + return "", false + } + if strings.Contains(target, "..") || strings.Contains(target, "?") || hasUnresolvableRunes(target) { + return "", false + } + return target, true +} + +// linkResolves reports whether a checkable link target exists. Relative +// targets resolve against the doc's own directory (markdown semantics) and +// the repo root (the common shorthand); absolute /path targets resolve +// against the repo root only, the convention hosted viewers apply — an +// absolute filesystem path baked into a doc resolves nowhere and is exactly +// the rot this rule exists to catch. +func linkResolves(resolver *repoResolver, docRel string, target string) bool { + if strings.HasPrefix(target, "/") { + return resolver.pathExists(strings.TrimPrefix(target, "/")) + } + if resolver.pathExists(target) { + return true + } + docDir := path.Dir(docRel) + return docDir != "." && resolver.pathExists(path.Join(docDir, target)) +} diff --git a/internal/codeguard/checks/agentcontext/oversized_docs.go b/internal/codeguard/checks/agentcontext/oversized_docs.go new file mode 100644 index 0000000..22958a9 --- /dev/null +++ b/internal/codeguard/checks/agentcontext/oversized_docs.go @@ -0,0 +1,49 @@ +package agentcontext + +import ( + "fmt" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// defaultMaxAgentDocLines is the documented default budget for a single agent +// instruction file. Agent docs are loaded into every session verbatim, so a +// doc that sprawls consumes the very context window it exists to conserve. +const defaultMaxAgentDocLines = 600 + +// maxAgentDocLinesBudget resolves the configured agent-doc budget, falling +// back to the documented default for configs assembled without ApplyDefaults. +func maxAgentDocLinesBudget(rules core.ContextRulesConfig) int { + if rules.MaxAgentDocLines > 0 { + return rules.MaxAgentDocLines + } + return defaultMaxAgentDocLines +} + +// oversizedAgentDocFindings reports agent instruction files whose line count +// exceeds the agent-doc budget. Only the recognized agent docs are measured; +// the README and linked reference material are free to be long. +func oversizedAgentDocFindings(env support.Context, root string, agentDocs []string, maxLines int) []core.Finding { + findings := make([]core.Finding, 0) + for _, rel := range agentDocs { + data, ok := readCappedDocFile(root, rel) + if !ok { + continue + } + lines := env.CountLines(data) + if lines <= maxLines { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "context.oversized-agent-doc", + Level: "warn", + Path: rel, + Line: 1, + Column: 1, + Message: fmt.Sprintf("agent instruction file has %d lines, exceeding the %d-line agent doc budget; "+ + "a doc this large consumes the context window it exists to save — keep the instruction file to essentials and move reference material into linked docs", lines, maxLines), + })) + } + return findings +} diff --git a/internal/codeguard/checks/agentcontext/references.go b/internal/codeguard/checks/agentcontext/references.go index d050f86..b9157c5 100644 --- a/internal/codeguard/checks/agentcontext/references.go +++ b/internal/codeguard/checks/agentcontext/references.go @@ -22,13 +22,6 @@ type docReference struct { display string } -// extractOptions controls how much of a document is mined for references. -// Command fences (bash/sh/shell) are always parsed; commandFencesOnly limits -// extraction to them, which is the README rule's scope. -type extractOptions struct { - commandFencesOnly bool -} - var ( inlineCodePattern = regexp.MustCompile("`([^`\n]+)`") markdownLinkPattern = regexp.MustCompile(`\]\(([^)\s]+)\)`) @@ -39,7 +32,10 @@ var ( // references that can be positively resolved against the repository. Fenced // blocks other than shell command fences are skipped entirely: code samples, // JSON, and captured output are full of tokens that merely look like paths. -func extractDocReferences(content string, opts extractOptions) []docReference { +// Markdown link targets are not extracted here; they belong to the +// doc-link-rot rule (see linkrot.go), which keeps the two rules from +// reporting the same broken reference twice. +func extractDocReferences(content string) []docReference { var refs []docReference seen := map[string]struct{}{} fence := fenceState{} @@ -51,7 +47,7 @@ func extractDocReferences(content string, opts extractOptions) []docReference { switch { case fence.inCommandFence(): refs = appendRefs(refs, seen, lineNo, commandLineReferences(line, &fence)) - case fence.inFence() || opts.commandFencesOnly: + case fence.inFence(): continue default: refs = appendRefs(refs, seen, lineNo, proseLineReferences(line)) @@ -74,8 +70,9 @@ func appendRefs(refs []docReference, seen map[string]struct{}, lineNo int, found } // proseLineReferences extracts references from a line outside any fence: -// inline code spans (commands or paths), markdown link targets, and bare -// prose tokens that obviously denote files. +// inline code spans (commands or paths) and bare prose tokens that obviously +// denote files. Markdown link targets are stripped, not extracted — the +// doc-link-rot rule owns them. func proseLineReferences(line string) []docReference { var refs []docReference stripped := line @@ -90,11 +87,6 @@ func proseLineReferences(line string) []docReference { refs = append(refs, docReference{kind: refPath, value: value, display: span}) } } - for _, match := range markdownLinkPattern.FindAllStringSubmatch(stripped, -1) { - if value, ok := pathToken(strings.TrimPrefix(match[1], "#"), false); ok { - refs = append(refs, docReference{kind: refPath, value: value, display: match[1]}) - } - } for _, token := range strings.Fields(markdownLinkPattern.ReplaceAllString(stripped, " ")) { if value, ok := pathToken(token, true); ok { refs = append(refs, docReference{kind: refPath, value: value, display: value}) diff --git a/internal/codeguard/checks/agentcontext/threshold.go b/internal/codeguard/checks/agentcontext/threshold.go new file mode 100644 index 0000000..cc71c06 --- /dev/null +++ b/internal/codeguard/checks/agentcontext/threshold.go @@ -0,0 +1,51 @@ +package agentcontext + +import ( + "fmt" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// legibilityThresholdFindings turns the repo_legibility score into an +// enforceable gate. Legibility is good-high (the inverse of the slop score), +// so the finding fires when the score falls BELOW a configured threshold: +// context_rules.legibility_fail_threshold fails the scan, +// legibility_warn_threshold warns, and 0 (the default) disables each +// threshold. The fail threshold takes precedence when both match. +func legibilityThresholdFindings(env support.Context, legibility *core.RepoLegibilityArtifact) []core.Finding { + if legibility == nil { + return nil + } + rules := env.Config.Checks.ContextRules + var level string + var threshold int + switch { + case rules.LegibilityFailThreshold > 0 && legibility.Score < rules.LegibilityFailThreshold: + level, threshold = "fail", rules.LegibilityFailThreshold + case rules.LegibilityWarnThreshold > 0 && legibility.Score < rules.LegibilityWarnThreshold: + level, threshold = "warn", rules.LegibilityWarnThreshold + default: + return nil + } + return []core.Finding{env.NewFinding(support.FindingInput{ + RuleID: "context.legibility-threshold", + Level: level, + Message: fmt.Sprintf( + "repository legibility score %d fell below the configured %s threshold %d (%s); "+ + "improve the weakest components or adjust context_rules.legibility_%s_threshold", + legibility.Score, level, threshold, + formatLegibilityComponents(legibility.Components), level), + })} +} + +// formatLegibilityComponents renders the component breakdown for the +// threshold finding message, e.g. "agent_docs 10/25, readme 10/10, ...". +func formatLegibilityComponents(components []core.RepoLegibilityComponent) string { + parts := make([]string, 0, len(components)) + for _, component := range components { + parts = append(parts, fmt.Sprintf("%s %d/%d", component.Label, component.Score, component.Max)) + } + return strings.Join(parts, ", ") +} diff --git a/internal/codeguard/checks/agentcontext/undocumented.go b/internal/codeguard/checks/agentcontext/undocumented.go new file mode 100644 index 0000000..a793c49 --- /dev/null +++ b/internal/codeguard/checks/agentcontext/undocumented.go @@ -0,0 +1,152 @@ +package agentcontext + +import ( + "fmt" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// highSignalCommandNames is the allowlist of canonical dev-workflow command +// names the undocumented-commands rule cares about. Obscure internal targets +// are deliberately outside the list: not every Makefile target deserves a +// place in the agent docs, but these names are the entrypoints an agent (or a +// new contributor) must be able to discover. Matching is exact — build-ci or +// fmt-check are not high-signal just because they share a prefix. +var highSignalCommandNames = []string{ + "build", "check", "dev", "fmt", "lint", "run", "start", "test", +} + +// maxUndocumentedCommandFindings caps this rule's output so a large canonical +// surface reports a representative sample instead of a wall. +const maxUndocumentedCommandFindings = 10 + +// docContent pairs a doc's repo-relative path with its raw text so mention +// checks can run over every doc that counts as documentation. +type docContent struct { + rel string + text string +} + +// undocumentedCommandFindings reports high-signal Makefile targets and +// package.json scripts that no agent doc and not even the root README +// mentions — the inverse of drift: the command exists, the docs are silent. +// When the repo has no agent docs at all the rule stays silent; +// context.agent-docs-missing already covers that hole. +func undocumentedCommandFindings(env support.Context, resolver *repoResolver, agentDocs []string) []core.Finding { + if len(agentDocs) == 0 { + return nil + } + docs := loadDocContents(resolver.root, append(append([]string{}, agentDocs...), "README.md")) + documented := documentedCommandSet(docs) + findings := make([]core.Finding, 0) + for _, name := range highSignalCommandNames { + if len(findings) >= maxUndocumentedCommandFindings { + return findings + } + if canonicalMakeTarget(resolver, name) && !commandDocumented(docs, documented, refMake, name) { + findings = append(findings, undocumentedCommandFinding(env, "Makefile", "make "+name)) + } + } + for _, name := range highSignalCommandNames { + if len(findings) >= maxUndocumentedCommandFindings { + return findings + } + if canonicalNpmScript(resolver, name) && !commandDocumented(docs, documented, refNpmScript, name) { + findings = append(findings, undocumentedCommandFinding(env, "package.json", "npm run "+name)) + } + } + return findings +} + +// canonicalMakeTarget reports whether name is a target the root Makefile +// provably defines. Unreliable Makefiles (includes, pattern rules) yield no +// canonical set — the same conservative stance the drift rules take. +func canonicalMakeTarget(resolver *repoResolver, name string) bool { + if !resolver.makeReliable { + return false + } + _, ok := resolver.makeTargets[name] + return ok +} + +// canonicalNpmScript reports whether name is a script the root package.json +// provably defines; workspace roots yield no canonical set. +func canonicalNpmScript(resolver *repoResolver, name string) bool { + if !resolver.npmReliable { + return false + } + _, ok := resolver.npmScripts[name] + return ok +} + +// loadDocContents reads each existing doc once for the mention checks. +func loadDocContents(root string, rels []string) []docContent { + docs := make([]docContent, 0, len(rels)) + for _, rel := range rels { + if data, ok := readCappedDocFile(root, rel); ok { + docs = append(docs, docContent{rel: rel, text: string(data)}) + } + } + return docs +} + +// documentedCommandSet collects every make target and npm script the docs +// reference through the structured extractor (inline code and shell fences). +func documentedCommandSet(docs []docContent) map[string]struct{} { + documented := map[string]struct{}{} + for _, doc := range docs { + for _, ref := range extractDocReferences(doc.text) { + if ref.kind == refMake || ref.kind == refNpmScript { + documented[string(ref.kind)+"|"+ref.value] = struct{}{} + } + } + } + return documented +} + +// commandDocumented reports whether any doc mentions the command. The stance +// is the mirror image of drift's precision rule: a finding here claims the +// docs are silent, so ANY plausible mention counts — a structured reference +// from the extractor, or a plain-text invocation anywhere in the doc +// (prose without backticks, non-shell fences, captured output). +func commandDocumented(docs []docContent, documented map[string]struct{}, kind refKind, name string) bool { + if _, ok := documented[string(kind)+"|"+name]; ok { + return true + } + mention := commandMentionPattern(kind, name) + for _, doc := range docs { + if mention.MatchString(doc.text) { + return true + } + } + return false +} + +// commandMentionPattern matches a literal invocation of the command with the +// same word-boundary alphabet isPlainCommandWord accepts, so `make fmt-check` +// never counts as a mention of `make fmt`. +func commandMentionPattern(kind refKind, name string) *regexp.Regexp { + const boundary = `[^A-Za-z0-9._/:-]` + quoted := regexp.QuoteMeta(name) + switch kind { + case refNpmScript: + return regexp.MustCompile(`(?m)(?:^|` + boundary + `)(?:npm|pnpm|yarn)[ \t]+(?:run(?:-script)?[ \t]+)?` + quoted + `(?:$|` + boundary + `)`) + default: + return regexp.MustCompile(`(?m)(?:^|` + boundary + `)make[ \t]+` + quoted + `(?:$|` + boundary + `)`) + } +} + +func undocumentedCommandFinding(env support.Context, path string, invocation string) core.Finding { + return env.NewFinding(support.FindingInput{ + RuleID: "context.undocumented-commands", + Level: "warn", + Path: path, + Line: 1, + Column: 1, + Message: fmt.Sprintf("canonical dev command %q is not mentioned in any agent instruction file or the root README; "+ + "agents cannot discover entrypoints the docs never name — document it in CLAUDE.md/AGENTS.md or the README", strings.TrimSpace(invocation)), + }) +} diff --git a/internal/codeguard/checks/quality/quality_ai_corpus.go b/internal/codeguard/checks/quality/quality_ai_corpus.go new file mode 100644 index 0000000..c433351 --- /dev/null +++ b/internal/codeguard/checks/quality/quality_ai_corpus.go @@ -0,0 +1,64 @@ +package quality + +import ( + "os" + "path/filepath" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +// listAITargetFiles returns the target files matching include, sharing the +// per-scan corpus walk when the runner wired the hook and falling back to a +// direct walk for unit-test contexts. Both paths apply the same configured +// excludes and return nil when the walk fails, matching the historical +// WalkFiles-based behavior. +func listAITargetFiles(env support.Context, target core.TargetConfig, include func(string) bool) []string { + if env.ListTargetFiles != nil { + all, err := env.ListTargetFiles(target) + if err != nil { + return nil + } + files := make([]string, 0, len(all)) + for _, rel := range all { + if include(rel) { + files = append(files, rel) + } + } + return files + } + files, err := runnersupport.WalkFiles(target.Path, env.Config.Exclude, include) + if err != nil { + return nil + } + return files +} + +// readAITargetFile reads a walk-enumerated file under the target root through +// the shared per-scan corpus when the runner wired the hook, so the AI checks +// no longer re-read the full source corpus that other sections already loaded. +// Files reaching this path came from the corpus walk, which already enforces +// the scan size cap, so the capped corpus read behaves identically to the +// direct os.ReadFile it replaces. +func readAITargetFile(env support.Context, target core.TargetConfig, rel string) ([]byte, error) { + if env.ReadTargetFile != nil { + return env.ReadTargetFile(target, rel) + } + return os.ReadFile(filepath.Join(target.Path, rel)) //nolint:gosec // path resolved under the scan-target root +} + +// aiTargetSourceFiles lists the target files whose lowercased path ends with +// one of the given suffixes, honoring configured excludes. +func aiTargetSourceFiles(env support.Context, target core.TargetConfig, suffixes ...string) []string { + return listAITargetFiles(env, target, func(rel string) bool { + lower := strings.ToLower(rel) + for _, suffix := range suffixes { + if strings.HasSuffix(lower, suffix) { + return true + } + } + return false + }) +} diff --git a/internal/codeguard/checks/quality/quality_ai_dead_code.go b/internal/codeguard/checks/quality/quality_ai_dead_code.go index f98a71c..d9a9028 100644 --- a/internal/codeguard/checks/quality/quality_ai_dead_code.go +++ b/internal/codeguard/checks/quality/quality_ai_dead_code.go @@ -115,10 +115,13 @@ func goFuncEligibleForUnusedCheck(rel string, fn *ast.FuncDecl) bool { if strings.HasSuffix(rel, "_test.go") && hasGoTestEntrypointPrefix(name) { return false } - // Compiler directives such as go:linkname or cgo exports imply external use. - if fn.Doc != nil && strings.Contains(fn.Doc.Text(), "go:") { - return false - } + // NOTE: an earlier revision tried to exempt functions whose doc comment + // carries a compiler directive (go:linkname, cgo exports), but the AST it + // received was parsed without ParseComments, so fn.Doc was always nil and + // the exemption never fired. The parse now comes from the shared + // ParseComments corpus cache, which would have silently activated it and + // changed findings; the check stays out to keep the detector's output + // identical. Activating it is a candidate behavior fix, not an optimization. return true } diff --git a/internal/codeguard/checks/quality/quality_ai_error_style_python.go b/internal/codeguard/checks/quality/quality_ai_error_style_python.go index c3d7f1a..87ba81d 100644 --- a/internal/codeguard/checks/quality/quality_ai_error_style_python.go +++ b/internal/codeguard/checks/quality/quality_ai_error_style_python.go @@ -1,8 +1,6 @@ package quality import ( - "os" - "path/filepath" "regexp" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" @@ -26,10 +24,10 @@ func pythonErrorStyleCounts(source string) pythonErrorStyleSummary { } } -func pythonRepoErrorStyle(root string, files []string) pythonErrorStyleSummary { +func pythonRepoErrorStyle(env support.Context, target core.TargetConfig, files []string) pythonErrorStyleSummary { total := pythonErrorStyleSummary{} for _, rel := range files { - data, err := os.ReadFile(filepath.Join(root, rel)) //nolint:gosec // file under the scan-target root + data, err := readAITargetFile(env, target, rel) if err != nil { continue } diff --git a/internal/codeguard/checks/quality/quality_ai_naming_drift.go b/internal/codeguard/checks/quality/quality_ai_naming_drift.go index 8b2d703..eeeeda8 100644 --- a/internal/codeguard/checks/quality/quality_ai_naming_drift.go +++ b/internal/codeguard/checks/quality/quality_ai_naming_drift.go @@ -2,8 +2,6 @@ package quality import ( "fmt" - "os" - "path/filepath" "regexp" "strings" @@ -62,10 +60,10 @@ func namingCounts(source string, extract nameExtractor) map[string]int { // dominantNamingConvention establishes the repository-dominant identifier // convention for the language, requiring a minimum amount of signal. -func dominantNamingConvention(root string, files []string, extract nameExtractor) string { +func dominantNamingConvention(env support.Context, target core.TargetConfig, files []string, extract nameExtractor) string { totals := map[string]int{} for _, rel := range files { - data, err := os.ReadFile(filepath.Join(root, rel)) //nolint:gosec // file under the scan-target root + data, err := readAITargetFile(env, target, rel) if err != nil { continue } @@ -73,11 +71,7 @@ func dominantNamingConvention(root string, files []string, extract nameExtractor totals[convention] += count } } - dominant := dominantFrameworkFromCounts(totals) - if totals[dominant] < 3 { - return "" - } - return dominant + return dominantStyleFromTotals(totals) } func namingDriftFinding(env support.Context, file string, source string, dominant string, extract nameExtractor) []core.Finding { diff --git a/internal/codeguard/checks/quality/quality_ai_resolution.go b/internal/codeguard/checks/quality/quality_ai_resolution.go index c6ac34c..40d5ecd 100644 --- a/internal/codeguard/checks/quality/quality_ai_resolution.go +++ b/internal/codeguard/checks/quality/quality_ai_resolution.go @@ -12,27 +12,8 @@ import ( "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" - runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" ) -// aiTargetSourceFiles walks a target and returns the files whose lowercased -// path ends with one of the given suffixes, honoring configured excludes. -func aiTargetSourceFiles(env support.Context, target core.TargetConfig, suffixes ...string) []string { - files, err := runnersupport.WalkFiles(target.Path, env.Config.Exclude, func(rel string) bool { - lower := strings.ToLower(rel) - for _, suffix := range suffixes { - if strings.HasSuffix(lower, suffix) { - return true - } - } - return false - }) - if err != nil { - return nil - } - return files -} - type packageManifest struct { Name string `json:"name"` Dependencies map[string]string `json:"dependencies"` @@ -40,11 +21,18 @@ type packageManifest struct { PeerDependencies map[string]string `json:"peerDependencies"` } +// readPackageManifest reads the root package.json directly rather than through +// the corpus: the fixed filename is not walk-enumerated, so a direct uncapped +// read preserves the historical behavior. func readPackageManifest(root string) (packageManifest, bool) { data, err := os.ReadFile(filepath.Join(root, "package.json")) //nolint:gosec // fixed filename under the scan-target root if err != nil { return packageManifest{}, false } + return parsePackageManifest(data) +} + +func parsePackageManifest(data []byte) (packageManifest, bool) { var manifest packageManifest if err := json.Unmarshal(data, &manifest); err != nil { return packageManifest{}, false @@ -69,16 +57,17 @@ func packageManifestDeps(manifest packageManifest) map[string]struct{} { return deps } -func readWorkspacePackageNames(root string, excludes []string) map[string]struct{} { - files, err := runnersupport.WalkFiles(root, excludes, func(rel string) bool { +func readWorkspacePackageNames(env support.Context, target core.TargetConfig) map[string]struct{} { + files := listAITargetFiles(env, target, func(rel string) bool { return filepath.Base(rel) == "package.json" }) - if err != nil { - return map[string]struct{}{} - } names := map[string]struct{}{} for _, rel := range files { - manifest, ok := readPackageManifest(filepath.Join(root, filepath.Dir(rel))) + data, err := readAITargetFile(env, target, rel) + if err != nil { + continue + } + manifest, ok := parsePackageManifest(data) if !ok || strings.TrimSpace(manifest.Name) == "" { continue } diff --git a/internal/codeguard/checks/quality/quality_ai_style_drift.go b/internal/codeguard/checks/quality/quality_ai_style_drift.go index 7b0a06c..a4addbb 100644 --- a/internal/codeguard/checks/quality/quality_ai_style_drift.go +++ b/internal/codeguard/checks/quality/quality_ai_style_drift.go @@ -2,8 +2,6 @@ package quality import ( "fmt" - "os" - "path/filepath" "regexp" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" @@ -46,10 +44,6 @@ func goErrorStyleCounts(source string) map[string]int { return counts } -func dominantGoErrorStyle(root string, files []string) string { - return dominantStyle(root, files, goErrorStyleCounts) -} - // goErrorStyleDriftFinding is direction-aware: a file whose dominant style is // %w wrapping is never reported, because wrapping preserves the error chain // and adopting it in an unwrapped-error repository is an improvement, not @@ -76,8 +70,8 @@ func scriptErrorStyleCounts(source string) map[string]int { return counts } -func dominantScriptErrorStyle(root string, files []string) string { - return dominantStyle(root, files, scriptErrorStyleCounts) +func dominantScriptErrorStyle(env support.Context, target core.TargetConfig, files []string) string { + return dominantStyle(env, target, files, scriptErrorStyleCounts) } func scriptErrorStyleDriftFinding(env support.Context, file string, source string, dominant string) []core.Finding { @@ -86,10 +80,10 @@ func scriptErrorStyleDriftFinding(env support.Context, file string, source strin // --- shared style machinery --- -func dominantStyle(root string, files []string, counter func(string) map[string]int) string { +func dominantStyle(env support.Context, target core.TargetConfig, files []string, counter func(string) map[string]int) string { totals := map[string]int{} for _, rel := range files { - data, err := os.ReadFile(filepath.Join(root, rel)) //nolint:gosec // file under the scan-target root + data, err := readAITargetFile(env, target, rel) if err != nil { continue } @@ -97,6 +91,12 @@ func dominantStyle(root string, files []string, counter func(string) map[string] totals[style] += count } } + return dominantStyleFromTotals(totals) +} + +// dominantStyleFromTotals picks the highest-count style, requiring a minimum +// amount of repository-wide signal before declaring a dominant style. +func dominantStyleFromTotals(totals map[string]int) string { dominant := dominantFrameworkFromCounts(totals) if totals[dominant] < 3 { return "" diff --git a/internal/codeguard/checks/quality/quality_ai_target_go.go b/internal/codeguard/checks/quality/quality_ai_target_go.go index e8a3f1d..0410289 100644 --- a/internal/codeguard/checks/quality/quality_ai_target_go.go +++ b/internal/codeguard/checks/quality/quality_ai_target_go.go @@ -3,7 +3,6 @@ package quality import ( "fmt" "go/ast" - "go/parser" "go/token" "os" "path/filepath" @@ -24,17 +23,15 @@ func goAITargetFindings(env support.Context, target core.TargetConfig) []core.Fi return nil } metadata := readGoModuleMetadata(target.Path) - dominant := dominantGoTestFramework(target.Path, files) - errorStyle := dominantGoErrorStyle(target.Path, files) - naming := dominantNamingConvention(target.Path, files, goDeclaredNames) + profile := goRepoStyleProfile(env, target, files) packageFiles := map[string][]goParsedFile{} findings := make([]core.Finding, 0) for _, rel := range files { - fileFindings, parsedFile := goFileAIQualityFindings(env, target.Path, rel, goFileScanInput{ + fileFindings, parsedFile := goFileAIQualityFindings(env, target, rel, goFileScanInput{ metadata: metadata, - dominant: dominant, - errorStyle: errorStyle, - naming: naming, + dominant: profile.testFramework, + errorStyle: profile.errorStyle, + naming: profile.naming, }) findings = append(findings, fileFindings...) if parsedFile != nil { @@ -57,9 +54,46 @@ type goFileScanInput struct { naming string } -func goFileAIQualityFindings(env support.Context, root string, rel string, input goFileScanInput) ([]core.Finding, *goParsedFile) { - abs := filepath.Join(root, rel) - data, err := os.ReadFile(abs) //nolint:gosec // path resolved under the scan-target root +type goRepoStyle struct { + testFramework string + errorStyle string + naming string +} + +// goRepoStyleProfile computes the three repository-dominant style signals +// (test framework, error style, naming convention) in a single pass over the +// corpus instead of one read of every file per signal. Each signal is an +// independent per-file sum, so folding them into one loop is +// behavior-identical to the previous three passes. +func goRepoStyleProfile(env support.Context, target core.TargetConfig, files []string) goRepoStyle { + frameworkCounts := map[string]int{} + styleTotals := map[string]int{} + namingTotals := map[string]int{} + for _, rel := range files { + data, err := readAITargetFile(env, target, rel) + if err != nil { + continue + } + source := string(data) + if framework := goTestFramework(source); framework != "" { + frameworkCounts[framework]++ + } + for style, count := range goErrorStyleCounts(source) { + styleTotals[style] += count + } + for convention, count := range namingCounts(source, goDeclaredNames) { + namingTotals[convention] += count + } + } + return goRepoStyle{ + testFramework: dominantFrameworkFromCounts(frameworkCounts), + errorStyle: dominantStyleFromTotals(styleTotals), + naming: dominantStyleFromTotals(namingTotals), + } +} + +func goFileAIQualityFindings(env support.Context, target core.TargetConfig, rel string, input goFileScanInput) ([]core.Finding, *goParsedFile) { + data, err := readAITargetFile(env, target, rel) if err != nil { return nil, nil } @@ -67,8 +101,7 @@ func goFileAIQualityFindings(env support.Context, root string, rel string, input checks := env.Config.Checks.QualityRules.AIChecks findings := make([]core.Finding, 0) var parsedFile *goParsedFile - fset := token.NewFileSet() - if parsed, err := parser.ParseFile(fset, abs, data, 0); err == nil { + if fset, parsed, err := support.ParseGoSource(env, rel, data); err == nil { parsedFile = &goParsedFile{rel: rel, fset: fset, parsed: parsed} if aiCheckEnabled(checks.HallucinatedImport) { findings = append(findings, goHallucinatedImportFindings(env, rel, fset, parsed, input.metadata)...) diff --git a/internal/codeguard/checks/quality/quality_ai_target_python.go b/internal/codeguard/checks/quality/quality_ai_target_python.go index ca12e6c..8bdb1dd 100644 --- a/internal/codeguard/checks/quality/quality_ai_target_python.go +++ b/internal/codeguard/checks/quality/quality_ai_target_python.go @@ -24,11 +24,11 @@ func pythonAITargetFindings(env support.Context, target core.TargetConfig) []cor } catalog := readPythonDependencyCatalog(target.Path) localModules := pythonLocalModuleNames(target.Path, files) - repoErrorStyle := pythonRepoErrorStyle(target.Path, files) - repoNaming := dominantNamingConvention(target.Path, files, pythonDeclaredNames) + repoErrorStyle := pythonRepoErrorStyle(env, target, files) + repoNaming := dominantNamingConvention(env, target, files, pythonDeclaredNames) findings := make([]core.Finding, 0) for _, rel := range files { - findings = append(findings, pythonFileAIQualityFindings(env, target.Path, rel, pythonFileScanInput{ + findings = append(findings, pythonFileAIQualityFindings(env, target, rel, pythonFileScanInput{ catalog: catalog, localModules: localModules, errorStyle: repoErrorStyle, @@ -45,15 +45,15 @@ type pythonFileScanInput struct { naming string } -func pythonFileAIQualityFindings(env support.Context, root string, rel string, input pythonFileScanInput) []core.Finding { - data, err := os.ReadFile(filepath.Join(root, rel)) //nolint:gosec // file under the scan-target root +func pythonFileAIQualityFindings(env support.Context, target core.TargetConfig, rel string, input pythonFileScanInput) []core.Finding { + data, err := readAITargetFile(env, target, rel) if err != nil { return nil } source := strings.ReplaceAll(string(data), "\r\n", "\n") findings := make([]core.Finding, 0) if aiCheckEnabled(env.Config.Checks.QualityRules.AIChecks.HallucinatedImport) { - findings = append(findings, pythonImportFindings(env, root, rel, source, input)...) + findings = append(findings, pythonImportFindings(env, target.Path, rel, source, input)...) } if aiCheckEnabled(env.Config.Checks.QualityRules.AIChecks.DeadCode) { findings = append(findings, pythonDeadCodeFindings(env, rel, source)...) diff --git a/internal/codeguard/checks/quality/quality_ai_target_script.go b/internal/codeguard/checks/quality/quality_ai_target_script.go index cc8f019..550f5dd 100644 --- a/internal/codeguard/checks/quality/quality_ai_target_script.go +++ b/internal/codeguard/checks/quality/quality_ai_target_script.go @@ -31,18 +31,18 @@ func typeScriptAITargetFindings(env support.Context, target core.TargetConfig) [ catalog := scriptImportCatalog{ hasManifest: hasManifest, deps: packageManifestDeps(manifest), - workspacePackage: readWorkspacePackageNames(target.Path, env.Config.Exclude), + workspacePackage: readWorkspacePackageNames(env, target), } - dominant := dominantScriptTestFramework(target.Path, files, manifest) + dominant := dominantScriptTestFramework(env, target, files, manifest) input := scriptFileScanInput{ catalog: catalog, dominant: dominant, - errorStyle: dominantScriptErrorStyle(target.Path, files), - naming: dominantNamingConvention(target.Path, files, scriptDeclaredNames), + errorStyle: dominantScriptErrorStyle(env, target, files), + naming: dominantNamingConvention(env, target, files, scriptDeclaredNames), } findings := make([]core.Finding, 0) for _, rel := range files { - findings = append(findings, scriptFileAIQualityFindings(env, target.Path, rel, input)...) + findings = append(findings, scriptFileAIQualityFindings(env, target, rel, input)...) } return findings } @@ -54,9 +54,8 @@ type scriptFileScanInput struct { naming string } -func scriptFileAIQualityFindings(env support.Context, root string, rel string, input scriptFileScanInput) []core.Finding { - abs := filepath.Join(root, rel) - data, err := os.ReadFile(abs) //nolint:gosec // path resolved under the scan-target root +func scriptFileAIQualityFindings(env support.Context, target core.TargetConfig, rel string, input scriptFileScanInput) []core.Finding { + data, err := readAITargetFile(env, target, rel) if err != nil { return nil } @@ -64,7 +63,7 @@ func scriptFileAIQualityFindings(env support.Context, root string, rel string, i checks := env.Config.Checks.QualityRules.AIChecks findings := make([]core.Finding, 0) if aiCheckEnabled(checks.HallucinatedImport) { - findings = append(findings, scriptImportFindings(env, root, rel, source, input.catalog)...) + findings = append(findings, scriptImportFindings(env, target.Path, rel, source, input.catalog)...) } if aiCheckEnabled(checks.DeadCode) { findings = append(findings, scriptDeadCodeFindings(env, rel, source)...) diff --git a/internal/codeguard/checks/quality/quality_ai_test_idioms.go b/internal/codeguard/checks/quality/quality_ai_test_idioms.go index 2af94b9..8598d7e 100644 --- a/internal/codeguard/checks/quality/quality_ai_test_idioms.go +++ b/internal/codeguard/checks/quality/quality_ai_test_idioms.go @@ -2,20 +2,12 @@ package quality import ( "fmt" - "os" - "path/filepath" "strings" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" ) -func dominantGoTestFramework(root string, files []string) string { - return dominantFramework(root, files, func(rel string, data string) (string, bool) { - return goTestFramework(data), strings.HasSuffix(rel, "_test.go") - }) -} - func goTestFramework(source string) string { switch { case strings.Contains(source, "github.com/onsi/ginkgo"): @@ -41,26 +33,11 @@ func countMarkers(source string, markers []string) int { return total } -func dominantFramework(root string, files []string, detector func(string, string) (string, bool)) string { - counts := map[string]int{} - for _, rel := range files { - framework, include := readFrameworkFile(root, rel, func(string) bool { return true }, func(data string) string { - framework, _ := detector(rel, data) - return framework - }) - if !include || framework == "" { - continue - } - counts[framework]++ - } - return dominantFrameworkFromCounts(counts) -} - -func readFrameworkFile(root string, rel string, include func(string) bool, detect func(string) string) (string, bool) { +func readFrameworkFile(env support.Context, target core.TargetConfig, rel string, include func(string) bool, detect func(string) string) (string, bool) { if !include(rel) { return "", false } - data, err := os.ReadFile(filepath.Join(root, rel)) //nolint:gosec // file under the scan-target root + data, err := readAITargetFile(env, target, rel) if err != nil { return "", false } diff --git a/internal/codeguard/checks/quality/quality_ai_test_idioms_script.go b/internal/codeguard/checks/quality/quality_ai_test_idioms_script.go index 95d708c..908255b 100644 --- a/internal/codeguard/checks/quality/quality_ai_test_idioms_script.go +++ b/internal/codeguard/checks/quality/quality_ai_test_idioms_script.go @@ -14,10 +14,10 @@ func isScriptTestFile(rel string) bool { return scriptTestFilePattern.MatchString(filepath.ToSlash(rel)) } -func dominantScriptTestFramework(root string, files []string, manifest packageManifest) string { +func dominantScriptTestFramework(env support.Context, target core.TargetConfig, files []string, manifest packageManifest) string { counts := frameworkSeedCounts(manifest) for _, rel := range files { - framework, include := readFrameworkFile(root, rel, isScriptTestFile, scriptTestFramework) + framework, include := readFrameworkFile(env, target, rel, isScriptTestFile, scriptTestFramework) if !include || framework == "" { continue } diff --git a/internal/codeguard/checks/quality/quality_clone.go b/internal/codeguard/checks/quality/quality_clone.go index 69da252..20b8c36 100644 --- a/internal/codeguard/checks/quality/quality_clone.go +++ b/internal/codeguard/checks/quality/quality_clone.go @@ -72,11 +72,39 @@ func detectCloneCandidates(docs []cloneDocument, threshold int) []cloneCandidate return candidates } +// cloneWindowMultiplier is the odd multiplier for the polynomial rolling +// window hash (Knuth's MMIX LCG constant). Multiplication by an odd constant +// modulo 2^64 is invertible, which lets a window's hash be updated in O(1) +// when the window slides by one token. +const cloneWindowMultiplier uint64 = 6364136223846793005 + +// cloneWindowIndex groups every threshold-token window by a rolling polynomial +// hash over the per-token hashes computed at tokenize time. Compared with the +// previous implementation (a fresh FNV over every token's bytes per window) +// this allocates nothing and slides in O(1) per window instead of O(threshold). +// Equal windows always collide (the hash is a deterministic function of the +// normalized tokens), and unequal windows that collide are discarded by the +// token-by-token verification in sharedCloneLength, so the resulting clone +// candidates are identical to the old per-window byte hashing. func cloneWindowIndex(docs []cloneDocument, threshold int) cloneIndex { index := make(cloneIndex) + // top = multiplier^(threshold-1), the weight of the token leaving the + // window on each slide. + top := uint64(1) + for i := 0; i < threshold-1; i++ { + top *= cloneWindowMultiplier + } for docIdx, doc := range docs { - for tokenIdx := 0; tokenIdx+threshold <= len(doc.Tokens); tokenIdx++ { - hash := cloneWindowHash(doc.Tokens[tokenIdx : tokenIdx+threshold]) + if len(doc.Tokens) < threshold { + continue + } + var hash uint64 + for i := 0; i < threshold; i++ { + hash = hash*cloneWindowMultiplier + doc.Tokens[i].Hash + } + index[hash] = append(index[hash], cloneOccurrence{DocIndex: docIdx, TokenIndex: 0}) + for tokenIdx := 1; tokenIdx+threshold <= len(doc.Tokens); tokenIdx++ { + hash = (hash-doc.Tokens[tokenIdx-1].Hash*top)*cloneWindowMultiplier + doc.Tokens[tokenIdx+threshold-1].Hash index[hash] = append(index[hash], cloneOccurrence{DocIndex: docIdx, TokenIndex: tokenIdx}) } } diff --git a/internal/codeguard/checks/quality/quality_clone_support.go b/internal/codeguard/checks/quality/quality_clone_support.go index fc98986..9b67668 100644 --- a/internal/codeguard/checks/quality/quality_clone_support.go +++ b/internal/codeguard/checks/quality/quality_clone_support.go @@ -1,7 +1,6 @@ package quality import ( - "hash/fnv" "path" "path/filepath" "regexp" @@ -75,26 +74,71 @@ func tokenizeNormalizedCloneText(source string) []cloneToken { prev := 0 for _, match := range matches { line += strings.Count(source[prev:match[0]], "\n") - value := strings.ToLower(source[match[0]:match[1]]) - tokens = append(tokens, cloneToken{Value: value, Line: line}) + // Slice the source directly instead of materializing a lowercased + // copy per token; normalization happens in the hash and in the + // case-folding comparison, both of which equal the historical + // lowercase semantics because tokens are ASCII by construction. + value := source[match[0]:match[1]] + tokens = append(tokens, cloneToken{Value: value, Hash: cloneTokenHash(value), Line: line}) prev = match[1] } return tokens } -func cloneWindowHash(tokens []cloneToken) uint64 { - hasher := fnv.New64a() - for _, token := range tokens { - _, _ = hasher.Write([]byte(token.Value)) - _, _ = hasher.Write([]byte{0}) +// FNV-1a constants (hash/fnv is not used directly so token hashing can fold +// ASCII case inline without allocating a lowercased copy of each token). +const ( + fnvOffset64 uint64 = 14695981039346694211 + fnvPrime64 uint64 = 1099511628211 +) + +// cloneTokenHash hashes a token's text with FNV-1a, lowercasing ASCII letters +// on the fly. Tokens only ever contain ASCII (see cloneTokenPattern), so this +// equals hashing strings.ToLower(text). +func cloneTokenHash(text string) uint64 { + hash := fnvOffset64 + for i := 0; i < len(text); i++ { + b := text[i] + if 'A' <= b && b <= 'Z' { + b += 'a' - 'A' + } + hash ^= uint64(b) + hash *= fnvPrime64 + } + return hash +} + +// cloneTokenTextEqual reports whether two token texts are equal ignoring ASCII +// case — exactly the historical comparison of lowercased token values, since +// token text is ASCII-only. +func cloneTokenTextEqual(a string, b string) bool { + if len(a) != len(b) { + return false + } + for i := 0; i < len(a); i++ { + ca, cb := a[i], b[i] + if 'A' <= ca && ca <= 'Z' { + ca += 'a' - 'A' + } + if 'A' <= cb && cb <= 'Z' { + cb += 'a' - 'A' + } + if ca != cb { + return false + } } - return hasher.Sum64() + return true } func sharedCloneLength(left []cloneToken, leftStart int, right []cloneToken, rightStart int) int { length := 0 for leftStart+length < len(left) && rightStart+length < len(right) { - if left[leftStart+length].Value != right[rightStart+length].Value { + l, r := left[leftStart+length], right[rightStart+length] + // Equal normalized text implies equal hashes, so a hash mismatch is a + // definitive inequality; the text comparison then guards against hash + // collisions, keeping the match semantics identical to comparing + // lowercased token values. + if l.Hash != r.Hash || !cloneTokenTextEqual(l.Value, r.Value) { break } length++ diff --git a/internal/codeguard/checks/quality/quality_clone_types.go b/internal/codeguard/checks/quality/quality_clone_types.go index 1af8e27..a2fa03f 100644 --- a/internal/codeguard/checks/quality/quality_clone_types.go +++ b/internal/codeguard/checks/quality/quality_clone_types.go @@ -1,8 +1,14 @@ package quality type cloneToken struct { + // Value is the token's original source text (always ASCII: the token + // pattern matches ASCII characters only). Comparisons fold ASCII case so + // behavior matches the historical lowercased-token equality. Value string - Line int + // Hash is the FNV-1a hash of the ASCII-lowercased token text, computed + // once at tokenize time so window hashing never rehashes token bytes. + Hash uint64 + Line int } type cloneDocument struct { diff --git a/internal/codeguard/checks/support/context.go b/internal/codeguard/checks/support/context.go index e93903e..d3822ff 100644 --- a/internal/codeguard/checks/support/context.go +++ b/internal/codeguard/checks/support/context.go @@ -33,8 +33,17 @@ type Context struct { ReadBaseFile func(target core.TargetConfig, rel string) ([]byte, error) DiffScope func() map[string]core.ChangedLineRanges VisitTargetFiles func(target core.TargetConfig, include func(string) bool, visit func(rel string, data []byte)) - ScanTargetFiles func(target core.TargetConfig, sectionID string, include func(string) bool, evaluator func(string, []byte) []core.Finding) []core.Finding - ParseGoFile func(path string, data []byte) (*token.FileSet, *ast.File, error) + // ListTargetFiles returns every non-excluded file under the target root from + // the shared per-scan corpus walk (the same listing VisitTargetFiles + // iterates). Nil in unit-test contexts; callers fall back to a direct walk. + ListTargetFiles func(target core.TargetConfig) ([]string, error) + // ReadTargetFile reads target-root-relative rel through the shared per-scan + // file corpus, so a file inspected by several checks is read from disk at + // most once per scan. Nil in unit-test contexts; callers fall back to a + // direct read. + ReadTargetFile func(target core.TargetConfig, rel string) ([]byte, error) + ScanTargetFiles func(target core.TargetConfig, sectionID string, include func(string) bool, evaluator func(string, []byte) []core.Finding) []core.Finding + ParseGoFile func(path string, data []byte) (*token.FileSet, *ast.File, error) // ParseScriptFile parses a TypeScript/TSX/JavaScript file through the // tree-sitter substrate. It is nil unless parsers.treesitter is "auto"; // checks treat nil (and any error) as "use the regex path". diff --git a/internal/codeguard/config/defaults_rules.go b/internal/codeguard/config/defaults_rules.go index 639d3e0..12c6ff7 100644 --- a/internal/codeguard/config/defaults_rules.go +++ b/internal/codeguard/config/defaults_rules.go @@ -136,9 +136,13 @@ func applyContextDefaults(dst *core.ContextRulesConfig, def core.ContextRulesCon &dst.DetectReadmeDrift, &dst.DetectOversizedFiles, &dst.DetectAmbiguousSymbols, + &dst.DetectUndocumentedCommands, + &dst.DetectOversizedAgentDocs, + &dst.DetectDocLinkRot, ) defaultInt(&dst.MaxFileLines, def.MaxFileLines) defaultInt(&dst.AmbiguousSymbolThreshold, def.AmbiguousSymbolThreshold) + defaultInt(&dst.MaxAgentDocLines, def.MaxAgentDocLines) } func applySupplyChainDefaults(dst *core.SupplyChainRulesConfig, def core.SupplyChainRulesConfig) { diff --git a/internal/codeguard/config/example.go b/internal/codeguard/config/example.go index 6406021..e4ba3bd 100644 --- a/internal/codeguard/config/example.go +++ b/internal/codeguard/config/example.go @@ -50,13 +50,17 @@ func exampleChecks() core.CheckConfig { func exampleContextRules() core.ContextRulesConfig { return core.ContextRulesConfig{ - DetectMissingAgentDocs: boolPtr(true), - DetectAgentDocsDrift: boolPtr(true), - DetectReadmeDrift: boolPtr(true), - DetectOversizedFiles: boolPtr(true), - DetectAmbiguousSymbols: boolPtr(true), - MaxFileLines: 1500, - AmbiguousSymbolThreshold: 4, + DetectMissingAgentDocs: boolPtr(true), + DetectAgentDocsDrift: boolPtr(true), + DetectReadmeDrift: boolPtr(true), + DetectOversizedFiles: boolPtr(true), + DetectAmbiguousSymbols: boolPtr(true), + DetectUndocumentedCommands: boolPtr(true), + DetectOversizedAgentDocs: boolPtr(true), + DetectDocLinkRot: boolPtr(true), + MaxFileLines: 1500, + AmbiguousSymbolThreshold: 4, + MaxAgentDocLines: 600, } } diff --git a/internal/codeguard/config/validate_context.go b/internal/codeguard/config/validate_context.go index 3525bcc..4cf56e3 100644 --- a/internal/codeguard/config/validate_context.go +++ b/internal/codeguard/config/validate_context.go @@ -13,5 +13,23 @@ func validateContextRules(cfg core.ContextRulesConfig) error { if cfg.AmbiguousSymbolThreshold < 0 || cfg.AmbiguousSymbolThreshold == 1 { return errors.New("context_rules.ambiguous_symbol_threshold must be at least 2") } + if cfg.MaxAgentDocLines < 0 { + return errors.New("context_rules.max_agent_doc_lines must be positive") + } + if cfg.LegibilityWarnThreshold < 0 || cfg.LegibilityWarnThreshold > 100 { + return errors.New("context_rules.legibility_warn_threshold must be between 0 and 100") + } + if cfg.LegibilityFailThreshold < 0 || cfg.LegibilityFailThreshold > 100 { + return errors.New("context_rules.legibility_fail_threshold must be between 0 and 100") + } + // Legibility is good-high: the finding fires when the score drops below a + // threshold, so the fail bar must sit at or below the warn bar (the + // inverse of the slop-score threshold ordering). + if cfg.LegibilityFailThreshold > 0 && cfg.LegibilityWarnThreshold > 0 && cfg.LegibilityFailThreshold > cfg.LegibilityWarnThreshold { + return errors.New("context_rules.legibility_fail_threshold must be less than or equal to legibility_warn_threshold") + } + if cfg.LegibilityHistoryLimit < 0 { + return errors.New("context_rules.legibility_history_limit must be non-negative") + } return nil } diff --git a/internal/codeguard/core/config_rule_types.go b/internal/codeguard/core/config_rule_types.go index 2809f89..38d6e40 100644 --- a/internal/codeguard/core/config_rule_types.go +++ b/internal/codeguard/core/config_rule_types.go @@ -176,6 +176,16 @@ type ContextRulesConfig struct { DetectReadmeDrift *bool `json:"detect_readme_drift,omitempty" yaml:"detect_readme_drift,omitempty"` DetectOversizedFiles *bool `json:"detect_oversized_files,omitempty" yaml:"detect_oversized_files,omitempty"` DetectAmbiguousSymbols *bool `json:"detect_ambiguous_symbols,omitempty" yaml:"detect_ambiguous_symbols,omitempty"` + // DetectUndocumentedCommands warns when a high-signal Makefile target or + // package.json script is not mentioned by any agent instruction file or the + // root README. Silent when the repo has no agent docs at all. + DetectUndocumentedCommands *bool `json:"detect_undocumented_commands,omitempty" yaml:"detect_undocumented_commands,omitempty"` + // DetectOversizedAgentDocs warns when an agent instruction file exceeds + // MaxAgentDocLines, consuming the context budget it exists to save. + DetectOversizedAgentDocs *bool `json:"detect_oversized_agent_docs,omitempty" yaml:"detect_oversized_agent_docs,omitempty"` + // DetectDocLinkRot warns when a markdown link in an agent doc or the root + // README points at a repository path that does not exist. + DetectDocLinkRot *bool `json:"detect_doc_link_rot,omitempty" yaml:"detect_doc_link_rot,omitempty"` // MaxFileLines is the agent context budget for a single source file. // Distinct from quality_rules.max_file_lines: this threshold is about how // much of an agent's context window one unit of work consumes, so its @@ -184,6 +194,33 @@ type ContextRulesConfig struct { // AmbiguousSymbolThreshold is the number of source files sharing one // basename at which the basename is reported as ambiguous (default 4). AmbiguousSymbolThreshold int `json:"ambiguous_symbol_threshold,omitempty" yaml:"ambiguous_symbol_threshold,omitempty"` + // MaxAgentDocLines is the line budget for a single agent instruction file + // (default 600); larger docs crowd out the working context they document. + MaxAgentDocLines int `json:"max_agent_doc_lines,omitempty" yaml:"max_agent_doc_lines,omitempty"` + // AmbiguousSymbolIgnore lists source-file basenames that never count as + // ambiguous: conventional names imposed by a language or framework + // (index.ts, __init__.py, mod.rs, ...) are expected to repeat. When set it + // REPLACES the built-in default list entirely (set it to [] to disable + // ignoring); when omitted the documented default set applies. Ignored + // basenames are excluded from both context.ambiguous-symbol findings and + // the repo_legibility navigability component. + AmbiguousSymbolIgnore []string `json:"ambiguous_symbol_ignore,omitempty" yaml:"ambiguous_symbol_ignore,omitempty"` + // LegibilityWarnThreshold and LegibilityFailThreshold gate the + // repo_legibility score (0-100, higher is better). Unlike the slop-score + // thresholds — where a HIGH score is bad and the finding fires when the + // score rises to the threshold — legibility is good-high, so the + // context.legibility-threshold finding fires when the computed score falls + // BELOW a threshold. 0 disables a threshold; when both are set the fail + // threshold must be less than or equal to the warn threshold. + LegibilityWarnThreshold int `json:"legibility_warn_threshold,omitempty" yaml:"legibility_warn_threshold,omitempty"` + LegibilityFailThreshold int `json:"legibility_fail_threshold,omitempty" yaml:"legibility_fail_threshold,omitempty"` + // LegibilityHistory gates persistence of the repo_legibility score trend + // next to the scan cache (nil = enabled, mirroring + // performance_rules.score_history and ai_checks.slop_history). + LegibilityHistory *bool `json:"legibility_history,omitempty" yaml:"legibility_history,omitempty"` + // LegibilityHistoryLimit caps retained repo_legibility history entries per + // target (0 = default limit of 100). + LegibilityHistoryLimit int `json:"legibility_history_limit,omitempty" yaml:"legibility_history_limit,omitempty"` } type CommandCheckConfig struct { diff --git a/internal/codeguard/core/report_artifact_legibility_history_types.go b/internal/codeguard/core/report_artifact_legibility_history_types.go new file mode 100644 index 0000000..be697f1 --- /dev/null +++ b/internal/codeguard/core/report_artifact_legibility_history_types.go @@ -0,0 +1,10 @@ +package core + +// LegibilityHistoryEntry is one persisted repo_legibility observation for a +// target, recorded once per scan so the AI-readiness trend can be reported +// over time, mirroring SlopHistoryEntry and PerformanceHistoryEntry. +type LegibilityHistoryEntry struct { + Timestamp string `json:"timestamp"` + Score int `json:"score"` + Components []RepoLegibilityComponent `json:"components,omitempty"` +} diff --git a/internal/codeguard/core/report_artifact_legibility_types.go b/internal/codeguard/core/report_artifact_legibility_types.go index 153a107..7328779 100644 --- a/internal/codeguard/core/report_artifact_legibility_types.go +++ b/internal/codeguard/core/report_artifact_legibility_types.go @@ -6,8 +6,10 @@ package core // README presence; Components carries the per-signal breakdown so the score // is explainable rather than a bare number. type RepoLegibilityArtifact struct { - Score int `json:"score"` - Components []RepoLegibilityComponent `json:"components,omitempty"` + Score int `json:"score"` + Components []RepoLegibilityComponent `json:"components,omitempty"` + PreviousScore *int `json:"previous_score,omitempty"` + Delta *int `json:"delta,omitempty"` } // RepoLegibilityComponent is one explainable slice of the legibility score: diff --git a/internal/codeguard/rules/catalog.go b/internal/codeguard/rules/catalog.go index d99f06f..ea7ed18 100644 --- a/internal/codeguard/rules/catalog.go +++ b/internal/codeguard/rules/catalog.go @@ -15,6 +15,7 @@ var catalog = withSecurityOWASP(mergeRuleCatalogs( securityExtraCatalog, supplyChainCatalog, contextCatalog, + contextReadinessCatalog, contractsCatalog, securityTaintCatalog, miscCatalog, diff --git a/internal/codeguard/rules/catalog_context.go b/internal/codeguard/rules/catalog_context.go index 1e932e8..74aa557 100644 --- a/internal/codeguard/rules/catalog_context.go +++ b/internal/codeguard/rules/catalog_context.go @@ -43,6 +43,16 @@ var contextCatalog = map[string]core.RuleMetadata{ Description: "Warns when a source file exceeds the agent context budget (context_rules.max_file_lines, default 1500): a file that large crowds out the rest of an AI agent's working context.", HowToFix: "Split the file into smaller, focused units so an agent can load only the part relevant to its task; extract cohesive sections into their own files.", }, + "context.legibility-threshold": { + ID: "context.legibility-threshold", + Section: "Agent Context", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Repository legibility below threshold", + Description: "Fires when the repo_legibility score (0-100, higher is better) falls below a configured floor: warns below context_rules.legibility_warn_threshold and fails below context_rules.legibility_fail_threshold (0 disables each). The message carries the per-component breakdown so the weakest signal is visible.", + HowToFix: "Raise the weakest components named in the finding: give agent docs real substance and fix their stale references, repair broken doc/README references, split files that exceed the context budget, and rename duplicated basenames — or lower the configured threshold if the current bar is intentional.", + }, "context.ambiguous-symbol": { ID: "context.ambiguous-symbol", Section: "Agent Context", diff --git a/internal/codeguard/rules/catalog_context_readiness.go b/internal/codeguard/rules/catalog_context_readiness.go new file mode 100644 index 0000000..ca05126 --- /dev/null +++ b/internal/codeguard/rules/catalog_context_readiness.go @@ -0,0 +1,39 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +// contextReadinessCatalog covers the "AI and human ready" extension of the +// agent-context family: commands the docs never mention, agent docs too large +// for the context they manage, and markdown links that rot. +var contextReadinessCatalog = map[string]core.RuleMetadata{ + "context.undocumented-commands": { + ID: "context.undocumented-commands", + Section: "Agent Context", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Undocumented dev command", + Description: "Warns when a high-signal Makefile target or package.json script (build, check, dev, fmt, lint, run, start, test) is mentioned by no agent instruction file and not even the root README, so agents cannot discover the repo's canonical entrypoints. Silent when the repo has no agent docs at all.", + HowToFix: "Mention the command in CLAUDE.md/AGENTS.md (or the README) alongside when to use it, or remove the target/script if it is no longer part of the canonical workflow.", + }, + "context.oversized-agent-doc": { + ID: "context.oversized-agent-doc", + Section: "Agent Context", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Oversized agent instructions", + Description: "Warns when an agent instruction file exceeds context_rules.max_agent_doc_lines (default 600): agent docs are loaded into every session verbatim, so an oversized one consumes the context window it exists to save.", + HowToFix: "Keep the agent instruction file to essentials — build/test commands, layout, conventions — and move reference material into separate docs the agent can load on demand.", + }, + "context.doc-link-rot": { + ID: "context.doc-link-rot", + Section: "Agent Context", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: core.RepositoryWideRuleLanguageCoverage(), + Title: "Documentation link rot", + Description: "Warns when a markdown link in an agent instruction file or the root README points at a repository file or directory that does not exist. External URLs are never checked (no network I/O) and anchors are ignored.", + HowToFix: "Point the link at the file's current location, or remove the link if the document it referenced is gone.", + }, +} diff --git a/internal/codeguard/rules/catalog_fix_templates.go b/internal/codeguard/rules/catalog_fix_templates.go index f5f4422..53bd967 100644 --- a/internal/codeguard/rules/catalog_fix_templates.go +++ b/internal/codeguard/rules/catalog_fix_templates.go @@ -27,6 +27,7 @@ var fixTemplates = mergeFixTemplates( designFixTemplates, miscFixTemplates, contextFixTemplates, + contextReadinessFixTemplates, ) func mergeFixTemplates(parts ...map[string]core.FixTemplate) map[string]core.FixTemplate { diff --git a/internal/codeguard/rules/catalog_fix_templates_context.go b/internal/codeguard/rules/catalog_fix_templates_context.go index 38bd5f5..19ddd21 100644 --- a/internal/codeguard/rules/catalog_fix_templates_context.go +++ b/internal/codeguard/rules/catalog_fix_templates_context.go @@ -9,4 +9,5 @@ var contextFixTemplates = map[string]core.FixTemplate{ "context.readme-drift": {Kind: guided, Text: "Update the README's command examples to match the current scripts and make targets.\n\nBefore:\n```bash\n./scripts/setup.sh\nmake bootstrap\n```\n\nAfter:\n```bash\n./scripts/dev-setup.sh\nmake build\n```"}, "context.oversized-context-unit": {Kind: guided, Text: "Split the file into cohesive units that each fit an agent's working context.\n\nBefore:\n// handlers.go: 2400 lines mixing auth, billing, and admin endpoints\n\nAfter:\n// handlers_auth.go, handlers_billing.go, handlers_admin.go\n// each under the configured line budget, grouped by responsibility"}, "context.ambiguous-symbol": {Kind: guided, Text: "Rename duplicated basenames so search results identify a file unambiguously.\n\nBefore:\napi/utils.ts, billing/utils.ts, auth/utils.ts, admin/utils.ts\n\nAfter:\napi/http_helpers.ts, billing/invoice_math.ts, auth/token_helpers.ts, admin/audit_format.ts"}, + "context.legibility-threshold": {Kind: guided, Text: "Raise the components the finding names as weakest until the score clears the configured floor.\n\nBefore:\nrepository legibility score 58 below warn threshold 80\n(agent_docs 5/25, readme 10/10, doc_accuracy 8/20, context_economy 15/25, navigability 20/20)\n\nAfter:\n1. agent_docs: flesh out CLAUDE.md with real build/test/layout guidance and fix its stale references\n2. doc_accuracy: update or delete doc references that no longer resolve\n3. context_economy: split files that exceed context_rules.max_file_lines\nRe-run the scan; the score and trend are visible via codeguard report -legibility-history"}, } diff --git a/internal/codeguard/rules/catalog_fix_templates_context_readiness.go b/internal/codeguard/rules/catalog_fix_templates_context_readiness.go new file mode 100644 index 0000000..182a20a --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_context_readiness.go @@ -0,0 +1,11 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +// contextReadinessFixTemplates covers the AI-and-human-readiness extension of +// the agent-context family. +var contextReadinessFixTemplates = map[string]core.FixTemplate{ + "context.undocumented-commands": {Kind: guided, Text: "Name the canonical command in an agent doc (or the README) so agents can discover it.\n\nBefore:\n# CLAUDE.md\n## Build & test\n- make build\n\nAfter:\n# CLAUDE.md\n## Build & test\n- make build\n- make test — run the unit suite\n- make lint — golangci-lint, CI-blocking"}, + "context.oversized-agent-doc": {Kind: guided, Text: "Trim the agent doc to essentials and link out to reference material.\n\nBefore:\n# CLAUDE.md (900 lines: commands, style guide, API reference, changelog)\n\nAfter:\n# CLAUDE.md (essentials only)\n## Build & test\n- make build / make test\n## Deep dives\n- docs/style-guide.md\n- docs/api-reference.md"}, + "context.doc-link-rot": {Kind: guided, Text: "Repoint the broken link at the file's current location, or drop it.\n\nBefore:\n- [Architecture](docs/design/architecture.md)\n\nAfter:\n- [Architecture](docs/architecture.md)"}, +} diff --git a/internal/codeguard/runner/checks/checks.go b/internal/codeguard/runner/checks/checks.go index 15e90e0..1d18651 100644 --- a/internal/codeguard/runner/checks/checks.go +++ b/internal/codeguard/runner/checks/checks.go @@ -150,6 +150,12 @@ func buildCheckContext(ctx context.Context, sc runnersupport.Context) checkSuppo VisitTargetFiles: func(target core.TargetConfig, include func(string) bool, visit func(rel string, data []byte)) { runnersupport.VisitTargetFiles(sc, target, include, visit) }, + ListTargetFiles: func(target core.TargetConfig) ([]string, error) { + return runnersupport.ListTargetFiles(sc, target) + }, + ReadTargetFile: func(target core.TargetConfig, rel string) ([]byte, error) { + return runnersupport.ReadTargetFile(sc, target, rel) + }, DiffScope: func() map[string]core.ChangedLineRanges { out := make(map[string]core.ChangedLineRanges, len(sc.Diff)) for path, ranges := range sc.Diff { diff --git a/internal/codeguard/runner/legibility_history.go b/internal/codeguard/runner/legibility_history.go new file mode 100644 index 0000000..c8fcea4 --- /dev/null +++ b/internal/codeguard/runner/legibility_history.go @@ -0,0 +1,20 @@ +package runner + +import ( + "github.com/devr-tools/codeguard/internal/codeguard/config" + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +// LegibilityHistoryPath derives the repo_legibility history file path for a +// config, mirroring SlopHistoryPath and PerfScoreHistoryPath. +func LegibilityHistoryPath(cfg core.Config) string { + config.ApplyDefaults(&cfg) + return runnersupport.LegibilityHistoryPathForBase(cfg.Cache.Path) +} + +// LoadLegibilityHistory reads the persisted repo_legibility trend, keyed by +// artifact ID. +func LoadLegibilityHistory(path string) map[string][]core.LegibilityHistoryEntry { + return runnersupport.LoadLegibilityHistory(path) +} diff --git a/internal/codeguard/runner/support/artifacts.go b/internal/codeguard/runner/support/artifacts.go index 0561234..b8df470 100644 --- a/internal/codeguard/runner/support/artifacts.go +++ b/internal/codeguard/runner/support/artifacts.go @@ -76,6 +76,21 @@ func VisitTargetFiles(sc Context, target core.TargetConfig, include func(string) } } +// ListTargetFiles returns every non-excluded file under the target root, +// sharing the per-scan corpus walk with every other section (the same listing +// VisitTargetFiles iterates). Callers apply their own include filter to the +// result. +func ListTargetFiles(sc Context, target core.TargetConfig) ([]string, error) { + return sc.corpusFiles(target.Path) +} + +// ReadTargetFile returns the bytes of target-root-relative rel via the shared +// per-scan corpus, so a file inspected by several checks is read from disk at +// most once per scan. +func ReadTargetFile(sc Context, target core.TargetConfig, rel string) ([]byte, error) { + return sc.corpusRead(target.Path, rel) +} + // ChangedDiffFiles returns the sorted set of changed file paths in diff mode. func ChangedDiffFiles(sc Context) []string { if len(sc.Diff) == 0 { diff --git a/internal/codeguard/runner/support/legibility_history.go b/internal/codeguard/runner/support/legibility_history.go new file mode 100644 index 0000000..c22a9e9 --- /dev/null +++ b/internal/codeguard/runner/support/legibility_history.go @@ -0,0 +1,90 @@ +package support + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +const legibilityHistoryVersion = 1 + +// DefaultLegibilityHistoryLimit caps how many scans are retained per target +// key, mirroring DefaultSlopHistoryLimit. +const DefaultLegibilityHistoryLimit = 100 + +type legibilityHistoryFile struct { + Version int `json:"version"` + Entries map[string][]core.LegibilityHistoryEntry `json:"entries"` +} + +// LegibilityHistoryPathForBase derives the repo_legibility history file path +// from the scan cache path, mirroring SlopHistoryPathForBase. +func LegibilityHistoryPathForBase(base string) string { + trimmed := strings.TrimSpace(base) + if trimmed == "" { + return "" + } + ext := filepath.Ext(trimmed) + if ext == "" { + return trimmed + ".legibility-history" + } + return strings.TrimSuffix(trimmed, ext) + ".legibility-history" + ext +} + +// LoadLegibilityHistory reads the persisted legibility-score history keyed by +// artifact ID. A missing or unreadable file yields an empty history. +func LoadLegibilityHistory(path string) map[string][]core.LegibilityHistoryEntry { + if strings.TrimSpace(path) == "" { + return map[string][]core.LegibilityHistoryEntry{} + } + data, err := os.ReadFile(path) //nolint:gosec // config-supplied legibility-history cache path + if err != nil { + return map[string][]core.LegibilityHistoryEntry{} + } + var file legibilityHistoryFile + if err := json.Unmarshal(data, &file); err != nil || file.Version != legibilityHistoryVersion || file.Entries == nil { + return map[string][]core.LegibilityHistoryEntry{} + } + return file.Entries +} + +// AppendLegibilityHistory records a new observation for key, trims the +// history to limit entries, and persists the file. It returns the previous +// entry, if one existed, so callers can report score deltas. +func AppendLegibilityHistory(path string, key string, entry core.LegibilityHistoryEntry, limit int) (core.LegibilityHistoryEntry, bool) { + if strings.TrimSpace(path) == "" || strings.TrimSpace(key) == "" { + return core.LegibilityHistoryEntry{}, false + } + if limit <= 0 { + limit = DefaultLegibilityHistoryLimit + } + entries := LoadLegibilityHistory(path) + history := entries[key] + var previous core.LegibilityHistoryEntry + hasPrevious := len(history) > 0 + if hasPrevious { + previous = history[len(history)-1] + } + history = append(history, entry) + if len(history) > limit { + history = history[len(history)-limit:] + } + entries[key] = history + saveLegibilityHistory(path, entries) + return previous, hasPrevious +} + +func saveLegibilityHistory(path string, entries map[string][]core.LegibilityHistoryEntry) { + payload := legibilityHistoryFile{Version: legibilityHistoryVersion, Entries: entries} + data, err := json.MarshalIndent(payload, "", " ") + if err != nil { + return + } + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + return + } + _ = os.WriteFile(path, append(data, '\n'), 0o600) +} diff --git a/internal/codeguard/runner/support/utils.go b/internal/codeguard/runner/support/utils.go index 50aba81..c8d5042 100644 --- a/internal/codeguard/runner/support/utils.go +++ b/internal/codeguard/runner/support/utils.go @@ -1,6 +1,7 @@ package support import ( + "bytes" "go/ast" "io/fs" "path/filepath" @@ -164,8 +165,17 @@ func compilePattern(pattern string) (*regexp.Regexp, bool) { return re, true } +// CountLines reports how many lines data spans without allocating. It +// preserves the exact semantics of the previous +// strings.Split(strings.TrimRight(...))-based implementation: all trailing +// newlines are ignored and empty input still counts as one line, so e.g. +// "a\n" and "a\n\n" are 1 line and "" is 1 line. func CountLines(data []byte) int { - return len(strings.Split(strings.TrimRight(string(data), "\n"), "\n")) + end := len(data) + for end > 0 && data[end-1] == '\n' { + end-- + } + return bytes.Count(data[:end], []byte{'\n'}) + 1 } func TypeName(expr ast.Expr) string { diff --git a/pkg/codeguard/sdk_legibility.go b/pkg/codeguard/sdk_legibility.go new file mode 100644 index 0000000..80d23c4 --- /dev/null +++ b/pkg/codeguard/sdk_legibility.go @@ -0,0 +1,22 @@ +package codeguard + +import ( + "github.com/devr-tools/codeguard/internal/codeguard/core" + "github.com/devr-tools/codeguard/internal/codeguard/runner" +) + +// LegibilityHistoryEntry is one persisted repo_legibility observation for a +// target. +type LegibilityHistoryEntry = core.LegibilityHistoryEntry + +// LegibilityHistoryPath derives the repo_legibility history file path for a +// config, mirroring SlopHistoryPath and PerfScoreHistoryPath. +func LegibilityHistoryPath(cfg Config) string { + return runner.LegibilityHistoryPath(cfg) +} + +// LoadLegibilityHistory reads the persisted repo_legibility trend, keyed by +// artifact ID. +func LoadLegibilityHistory(path string) map[string][]LegibilityHistoryEntry { + return runner.LoadLegibilityHistory(path) +} diff --git a/tests/checks/agentcontext_artifact_test.go b/tests/checks/agentcontext_artifact_test.go index a17fa9a..22091ab 100644 --- a/tests/checks/agentcontext_artifact_test.go +++ b/tests/checks/agentcontext_artifact_test.go @@ -41,15 +41,16 @@ func TestRepoLegibilityArtifactExplainsPenalties(t *testing.T) { } artifact := requireRepoLegibilityArtifact(t, report) - // agent_docs 0/25, readme 10/10, doc_accuracy 16/20 (one broken - // reference), context_economy 25/25, navigability 20/20. - if artifact.RepoLegibility.Score != 71 { - t.Fatalf("score = %d, want 71: %+v", artifact.RepoLegibility.Score, artifact.RepoLegibility.Components) + // agent_docs 0/25, readme 10/10, doc_accuracy 0/20 (the single doc + // reference is broken, so 100% of the documentation is wrong), + // context_economy 25/25, navigability 20/20. + if artifact.RepoLegibility.Score != 55 { + t.Fatalf("score = %d, want 55: %+v", artifact.RepoLegibility.Score, artifact.RepoLegibility.Components) } if component := legibilityComponent(t, artifact, "agent_docs"); component.Score != 0 { t.Fatalf("agent_docs score = %d, want 0", component.Score) } - if component := legibilityComponent(t, artifact, "doc_accuracy"); component.Score != 16 || !strings.Contains(component.Detail, "1 unresolvable") { + if component := legibilityComponent(t, artifact, "doc_accuracy"); component.Score != 0 || !strings.Contains(component.Detail, "1 of 1 doc references unresolvable") { t.Fatalf("unexpected doc_accuracy component: %+v", component) } if component := legibilityComponent(t, artifact, "readme"); component.Score != 10 { @@ -101,8 +102,9 @@ func TestRepoLegibilityArtifactCountsOversizedAndAmbiguousRatios(t *testing.T) { } artifact := requireRepoLegibilityArtifact(t, report) - // 1 of 5 source files oversized: penalty min(25, 25*1*10/5) = 25. - if component := legibilityComponent(t, artifact, "context_economy"); component.Score != 0 || !strings.Contains(component.Detail, "1 of 5") { + // 1 of 5 source files oversized (20%): the linear ramp to zero at 25% + // yields penalty round(100*1/5) = 20, so 5 of 25 points remain. + if component := legibilityComponent(t, artifact, "context_economy"); component.Score != 5 || !strings.Contains(component.Detail, "1 of 5") { t.Fatalf("unexpected context_economy component: %+v", component) } // 4 of 5 source files share a basename: penalty min(20, 20*4*5/5) = 20. diff --git a/tests/checks/agentcontext_calibration_test.go b/tests/checks/agentcontext_calibration_test.go new file mode 100644 index 0000000..988198f --- /dev/null +++ b/tests/checks/agentcontext_calibration_test.go @@ -0,0 +1,183 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// The agent_docs component is gated on substance: an empty agent doc no +// longer banks the full 25 points just by existing. +func TestRepoLegibilityAgentDocsComponentRequiresSubstance(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n") + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "\n\n\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "agent-docs-empty")) + if err != nil { + t.Fatalf("run: %v", err) + } + + artifact := requireRepoLegibilityArtifact(t, report) + component := legibilityComponent(t, artifact, "agent_docs") + if component.Score != 0 { + t.Fatalf("empty CLAUDE.md agent_docs score = %d, want 0: %+v", component.Score, component) + } + if !strings.Contains(component.Detail, "0 non-blank lines") || !strings.Contains(component.Detail, "substance 0/25") { + t.Fatalf("agent_docs detail should explain the substance formula: %q", component.Detail) + } +} + +// Substance credit scales linearly up to 10 non-blank lines. +func TestRepoLegibilityAgentDocsComponentScalesWithContent(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n") + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), strings.Repeat("guidance line without references\n", 4)) + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "agent-docs-partial")) + if err != nil { + t.Fatalf("run: %v", err) + } + + artifact := requireRepoLegibilityArtifact(t, report) + // 4 non-blank lines of 10 required: substance 25*4/10 = 10. + if component := legibilityComponent(t, artifact, "agent_docs"); component.Score != 10 { + t.Fatalf("agent_docs score = %d, want 10: %+v", component.Score, component) + } +} + +// A substantial agent doc riddled with stale references loses agent_docs +// credit on top of the shared doc_accuracy penalty. +func TestRepoLegibilityAgentDocsComponentPenalizesDrift(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n") + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + lines := make([]string, 0, 12) + for i := 0; i < 10; i++ { + lines = append(lines, "guidance line without references") + } + lines = append(lines, "Edit `internal/gone/one.go` and `internal/gone/two.go`.") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), strings.Join(lines, "\n")+"\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "agent-docs-drift-penalty")) + if err != nil { + t.Fatalf("run: %v", err) + } + + artifact := requireRepoLegibilityArtifact(t, report) + // Full substance (11 non-blank lines), minus 2 per unresolvable agent-doc + // reference: 25 - 2*2 = 21. + component := legibilityComponent(t, artifact, "agent_docs") + if component.Score != 21 || !strings.Contains(component.Detail, "2 unresolvable references") { + t.Fatalf("agent_docs score = %d, want 21: %+v", component.Score, component) + } +} + +// doc_accuracy scales with the broken share instead of the old flat -4 that +// saturated at 5 references: 2 broken of 10 costs 4 points, not 8. +func TestRepoLegibilityDocAccuracyScalesProportionally(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + for i := 0; i < 8; i++ { + writeFile(t, filepath.Join(dir, "scripts", "tool"+string(rune('a'+i))+".sh"), "#!/bin/sh\necho ok\n") + } + commands := []string{"# demo", "", "```bash"} + for i := 0; i < 8; i++ { + commands = append(commands, "./scripts/tool"+string(rune('a'+i))+".sh") + } + commands = append(commands, "./scripts/gone-one.sh", "./scripts/gone-two.sh", "```") + writeFile(t, filepath.Join(dir, "README.md"), strings.Join(commands, "\n")+"\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "doc-accuracy-proportional")) + if err != nil { + t.Fatalf("run: %v", err) + } + + artifact := requireRepoLegibilityArtifact(t, report) + // 2 broken of 10 references: penalty round(20*2/10) = 4. + component := legibilityComponent(t, artifact, "doc_accuracy") + if component.Score != 16 || !strings.Contains(component.Detail, "2 of 10 doc references unresolvable") { + t.Fatalf("doc_accuracy score = %d, want 16: %+v", component.Score, component) + } +} + +// context_economy degrades gradually: 10% oversized used to zero the whole +// component; under the linear ramp to 25% it now costs 10 of 25 points. +func TestRepoLegibilityContextEconomySoftensCurve(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "# CLAUDE.md\n") + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n") + writeFile(t, filepath.Join(dir, "big.go"), goFileWithLines(40)) + for i := 0; i < 9; i++ { + writeFile(t, filepath.Join(dir, "pkg"+string(rune('a'+i)), "small"+string(rune('a'+i))+".go"), "package fixture\n") + } + + cfg := agentContextTestConfig(dir, "economy-soft-curve") + cfg.Checks.ContextRules.MaxFileLines = 30 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + artifact := requireRepoLegibilityArtifact(t, report) + // 1 of 10 source files oversized: penalty round(100*1/10) = 10. + component := legibilityComponent(t, artifact, "context_economy") + if component.Score != 15 || !strings.Contains(component.Detail, "1 of 10") { + t.Fatalf("context_economy score = %d, want 15: %+v", component.Score, component) + } +} + +// Conventional basenames (index.ts, __init__.py, ...) are navigation noise an +// agent expects; they must not fire findings or drain the navigability score. +func TestRepoLegibilityNavigabilityIgnoresConventionalBasenames(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "# CLAUDE.md\n") + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n") + for _, sub := range []string{"api", "web", "cli", "db", "auth"} { + writeFile(t, filepath.Join(dir, sub, "index.ts"), "export const ns = \""+sub+"\";\n") + } + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "conventional-basenames")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Agent Context", "context.ambiguous-symbol") + artifact := requireRepoLegibilityArtifact(t, report) + if component := legibilityComponent(t, artifact, "navigability"); component.Score != 20 { + t.Fatalf("navigability score = %d, want 20 (index.ts is conventional): %+v", component.Score, component) + } +} + +// context_rules.ambiguous_symbol_ignore replaces the default ignore list: +// custom entries take effect and the built-in defaults stop applying. +func TestRepoLegibilityAmbiguousIgnoreListIsConfigurable(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "# CLAUDE.md\n") + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n") + for _, sub := range []string{"api", "web", "cli", "db"} { + writeFile(t, filepath.Join(dir, sub, "utils.ts"), "export const ns = \""+sub+"\";\n") + writeFile(t, filepath.Join(dir, sub, "index.ts"), "export default \""+sub+"\";\n") + } + + cfg := agentContextTestConfig(dir, "ambiguous-ignore-config") + cfg.Checks.ContextRules.AmbiguousSymbolIgnore = []string{"utils.ts"} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + // utils.ts is now ignored; index.ts is flagged because the custom list + // replaced the defaults. + messages := agentContextRuleMessages(report, "context.ambiguous-symbol") + if len(messages) != 1 || !strings.Contains(messages[0], `"index.ts"`) { + t.Fatalf("expected exactly one index.ts finding under the replaced ignore list, got: %v", messages) + } +} diff --git a/tests/checks/agentcontext_history_test.go b/tests/checks/agentcontext_history_test.go new file mode 100644 index 0000000..9a1b52e --- /dev/null +++ b/tests/checks/agentcontext_history_test.go @@ -0,0 +1,92 @@ +package checks_test + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// legibilityHistoryConfig enables the cache so the trend file is written next +// to it, mirroring the slop/perf history plumbing. +func legibilityHistoryConfig(t *testing.T, repo string, cacheDir string, name string) codeguard.Config { + t.Helper() + cfg := agentContextTestConfig(repo, name) + on := true + cfg.Cache.Enabled = &on + cfg.Cache.Path = filepath.Join(cacheDir, "cache.json") + return cfg +} + +func TestLegibilityHistoryPersistsTrendAndAnnotatesDelta(t *testing.T) { + dir := t.TempDir() + repo := filepath.Join(dir, "repo") + if err := os.MkdirAll(repo, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + writeLegibleRepoFixture(t, repo) + cfg := legibilityHistoryConfig(t, repo, dir, "legibility-history") + + first, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("first run: %v", err) + } + if artifact := requireRepoLegibilityArtifact(t, first); artifact.RepoLegibility.PreviousScore != nil { + t.Fatalf("first scan must not report a previous score, got %+v", artifact.RepoLegibility) + } + + second, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("second run: %v", err) + } + artifact := requireRepoLegibilityArtifact(t, second) + if artifact.RepoLegibility.PreviousScore == nil || artifact.RepoLegibility.Delta == nil { + t.Fatalf("second scan should carry previous_score and delta: %+v", artifact.RepoLegibility) + } + if *artifact.RepoLegibility.PreviousScore != artifact.RepoLegibility.Score || *artifact.RepoLegibility.Delta != 0 { + t.Fatalf("unchanged repo should have zero delta: %+v", artifact.RepoLegibility) + } + + path := codeguard.LegibilityHistoryPath(cfg) + if !strings.HasSuffix(path, ".legibility-history.json") { + t.Fatalf("unexpected history path: %q", path) + } + history := codeguard.LoadLegibilityHistory(path) + entries := history[artifact.ID] + if len(entries) != 2 { + t.Fatalf("history entries = %d, want 2 (keys: %v)", len(entries), historyKeys(history)) + } + if entries[0].Score != artifact.RepoLegibility.Score || len(entries[0].Components) == 0 { + t.Fatalf("history entry should carry score and components: %+v", entries[0]) + } +} + +func TestLegibilityHistoryDisabledByToggle(t *testing.T) { + dir := t.TempDir() + repo := filepath.Join(dir, "repo") + if err := os.MkdirAll(repo, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + writeLegibleRepoFixture(t, repo) + cfg := legibilityHistoryConfig(t, repo, dir, "legibility-history-off") + off := false + cfg.Checks.ContextRules.LegibilityHistory = &off + + if _, err := codeguard.Run(context.Background(), cfg); err != nil { + t.Fatalf("run: %v", err) + } + if _, err := os.Stat(codeguard.LegibilityHistoryPath(cfg)); !os.IsNotExist(err) { + t.Fatalf("history file should not exist when legibility_history=false, stat err: %v", err) + } +} + +func historyKeys(history map[string][]codeguard.LegibilityHistoryEntry) []string { + keys := make([]string, 0, len(history)) + for key := range history { + keys = append(keys, key) + } + return keys +} diff --git a/tests/checks/agentcontext_readiness_test.go b/tests/checks/agentcontext_readiness_test.go new file mode 100644 index 0000000..a57ec43 --- /dev/null +++ b/tests/checks/agentcontext_readiness_test.go @@ -0,0 +1,248 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestAgentContextFlagsUndocumentedCommands(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + // High-signal targets: build, test, fmt. internal-sync is not high-signal. + writeFile(t, filepath.Join(dir, "Makefile"), + ".PHONY: build test fmt internal-sync\nbuild:\n\techo build\ntest:\n\techo test\nfmt:\n\techo fmt\ninternal-sync:\n\techo sync\n") + // High-signal scripts: lint, dev. + writeFile(t, filepath.Join(dir, "package.json"), `{"name":"fixture","scripts":{"lint":"echo lint","dev":"echo dev"}}`) + // make build documented via inline code, make test via plain prose (no + // backticks), npm run lint via a README shell fence. make fmt and the dev + // script are documented nowhere. + writeFile(t, filepath.Join(dir, "CLAUDE.md"), strings.Join([]string{ + "# CLAUDE.md", + "", + "Build with `make build`.", + "Before pushing, run make test to execute the suite.", + }, "\n")+"\n") + writeFile(t, filepath.Join(dir, "README.md"), strings.Join([]string{ + "# fixture", + "", + "```bash", + "npm run lint", + "```", + }, "\n")+"\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "undocumented-commands")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "warn") + messages := agentContextRuleMessages(report, "context.undocumented-commands") + if len(messages) != 2 { + t.Fatalf("undocumented-commands findings = %d, want 2: %v", len(messages), messages) + } + joined := strings.Join(messages, "\n") + if !strings.Contains(joined, `"make fmt"`) || !strings.Contains(joined, `"npm run dev"`) { + t.Fatalf("unexpected undocumented-commands messages: %v", messages) + } + if strings.Contains(joined, "internal-sync") { + t.Fatalf("non-high-signal target must not demand documentation: %v", messages) + } +} + +func TestAgentContextUndocumentedCommandsExactNameMatch(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "Makefile"), "fmt:\n\techo fmt\nfmt-check:\n\techo fmt-check\n") + // The doc mentions make fmt-check, which must NOT count as a mention of + // make fmt; fmt-check itself is not on the high-signal allowlist. + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "# CLAUDE.md\n\nRun `make fmt-check` in CI.\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "undocumented-exact-match")) + if err != nil { + t.Fatalf("run: %v", err) + } + + messages := agentContextRuleMessages(report, "context.undocumented-commands") + if len(messages) != 1 || !strings.Contains(messages[0], `"make fmt"`) { + t.Fatalf("undocumented-commands findings = %v, want exactly the make fmt finding", messages) + } +} + +func TestAgentContextUndocumentedCommandsSilentWithoutAgentDocs(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "Makefile"), "build:\n\techo build\n") + writeFile(t, filepath.Join(dir, "README.md"), "# fixture\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "undocumented-no-agent-docs")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Agent Context", "context.agent-docs-missing") + assertFindingRuleAbsent(t, report, "Agent Context", "context.undocumented-commands") +} + +func TestAgentContextUndocumentedCommandsToggleOff(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "Makefile"), "build:\n\techo build\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "# CLAUDE.md\n") + + off := false + cfg := agentContextTestConfig(dir, "undocumented-toggle-off") + cfg.Checks.ContextRules.DetectUndocumentedCommands = &off + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Agent Context", "context.undocumented-commands") +} + +func TestAgentContextFlagsOversizedAgentDoc(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), markdownFileWithLines(40)) + writeFile(t, filepath.Join(dir, "AGENTS.md"), markdownFileWithLines(10)) + // A long README is reference material, not an agent doc; it must not be + // measured against the agent-doc budget. + writeFile(t, filepath.Join(dir, "README.md"), markdownFileWithLines(80)) + + cfg := agentContextTestConfig(dir, "oversized-agent-doc") + cfg.Checks.ContextRules.MaxAgentDocLines = 30 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "warn") + messages := agentContextRuleMessages(report, "context.oversized-agent-doc") + if len(messages) != 1 { + t.Fatalf("oversized-agent-doc findings = %d, want 1 (only CLAUDE.md is over budget): %v", len(messages), messages) + } + if !strings.Contains(messages[0], "30-line agent doc budget") { + t.Fatalf("unexpected oversized-agent-doc message: %q", messages[0]) + } +} + +func TestAgentContextOversizedAgentDocToggleOff(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), markdownFileWithLines(40)) + writeFile(t, filepath.Join(dir, "README.md"), "# fixture\n") + + off := false + cfg := agentContextTestConfig(dir, "oversized-agent-doc-off") + cfg.Checks.ContextRules.MaxAgentDocLines = 30 + cfg.Checks.ContextRules.DetectOversizedAgentDocs = &off + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "pass") + assertFindingRuleAbsent(t, report, "Agent Context", "context.oversized-agent-doc") +} + +func markdownFileWithLines(lines int) string { + var b strings.Builder + b.WriteString("# doc\n") + for i := 0; i < lines; i++ { + b.WriteString("Plain filler prose with no repository references at all.\n") + } + return b.String() +} + +func TestAgentContextFlagsDocLinkRot(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "docs", "real.md"), "# real\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), strings.Join([]string{ + "# CLAUDE.md", + "", + "- [existing doc](docs/real.md)", + "- [existing with anchor](docs/real.md#usage)", + "- [missing doc](docs/missing-guide.md)", + "- [missing with anchor](docs/gone.md#setup)", + "- [pure anchor](#conventions)", + "- [external](https://example.com/missing/page.md)", + "- [templated](docs/.md)", + "", + "```markdown", + "- [inside a fence](docs/fenced-away.md)", + "```", + }, "\n")+"\n") + writeFile(t, filepath.Join(dir, "README.md"), strings.Join([]string{ + "# fixture", + "", + "- [abs rot](/Users/nobody/code/thing.md)", + "- [abs ok](/docs/real.md)", + }, "\n")+"\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "doc-link-rot")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "warn") + messages := agentContextRuleMessages(report, "context.doc-link-rot") + if len(messages) != 3 { + t.Fatalf("doc-link-rot findings = %d, want 3: %v", len(messages), messages) + } + joined := strings.Join(messages, "\n") + for _, needle := range []string{"docs/missing-guide.md", "docs/gone.md#setup", "/Users/nobody/code/thing.md"} { + if !strings.Contains(joined, needle) { + t.Fatalf("doc-link-rot messages missing %q: %v", needle, messages) + } + } + for _, absent := range []string{"docs/real.md#usage", "#conventions", "example.com", "", "fenced-away"} { + if strings.Contains(joined, absent) { + t.Fatalf("doc-link-rot flagged an exempt link %q: %v", absent, messages) + } + } +} + +func TestAgentContextDocLinkRotResolvesRelativeToDocDirectory(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, ".github", "workflows-guide.md"), "# guide\n") + // copilot-instructions.md lives in .github/, so a sibling link must + // resolve against the doc's own directory. + writeFile(t, filepath.Join(dir, ".github", "copilot-instructions.md"), + "# instructions\n\nSee [the guide](workflows-guide.md).\n") + writeFile(t, filepath.Join(dir, "README.md"), "# fixture\n") + + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "doc-link-rot-docdir")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Agent Context", "context.doc-link-rot") +} + +func TestAgentContextDocLinkRotToggleOff(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "CLAUDE.md"), "# CLAUDE.md\n\n[gone](docs/gone.md)\n") + writeFile(t, filepath.Join(dir, "README.md"), "# fixture\n") + + off := false + cfg := agentContextTestConfig(dir, "doc-link-rot-off") + cfg.Checks.ContextRules.DetectDocLinkRot = &off + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "pass") + assertFindingRuleAbsent(t, report, "Agent Context", "context.doc-link-rot") +} diff --git a/tests/checks/agentcontext_test.go b/tests/checks/agentcontext_test.go index 382fed1..d100f9a 100644 --- a/tests/checks/agentcontext_test.go +++ b/tests/checks/agentcontext_test.go @@ -118,14 +118,15 @@ func TestAgentContextFlagsAgentDocsDrift(t *testing.T) { assertFindingRuleAbsent(t, report, "Agent Context", "context.agent-docs-missing") } -func TestAgentContextFlagsReadmeDriftOnlyInShellFences(t *testing.T) { +func TestAgentContextFlagsReadmeDriftInProseAndShellFences(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") writeFile(t, filepath.Join(dir, "Makefile"), "build:\n\techo build\n") writeFile(t, filepath.Join(dir, "README.md"), strings.Join([]string{ "# fixture", "", - "Prose mention of `./scripts/not-checked-in-prose.sh` stays inline-only.", + "Prose mention of `./scripts/not-checked-in-prose.sh` gets the same scrutiny as agent docs.", + "Entry point is `cmd/missing/main.go`.", "", "```bash", "./scripts/setup.sh", @@ -144,15 +145,17 @@ func TestAgentContextFlagsReadmeDriftOnlyInShellFences(t *testing.T) { assertSectionStatus(t, report, "Agent Context", "warn") messages := agentContextRuleMessages(report, "context.readme-drift") - if len(messages) != 2 { - t.Fatalf("readme-drift findings = %d, want 2: %v", len(messages), messages) + if len(messages) != 4 { + t.Fatalf("readme-drift findings = %d, want 4: %v", len(messages), messages) } joined := strings.Join(messages, "\n") - if !strings.Contains(joined, "./scripts/setup.sh") || !strings.Contains(joined, `make target "bootstrap"`) { - t.Fatalf("unexpected readme drift messages: %v", messages) + for _, needle := range []string{"./scripts/setup.sh", `make target "bootstrap"`, "not-checked-in-prose", "cmd/missing/main.go"} { + if !strings.Contains(joined, needle) { + t.Fatalf("readme drift messages missing %q: %v", needle, messages) + } } - if strings.Contains(joined, "not-checked-in-prose") || strings.Contains(joined, "example-output") { - t.Fatalf("readme drift flagged out-of-scope references: %v", messages) + if strings.Contains(joined, "example-output") { + t.Fatalf("readme drift flagged references inside a non-shell fence: %v", messages) } } diff --git a/tests/checks/agentcontext_threshold_test.go b/tests/checks/agentcontext_threshold_test.go new file mode 100644 index 0000000..8ab1c41 --- /dev/null +++ b/tests/checks/agentcontext_threshold_test.go @@ -0,0 +1,114 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// writeLowLegibilityFixture builds a repo that scores well below 100: no +// agent docs and a README whose only command reference is broken (agent_docs +// 0/25, readme 10/10, doc_accuracy 0/20, context_economy 25/25, navigability +// 20/20 = 55). +func writeLowLegibilityFixture(t *testing.T, dir string) { + t.Helper() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "README.md"), "# demo\n\n```bash\n./scripts/gone.sh\n```\n") +} + +func TestLegibilityThresholdWarnsWhenScoreFallsBelow(t *testing.T) { + dir := t.TempDir() + writeLowLegibilityFixture(t, dir) + + cfg := agentContextTestConfig(dir, "legibility-threshold-warn") + cfg.Checks.ContextRules.LegibilityWarnThreshold = 80 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "warn") + messages := agentContextRuleMessages(report, "context.legibility-threshold") + if len(messages) != 1 { + t.Fatalf("legibility-threshold findings = %d, want 1: %v", len(messages), messages) + } + for _, needle := range []string{"score 55", "warn threshold 80", "agent_docs 0/25", "doc_accuracy 0/20", "navigability 20/20"} { + if !strings.Contains(messages[0], needle) { + t.Fatalf("threshold message missing %q: %q", needle, messages[0]) + } + } +} + +func TestLegibilityThresholdFailsBelowFailThreshold(t *testing.T) { + dir := t.TempDir() + writeLowLegibilityFixture(t, dir) + + cfg := agentContextTestConfig(dir, "legibility-threshold-fail") + cfg.Checks.ContextRules.LegibilityWarnThreshold = 80 + cfg.Checks.ContextRules.LegibilityFailThreshold = 60 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "fail") + messages := agentContextRuleMessages(report, "context.legibility-threshold") + if len(messages) != 1 || !strings.Contains(messages[0], "fail threshold 60") { + t.Fatalf("expected one fail-level threshold finding, got: %v", messages) + } +} + +func TestLegibilityThresholdDisabledAtZero(t *testing.T) { + dir := t.TempDir() + writeLowLegibilityFixture(t, dir) + + // Thresholds default to 0: the score is published but never enforced. + report, err := codeguard.Run(context.Background(), agentContextTestConfig(dir, "legibility-threshold-off")) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Agent Context", "context.legibility-threshold") +} + +func TestLegibilityThresholdQuietWhenScoreMeetsBar(t *testing.T) { + dir := t.TempDir() + writeLegibleRepoFixture(t, dir) + + cfg := agentContextTestConfig(dir, "legibility-threshold-pass") + cfg.Checks.ContextRules.LegibilityWarnThreshold = 80 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Agent Context", "pass") + assertFindingRuleAbsent(t, report, "Agent Context", "context.legibility-threshold") +} + +func TestLegibilityThresholdConfigValidation(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc main() {}\n") + + cfg := agentContextTestConfig(dir, "legibility-threshold-invalid") + // Fail must sit at or below warn: legibility is good-high, so the fail + // bar is the lower one. + cfg.Checks.ContextRules.LegibilityWarnThreshold = 50 + cfg.Checks.ContextRules.LegibilityFailThreshold = 70 + + if _, err := codeguard.Run(context.Background(), cfg); err == nil || !strings.Contains(err.Error(), "legibility_fail_threshold") { + t.Fatalf("expected legibility_fail_threshold validation error, got: %v", err) + } + + cfg.Checks.ContextRules.LegibilityFailThreshold = 0 + cfg.Checks.ContextRules.LegibilityWarnThreshold = 101 + if _, err := codeguard.Run(context.Background(), cfg); err == nil || !strings.Contains(err.Error(), "legibility_warn_threshold") { + t.Fatalf("expected legibility_warn_threshold validation error, got: %v", err) + } +} diff --git a/tests/checks/quality_clone_differential_test.go b/tests/checks/quality_clone_differential_test.go new file mode 100644 index 0000000..be390dd --- /dev/null +++ b/tests/checks/quality_clone_differential_test.go @@ -0,0 +1,165 @@ +package checks_test + +import ( + "context" + "fmt" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// cloneDifferentialFixture is deliberately nontrivial: a three-way clone +// across files with differing identifier case (Total/total/ToTal) exercises +// case normalization, the long duplicated region exercises overlapping-window +// merging, delta.go's short duplicate stays below the threshold, and the +// unicode comment in alpha.go keeps byte-offset handling honest. +var cloneDifferentialFixture = map[string][]string{ + "alpha.go": { + "package sample", + "", + "func alphaOne(items []int) int {", + "\tTotal := 0", + "\tfor _, item := range items {", + "\t\tif item%3 == 0 {", + "\t\t\tTotal += item * 2", + "\t\t} else {", + "\t\t\tTotal -= item", + "\t\t}", + "\t}", + "\tif Total < 0 {", + "\t\tTotal = -Total", + "\t}", + "\treturn Total", + "}", + "", + "// café ünïcode ✓ comment keeps byte offsets honest", + "func alphaTwo(name string) string {", + "\tif name == \"\" {", + "\t\treturn \"unknown\"", + "\t}", + "\treturn name + \"-alpha\"", + "}", + "", + }, + "beta.go": { + "package sample", + "", + "func betaOne(items []int) int {", + "\ttotal := 0", + "\tfor _, item := range items {", + "\t\tif item%3 == 0 {", + "\t\t\ttotal += item * 2", + "\t\t} else {", + "\t\t\ttotal -= item", + "\t\t}", + "\t}", + "\tif total < 0 {", + "\t\ttotal = -total", + "\t}", + "\treturn total", + "}", + "", + }, + "gamma.go": { + "package sample", + "", + "func gammaOne(items []int) int {", + "\tToTal := 0", + "\tfor _, item := range items {", + "\t\tif item%3 == 0 {", + "\t\t\tToTal += item * 2", + "\t\t} else {", + "\t\t\tToTal -= item", + "\t\t}", + "\t}", + "\tif ToTal < 0 {", + "\t\tToTal = -ToTal", + "\t}", + "\treturn ToTal", + "}", + "", + "func gammaTwo(label string) string {", + "\tif label == \"\" {", + "\t\treturn \"unknown\"", + "\t}", + "\treturn label + \"-gamma\"", + "}", + "", + }, + "delta.go": { + "package sample", + "", + "func deltaOne(count int) int {", + "\tif count < 0 {", + "\t\tcount = -count", + "\t}", + "\treturn count", + "}", + "", + }, +} + +// cloneDifferentialExpected was captured verbatim from the pre-optimization +// clone detector (fresh FNV-1a over every token's bytes per window, lowercased +// token values) running on cloneDifferentialFixture; do not regenerate it with +// the current algorithm. +var cloneDifferentialExpected = []string{ + "alpha.go:3 duplicate normalized token sequence of 56 tokens also found in beta.go:3 (threshold 25)", + "alpha.go:3 duplicate normalized token sequence of 56 tokens also found in gamma.go:3 (threshold 25)", + "beta.go:3 duplicate normalized token sequence of 56 tokens also found in alpha.go:3 (threshold 25)", + "beta.go:3 duplicate normalized token sequence of 56 tokens also found in gamma.go:3 (threshold 25)", + "gamma.go:3 duplicate normalized token sequence of 56 tokens also found in alpha.go:3 (threshold 25)", + "gamma.go:3 duplicate normalized token sequence of 56 tokens also found in beta.go:3 (threshold 25)", +} + +// TestQualityCloneFindingsMatchPreOptimizationBaseline is a differential test +// for the clone-detector hashing rewrite (per-token hashes + rolling window +// hash): the optimized detector must reproduce the captured pre-optimization +// findings byte for byte. +func TestQualityCloneFindingsMatchPreOptimizationBaseline(t *testing.T) { + dir := t.TempDir() + for name, lines := range cloneDifferentialFixture { + writeFile(t, filepath.Join(dir, name), strings.Join(lines, "\n")) + } + + cfg := codeguard.ExampleConfig() + cfg.Name = "quality-clone-differential" + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: "go"}} + cfg.Checks.Quality = true + cfg.Checks.Design = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + cfg.Checks.QualityRules.CloneTokenThreshold = 25 + cfg.Checks.QualityRules.MaxFunctionLines = 100 + cfg.Checks.QualityRules.MaxParameters = 10 + cfg.Checks.QualityRules.MaxCyclomaticComplexity = 20 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + got := make([]string, 0) + for _, section := range report.Sections { + for _, finding := range section.Findings { + if finding.RuleID != "quality.duplicate-code" { + continue + } + got = append(got, fmt.Sprintf("%s:%d %s", finding.Path, finding.Line, finding.Message)) + } + } + sort.Strings(got) + + if len(got) != len(cloneDifferentialExpected) { + t.Fatalf("duplicate-code findings = %d, want %d:\n%s", len(got), len(cloneDifferentialExpected), strings.Join(got, "\n")) + } + for i, want := range cloneDifferentialExpected { + if got[i] != want { + t.Fatalf("finding %d = %q, want %q", i, got[i], want) + } + } +} diff --git a/tests/cli/report_legibility_test.go b/tests/cli/report_legibility_test.go new file mode 100644 index 0000000..389cf62 --- /dev/null +++ b/tests/cli/report_legibility_test.go @@ -0,0 +1,117 @@ +package cli_test + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/internal/cli" + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +// setupLegibilityHistoryReportFixture writes a small Go fixture repo with a +// CLAUDE.md, and seeds two scans so the report command has a recorded +// repo_legibility trend. It returns the config path. +func setupLegibilityHistoryReportFixture(t *testing.T, dir string) string { + t.Helper() + repo := filepath.Join(dir, "repo") + if err := os.MkdirAll(repo, 0o755); err != nil { + t.Fatalf("mkdir repo: %v", err) + } + files := map[string]string{ + "main.go": "package main\n\nfunc main() {}\n", + "README.md": "# fixture\n", + "CLAUDE.md": "# CLAUDE.md\n\nBuild with go build. Entry point is main.go.\n", + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(repo, name), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + + cachePath := filepath.Join(dir, ".codeguard", "cache.json") + configPath := filepath.Join(dir, "codeguard.json") + config := fmt.Sprintf(`{ + "name": "report-legibility-history", + "targets": [{"name": "repo", "path": %q, "language": "go"}], + "checks": {"performance": false, "quality": false, "design": false, "security": false, "prompts": false, "ci": false, "context": true}, + "output": {"format": "json"}, + "cache": {"enabled": true, "path": %q} +}`, repo, cachePath) + if err := os.WriteFile(configPath, []byte(config), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + cfg, err := codeguard.LoadConfigFile(configPath) + if err != nil { + t.Fatalf("load config: %v", err) + } + for i := 0; i < 2; i++ { + if _, err := codeguard.Run(context.Background(), cfg); err != nil { + t.Fatalf("scan %d: %v", i, err) + } + } + return configPath +} + +func TestRunReportPrintsLegibilityHistoryTrend(t *testing.T) { + configPath := setupLegibilityHistoryReportFixture(t, t.TempDir()) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := cli.Run([]string{"report", "-legibility-history", "-config", configPath}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("report exit code = %d, stderr = %s", code, stderr.String()) + } + output := stdout.String() + if !strings.Contains(output, "repo_legibility.repo") { + t.Fatalf("expected history key in output, got:\n%s", output) + } + if !strings.Contains(output, "score") || !strings.Contains(output, "(+0)") { + t.Fatalf("expected score trend with delta, got:\n%s", output) + } + if !strings.Contains(output, "agent_docs=") || !strings.Contains(output, "navigability=") { + t.Fatalf("expected component breakdown, got:\n%s", output) + } +} + +func TestRunReportHandlesMissingLegibilityHistory(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "codeguard.json") + config := fmt.Sprintf(`{ + "name": "report-legibility-empty", + "targets": [{"name": "repo", "path": %q, "language": "go"}], + "checks": {"performance": false, "quality": false, "design": false, "security": false, "prompts": false, "ci": false}, + "output": {"format": "json"}, + "cache": {"enabled": true, "path": %q} +}`, dir, filepath.Join(dir, ".codeguard", "cache.json")) + if err := os.WriteFile(configPath, []byte(config), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := cli.Run([]string{"report", "-legibility-history", "-config", configPath}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("report exit code = %d, stderr = %s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "no legibility-score history recorded") { + t.Fatalf("expected empty-history message, got: %s", stdout.String()) + } +} + +func TestRunReportRejectsLegibilityWithOtherModes(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + code := cli.Run([]string{"report", "-legibility-history", "-perf-history"}, strings.NewReader(""), &stdout, &stderr) + if code != 1 { + t.Fatalf("expected exit 1, got %d", code) + } + if !strings.Contains(stderr.String(), "only one mode flag") { + t.Fatalf("expected single-mode error, got: %s", stderr.String()) + } +} diff --git a/tests/support/count_lines_test.go b/tests/support/count_lines_test.go new file mode 100644 index 0000000..7f7739f --- /dev/null +++ b/tests/support/count_lines_test.go @@ -0,0 +1,47 @@ +package support_test + +import ( + "strings" + "testing" + + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +// TestCountLinesMatchesLegacySemantics pins CountLines to the exact semantics +// of the allocation-heavy implementation it replaced +// (len(strings.Split(strings.TrimRight(string(data), "\n"), "\n"))): every +// trailing newline is trimmed before counting and empty input is one line. +// Each case is also cross-checked against that legacy formula so the table +// itself cannot drift from the historical behavior. +func TestCountLinesMatchesLegacySemantics(t *testing.T) { + cases := []struct { + name string + data string + want int + }{ + {name: "empty", data: "", want: 1}, + {name: "single line no trailing newline", data: "a", want: 1}, + {name: "single line trailing newline", data: "a\n", want: 1}, + {name: "two lines no trailing newline", data: "a\nb", want: 2}, + {name: "two lines trailing newline", data: "a\nb\n", want: 2}, + {name: "interior blank line counts", data: "a\n\nb", want: 3}, + {name: "only one newline", data: "\n", want: 1}, + {name: "only newlines", data: "\n\n\n", want: 1}, + {name: "trailing blank lines trimmed", data: "a\n\n\n", want: 1}, + {name: "leading newline counts", data: "\na", want: 2}, + {name: "leading blank lines count", data: "\n\nx", want: 3}, + {name: "crlf keeps carriage return", data: "a\r\nb\r\n", want: 2}, + {name: "lone carriage return is not a newline", data: "a\rb", want: 1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + legacy := len(strings.Split(strings.TrimRight(tc.data, "\n"), "\n")) + if legacy != tc.want { + t.Fatalf("test table drifted from legacy semantics: legacy(%q) = %d, table wants %d", tc.data, legacy, tc.want) + } + if got := runnersupport.CountLines([]byte(tc.data)); got != tc.want { + t.Fatalf("CountLines(%q) = %d, want %d", tc.data, got, tc.want) + } + }) + } +}