feat: add comprehensive Go repository support with monorepo detection#412
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis pull request adds comprehensive Go language support to the agentready repository assessor framework. It introduces language-resolution helpers, expands six assessors with Go-specific logic (type annotations, complexity, logging, documentation, structure, testing), updates CI workflow templates, and adds extensive test coverage spanning Go monorepo layouts and tool integration patterns. ChangesGo Language Assessment Framework
Sequence DiagramsequenceDiagram
participant App as Assessor Framework
participant Base as Language Resolution
participant Assessor as Go Assessor<br/>(e.g., Types, Layout)
participant Repo as Repository<br/>Object
App->>Base: assess(repository)
Base->>Repo: query repository.languages<br/>+ repository.path
Base->>Base: _primary_language()<br/>(file counts + manifests)
Base-->>App: primary_language: "Go"
App->>Assessor: dispatch based on<br/>primary language
Assessor->>Repo: read_tree() for .go files<br/>+ go.mod locations
Assessor->>Assessor: _find_go_module_roots()<br/>(root + subdirs)
Assessor->>Assessor: analyze Go code<br/>(types, structure, docs, etc.)
Assessor->>Assessor: compute score<br/>+ evidence + remediation
Assessor-->>App: Finding(pass/fail)
App-->>App: collect findings across<br/>all assessors
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📈 Test Coverage Report
Coverage calculated from unit tests only |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agentready/assessors/base.py`:
- Around line 125-137: _find_go_module_roots currently only checks one directory
level; change it to search recursively (e.g., use rglob or equivalent) for all
go.mod files under repository.path, skip any matches that live inside vendor
directories, and collect the parent directories as module roots; also keep the
repository root if it contains go.mod and dedupe the resulting list (use
Path.parent to get roots and filter out duplicates).
- Around line 109-123: _primary_language() is non-deterministic because it
iterates a set of candidate languages; make the tie-break deterministic by
ordering contenders before checking manifests: build a sorted list of (lang,
count) from lang_counts (sort by descending count then by language name) and
iterate that list (skip top_lang), apply the existing 0.7 closeness check, then
check manifests using self._LANG_ROOT_MANIFESTS for the contender and for
top_lang in a fixed order; return the chosen language accordingly so repeated
runs yield the same result.
In `@src/agentready/assessors/code_quality.py`:
- Around line 554-575: The fallback that returns a Passing Finding when any
.golangci.* file exists is unsafe; update the loop that iterates search_dirs and
config_name to actually parse the matched config file and verify complexity
linters (gocyclo, cyclop, gocognit) are enabled—specifically, read the config
contents, check for disable/disable-all entries and the linters/linters-settings
sections to ensure those linters are not explicitly disabled (or are present in
enabled linters); only then return the Finding with measured_value
"golangci-lint configured". If parsing fails or the file explicitly disables
those linters, do not return the pass Finding (instead return no Finding or a
neutral/failed Finding); keep references to repository.path, search_dirs,
config_name and the Finding construction (attribute=self.attribute) when
implementing this change.
In `@src/agentready/assessors/documentation.py`:
- Around line 1224-1254: The exported-symbol detection and doc-comment check are
wrong: update exported_pattern to also match exported methods with receivers
(e.g., change exported_pattern to allow an optional receiver group before the
name so it matches lines like "func (s *Server) Start()") and adjust the
documentation check so it only counts a preceding comment when it actually
begins with the symbol name (use prev_line.startswith(f"// {symbol_name}")
rather than accepting any "//" line); also handle preceding block comments (/*
... */) by examining the last non-empty previous token block and ensuring its
first line starts with the symbol name before incrementing documented_exported
(references: exported_pattern, symbol_name, documented_exported, total_exported,
the loop over go_files/lines).
In `@src/agentready/assessors/structure.py`:
- Around line 124-125: The current check "if 'Go' in repository.languages"
wrongly triggers Go layout when any Go file exists; change it to only choose the
Go assessor when Go is the repository's primary language (or otherwise wins the
tiebreaker). Update the conditional around the call to
self._assess_go_layout(repository) to inspect the repository's primary-language
field or tiebreaker result (e.g., repository.primary_language or a helper that
returns the dominant language) instead of membership in repository.languages so
mixed repos aren't misclassified.
In `@src/agentready/assessors/stub_assessors.py`:
- Around line 67-71: The check for Go monorepo lockfiles only searches one level
deep (repository.path.glob("*/go.sum")), missing nested modules like
services/auth/go.sum; update the search to recursively find go.sum files (use
repository.path.rglob("go.sum") or repository.path.glob("**/go.sum")) and append
their relative paths to found_strict (keep the existing break logic if you only
need the first hit), referencing the found_strict list and the
repository.path.glob usage in stub_assessors.py.
In `@src/agentready/assessors/testing.py`:
- Around line 414-421: The current scan only looks one level deep for module
build files; update the search to recursively scan the repo for nested
build/task files by replacing the one-level glob used when appending to
files_to_check (currently repository.path.glob("*/Makefile")) with a recursive
search (repository.path.rglob or glob with "**/Makefile"), and also add
recursive patterns for other common build/task filenames (e.g.,
Taskfile.yml/Taskfile.yaml and deeper Makefiles) so files_to_check collects
matches like services/**/Makefile and **/Taskfile.{yml,yaml}; keep using the
same files_to_check list and ci_dir logic for .github workflows but apply rglob
patterns for completeness.
- Around line 369-374: The coverage detection currently only matches
"-coverprofile" and "-covermode" so plain "go test -cover" is missed; update the
regex used to set has_coverage in testing.py (the expression that assigns
has_coverage from re.search) to also recognize plain "-cover" (and variants like
"-cover=", "-cover ./...") by including "-cover" as an alternative (e.g., make
the pattern match "-cover" or "-coverprofile" or "-covermode" with appropriate
word-boundary/optional suffix handling) so the block that increments score and
appends evidence correctly flags repos that use "go test -cover".
In `@src/agentready/templates/bootstrap/go/workflows/security.yml.j2`:
- Around line 24-27: Replace the unpinned installation of govulncheck in the
workflow step that runs "go install golang.org/x/vuln/cmd/govulncheck@latest"
with a pinned version; update the install command to use a specific semantic
version (e.g., `@vX.Y.Z`) or a template variable so CI uses a fixed govulncheck
release, and ensure any documentation or template variables (the govulncheck
install line and the "Run govulncheck" step) are adjusted accordingly to avoid
using `@latest`.
In `@src/agentready/templates/bootstrap/go/workflows/tests.yml.j2`:
- Around line 32-34: The workflow currently sets golangci-lint action version to
"latest" (uses: golangci/golangci-lint-action@v6 with version: latest); change
the version key to a pinned, specific release (for example a concrete
semver/tag) so CI is deterministic, e.g. replace the version: latest entry with
a fixed version string and optionally add a comment or dependabot config to
update it periodically; keep the uses: golangci/golangci-lint-action@v6 line
as-is but ensure the version input is a specific pinned tag rather than
"latest".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 824f50a2-7a19-4949-a344-0d1311be59fe
📒 Files selected for processing (11)
src/agentready/assessors/base.pysrc/agentready/assessors/code_quality.pysrc/agentready/assessors/documentation.pysrc/agentready/assessors/structure.pysrc/agentready/assessors/stub_assessors.pysrc/agentready/assessors/testing.pysrc/agentready/data/Go.arsrcsrc/agentready/templates/bootstrap/go/workflows/security.yml.j2src/agentready/templates/bootstrap/go/workflows/tests.yml.j2tests/unit/test_assessors_go.pytests/unit/test_bootstrap_templates.py
| exported_pattern = re.compile(r"^(?:func|type|var|const)\s+([A-Z]\w*)") | ||
|
|
||
| for file_path in go_files: | ||
| full_path = repository.path / file_path | ||
| if full_path.name == "doc.go": | ||
| has_doc_go = True | ||
|
|
||
| try: | ||
| lines = full_path.read_text(encoding="utf-8").splitlines() | ||
| except (OSError, UnicodeDecodeError): | ||
| continue | ||
|
|
||
| for i, line in enumerate(lines): | ||
| match = exported_pattern.match(line.strip()) | ||
| if not match: | ||
| continue | ||
|
|
||
| total_exported += 1 | ||
| symbol_name = match.group(1) | ||
|
|
||
| # Check if previous non-empty line is a comment starting with symbol name | ||
| if i > 0: | ||
| prev_idx = i - 1 | ||
| while prev_idx >= 0 and not lines[prev_idx].strip(): | ||
| prev_idx -= 1 | ||
| if prev_idx >= 0: | ||
| prev_line = lines[prev_idx].strip() | ||
| if prev_line.startswith( | ||
| f"// {symbol_name}" | ||
| ) or prev_line.startswith("//"): | ||
| documented_exported += 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Go's documentation conventions, do exported methods require doc comments, and should doc comments on exported declarations start with the declaration name?
💡 Result:
Yes—Go’s documentation conventions say that every exported (capitalized) name should have a doc comment, and doc comments for types (and similarly for funcs/methods) should start with the declared symbol’s name. Go doc comments are defined as the comments that appear immediately before top-level package, const, func, type, and var declarations, with no intervening blank line. The documentation rules further state: “Every exported (capitalized) name should have a doc comment.”[1] It also says that “These rules all apply both to plain functions and to methods.”[1] For the “should start with the declaration name” part: the Go docs explain that, for types, “doc comments for types start with naming the declared symbol” and give examples; this same “rules all apply … to plain functions and to methods” guidance indicates the same convention applies to exported functions/methods as well.[1] So, in practice: exported methods (which are part of the doc-comment rules for funcs/methods) should have doc comments, and those doc comments should begin with the method name (the declared symbol), typically as the first sentence.
Citations:
Godoc coverage is undercounting methods and overcounting unrelated comments.
The regex pattern fails to capture exported methods because it doesn't account for receivers (e.g., func (s *Server) Start() won't match). Additionally, the documentation check accepts any preceding // line, not just those starting with the symbol name, inflating coverage. Go's official documentation requires every exported name—including methods—to have a doc comment starting with the declared symbol's name, so the assessment is both under- and over-inclusive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentready/assessors/documentation.py` around lines 1224 - 1254, The
exported-symbol detection and doc-comment check are wrong: update
exported_pattern to also match exported methods with receivers (e.g., change
exported_pattern to allow an optional receiver group before the name so it
matches lines like "func (s *Server) Start()") and adjust the documentation
check so it only counts a preceding comment when it actually begins with the
symbol name (use prev_line.startswith(f"// {symbol_name}") rather than accepting
any "//" line); also handle preceding block comments (/* ... */) by examining
the last non-empty previous token block and ensuring its first line starts with
the symbol name before incrementing documented_exported (references:
exported_pattern, symbol_name, documented_exported, total_exported, the loop
over go_files/lines).
| uses: golangci/golangci-lint-action@v6 | ||
| with: | ||
| version: latest |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the current stable golangci-lint release recommended for golangci/golangci-lint-action@v6 pinning in GitHub Actions workflows?
💡 Result:
The current stable golangci-lint release is v2.12.2, released on 2026-05-06.[1][2][3] For pinning in GitHub Actions workflows using golangci/golangci-lint-action@v6, the official documentation and examples recommend specifying a particular version like version: v1.60 or version: latest (which installs the latest stable).[4][5][6] While "latest" is shown in examples, for reproducible CI as emphasized in the official docs, pin to the current stable v2.12.2.[7] Note that v6 of the action supports golangci-lint v1 versions based on compatibility notes (e.g., v2.0.0+ works with >=v1.28.3), but given no explicit restriction to v1 and examples using high v1.x, it likely supports v2.x; however, for safety with v6 pinning, consider upgrading to v7+ if using v2.x is needed, as v7 explicitly supports v2 only.[8][9]
Citations:
- 1: https://github.com/golangci/golangci-lint/releases
- 2: https://golangci-lint.run/docs/product/changelog/
- 3: https://github.com/golangci/golangci-lint
- 4: https://github.com/golangci/golangci-lint-action/tree/v6.0.0
- 5: https://github.com/golangci/golangci-lint-action/tree/v6.5.0
- 6: https://github.com/golangci/golangci-lint-action/blob/v6.0.0/README.md
- 7: https://golangci-lint.run/docs/welcome/install/ci/
- 8: https://github.com/golangci/golangci-lint-action/releases
- 9: golangci/golangci-lint-action@v6.5.0...v7.0.0
Pin golangci-lint to a fixed version instead of latest.
Using version: latest makes generated CI nondeterministic and can cause unexpected build failures when upstream releases change behavior. Pinning to a specific version ensures reproducible builds across environments.
Proposed fix
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v6
with:
- version: latest
+ version: v1.60.3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: golangci/golangci-lint-action@v6 | |
| with: | |
| version: latest | |
| uses: golangci/golangci-lint-action@v6 | |
| with: | |
| version: v1.60.3 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentready/templates/bootstrap/go/workflows/tests.yml.j2` around lines 32
- 34, The workflow currently sets golangci-lint action version to "latest"
(uses: golangci/golangci-lint-action@v6 with version: latest); change the
version key to a pinned, specific release (for example a concrete semver/tag) so
CI is deterministic, e.g. replace the version: latest entry with a fixed version
string and optionally add a comment or dependabot config to update it
periodically; keep the uses: golangci/golangci-lint-action@v6 line as-is but
ensure the version input is a specific pinned tag rather than "latest".
|
Related: #378 |
🤖 AgentReady Code ReviewPR Status: 2 🔴 confirmed accuracy bugs, 1 🟡 code-level concern Real-World ValidationWe ran
The models-as-a-service result (58.8) closely matches the PR's claimed 59.6, validating the improvement. cli/cli and prometheus both correctly reach Silver, where Go repos previously couldn't score above their Python-assessed baseline. 🔴 Confirmed Accuracy Bugs1.
|
| Count | |
|---|---|
| Single-line godoc (correctly detected) | 120 |
| Multi-line godoc (incorrectly missed) | 91 |
| No documentation | 442 |
The assessor reports 14.8% coverage; actual coverage is 32.3%. 43% of documented symbols are silently missed. The fail status, low score, and remediation telling cli/cli maintainers to add godoc are all incorrect.
2. test_execution false negative on $(GO) test Make variable convention
Attribute: test_execution (Tier 1, 10%)
Location: src/agentready/assessors/testing.py, _read_go_build_files / _assess_go_coverage
gin-gonic/gin runs tests via make test, where the Makefile invokes $(GO) test — a standard convention for parameterizing the Go binary path. The assessor's pattern only matches the literal string go test, so has_test_cmd returns False even though tests, coverage config, and race detection are all present and accounted for.
The result: gin scores 80/100 for test execution (correctly finding test files, -coverprofile, and -race) but receives a fail status and remediation telling it to configure test execution. The score and the status contradict each other, and the remediation is misleading.
🟡 Code-Level Concern
3. _primary_language non-deterministic on exact file-count ties
Location: src/agentready/assessors/base.py, _primary_language
max(lang_counts, key=lang_counts.get) is non-deterministic when two languages share the same file count. One instance of this was already caught and corrected within this branch; a second instance survives in the same method.
Summary
The Go support is working and the real-world score improvements are genuine. Two accuracy bugs — the multi-line godoc checker and the $(GO) test pattern — produce meaningfully wrong output on well-maintained, well-documented projects. Both would affect leaderboard scores and the trust users place in the tool's assessment of their Go repos.
This review was generated by Claude Code under the supervision of Bill Murdock.
|
AgentReady Code Review — PR #412 PR Status: 5 issues found (2 Critical, 2 Major, 1 Minor) Critical Issues (Confidence >=90) - Auto-Fix Recommended
The godoc checker only inspects the last non-blank line before a symbol. Multi-line Go comments where the continuation line is adjacent to the symbol are missed:
The regex r"\binterface\s*{\s*}|\bany\b" matches any anywhere — including // fix any bugs and "any value". In typical Go codebases, any in comments inflates the type-weakness count. Fix: strip Major Issues (Confidence 80-89) - Manual Review Required
33 lines of Go non-source directory exclusions (vendor, testdata, hack, scripts, etc.) that are never loaded. _load_arsrc_file() exists but only loads Python.arsrc. Either integrate it or remove it.
CLAUDE.md documents Finding.create_pass()/Finding.create_fail() but all new Go code uses the raw constructor. Functionally correct but inconsistent with docs. Note: existing code has the same @natifridman another round of the review from the https://github.com/ambient-code/agentready/blob/main/.claude/commands/review-agentready.md skill. |
Add Go-specific assessment logic to 6 assessors that previously returned
not_applicable or used wrong language paths for Go repos. Includes monorepo
support for projects with go.mod in subdirectories (e.g., multi-service repos).
Assessors extended:
- TestExecutionAssessor: *_test.go detection, go test/coverage/race scoring
- TypeAnnotationsAssessor: auto-pass for static typing, interface{}/any penalty
- StandardLayoutAssessor: Go layout detection (go.mod, cmd/, internal/, pkg/)
- CyclomaticComplexityAssessor: gocyclo + golangci-lint cyclop detection
- StructuredLoggingAssessor: zap/logrus/zerolog/slog in go.mod and source
- InlineDocumentationAssessor: godoc comment coverage on exported symbols
Infrastructure:
- BaseAssessor._primary_language(): dispatches by file count for multi-lang repos
- BaseAssessor._find_go_module_roots(): finds go.mod at root and in subdirs
- Go.arsrc: directory exclusion list for Go projects
- Bootstrap templates updated to Go 1.23/1.24, golangci-lint-action v6, govulncheck
- go vet added to CI typecheck patterns, enhanced gitignore patterns
Tested against opendatahub-io/models-as-a-service (Go monorepo): score improved
from 40.9 to 59.6 (+18.7 points) with 6 previously-failing assessors now passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When file counts between two languages are within 30% of each other, use root-level project manifests (go.mod, pyproject.toml, package.json) as a tiebreaker to determine the primary language. This fixes repos like Go Kubernetes operators with Python SDK subdirectories, where Python may have slightly more files but Go owns the project root. Tested against opendatahub-io/kserve (524 Go vs 597 Python files): Go assessors now correctly fire instead of Python ones (+8.6 points). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes 9 of 10 CodeRabbit review findings: 1. _primary_language(): deterministic tiebreaker — only returns a winner when exactly one language has a root manifest match 2. _find_go_module_roots(): use rglob for recursive search, exclude vendor/testdata, deduplicate and sort results 3. CyclomaticComplexityAssessor: remove unsafe fallback that assumed golangci-lint default linters without parsing config 4. InlineDocumentationAssessor: fix godoc pattern to capture exported methods with receivers (func (s *Server) Start()); require doc comments to start with "// SymbolName" per Go convention 5. StandardLayoutAssessor: use _primary_language() dispatch instead of bare "Go" in repository.languages check 6. DependencyPinningAssessor: use rglob for go.sum search with vendor exclusion, find all matches (not just first) 7. TestExecutionAssessor: coverage regex now matches plain -cover flag in addition to -coverprofile/-covermode 8. TestExecutionAssessor: use _find_go_module_roots() for build file scanning instead of one-level glob 9. Bootstrap template: pin govulncheck to v1.3.0 instead of @latest Skipped ambient-code#10 (pin golangci-lint version: latest): intentional for bootstrap templates — golangci-lint-action@v6 documents this pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix multi-line godoc detection: walk back through consecutive // comment lines and check first line; support /* */ block comments - Relax godoc check to match go doc behavior: any comment immediately preceding a symbol counts as documentation (not just // SymbolName) - Strip comments AND string literals before matching \bany\b to avoid false positives in type_annotations assessor - Recognize $(GO) test and make test in CI quality gates and test execution assessors - Make _primary_language() max() deterministic with alphabetical tie-breaking - Upgrade golangci-lint-action@v6 to @v9 in bootstrap template - Remove dead Go.arsrc file (never loaded) - Add 12 new tests covering all fixes Real-world validation: gin-gonic/gin: 48.3 → 51.9 (test_execution FAIL→PASS, CI gates 75→85) cli/cli: 60.6 → 61.3 (inline_docs 14.8%→31.1%) models-as-a-service: 58.8 → 59.6 (stable, no regressions) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/agentready/templates/bootstrap/go/workflows/tests.yml.j2 (1)
34-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
version: latestremains unpinned.This reproduces the prior review finding: using
latestmakes CI nondeterministic and can break generated repositories when upstream releases change. Pin to a specific version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentready/templates/bootstrap/go/workflows/tests.yml.j2` at line 34, Replace the unpinned YAML key "version: latest" with a fixed, specific version string (for example "version: 1.2.3") or a project-configured pinned variable so the generated CI workflow is deterministic; update the templates/bootstrap/go/workflows/tests.yml.j2 occurrence of "version: latest" to the chosen pinned semver value or template variable that references a tested release.src/agentready/assessors/code_quality.py (1)
590-618:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe golangci fallback still false-passes disabled complexity linters.
This returns
passifgocyclo,cyclop, orgocognitappears anywhere in the config text, includingdisable:blocks, comments, or examples. That overstates complexity enforcement and masks missing tooling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentready/assessors/code_quality.py` around lines 590 - 618, The current fallback in code_quality.py incorrectly flags complexity linters by regex matching anywhere in config text (variables: config_path, content, has_complexity) which also matches commented or disabled entries; replace the blunt regex check with proper parsing: when config_name ends with .yaml/.yml use a YAML parser (safe_load) and when .toml use a TOML parser (toml.loads), then inspect the parsed structure for golangci-lint linter configuration keys (e.g., top-level "linters", "linters-settings", or "linters.enable"/"linters.disable", and "enable"/"disable") to determine if gocyclo/cyclop/gocognit are actually enabled (ensure they are present in enable lists or absent from disable lists), and only set has_complexity=true and return the Finding when the parsed config indicates the complexity linters are enabled; keep existing error handling around config_path.read_text and parsing failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agentready/assessors/base.py`:
- Around line 103-125: The current tie-break uses max(lang_counts, key=lambda k:
(lang_counts[k], k)) which selects the reverse-lexicographic language on equal
counts; change the logic to first compute top_count = max(lang_counts.values())
and then build the top candidates list = [lang for lang, c in
lang_counts.items() if c == top_count] and set top_lang =
sorted(top_candidates)[0] (i.e., pick the alphabetically first) so equal-count
ties choose the earliest alphabetical language; keep the subsequent close_langs,
manifest_winners, _LANG_ROOT_MANIFESTS and repository.path checks intact so the
70% manifest logic still applies.
In `@src/agentready/assessors/documentation.py`:
- Around line 1200-1214: The current logic walks past empty lines so a comment
separated by blank lines (e.g. "// comment\n\nfunc Exported()") is treated as
attached; change the behavior so a blank line immediately before the symbol
disqualifies documentation. Remove the while loop that decrements prev_idx over
empty lines and instead set prev_idx = symbol_idx - 1, immediately check if
prev_idx < 0 or if lines[prev_idx].strip() == "" then return False; otherwise
proceed to set prev_line = lines[prev_idx].strip() and keep the existing checks
for prev_line.startswith("//") and prev_line.endswith("*/") in the functions
using symbol_idx, prev_idx, lines, and prev_line.
---
Duplicate comments:
In `@src/agentready/assessors/code_quality.py`:
- Around line 590-618: The current fallback in code_quality.py incorrectly flags
complexity linters by regex matching anywhere in config text (variables:
config_path, content, has_complexity) which also matches commented or disabled
entries; replace the blunt regex check with proper parsing: when config_name
ends with .yaml/.yml use a YAML parser (safe_load) and when .toml use a TOML
parser (toml.loads), then inspect the parsed structure for golangci-lint linter
configuration keys (e.g., top-level "linters", "linters-settings", or
"linters.enable"/"linters.disable", and "enable"/"disable") to determine if
gocyclo/cyclop/gocognit are actually enabled (ensure they are present in enable
lists or absent from disable lists), and only set has_complexity=true and return
the Finding when the parsed config indicates the complexity linters are enabled;
keep existing error handling around config_path.read_text and parsing failures.
In `@src/agentready/templates/bootstrap/go/workflows/tests.yml.j2`:
- Line 34: Replace the unpinned YAML key "version: latest" with a fixed,
specific version string (for example "version: 1.2.3") or a project-configured
pinned variable so the generated CI workflow is deterministic; update the
templates/bootstrap/go/workflows/tests.yml.j2 occurrence of "version: latest" to
the chosen pinned semver value or template variable that references a tested
release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 06d4b4ec-3c6d-433d-bc09-6dbc908742ad
📒 Files selected for processing (9)
src/agentready/assessors/base.pysrc/agentready/assessors/code_quality.pysrc/agentready/assessors/documentation.pysrc/agentready/assessors/structure.pysrc/agentready/assessors/stub_assessors.pysrc/agentready/assessors/testing.pysrc/agentready/templates/bootstrap/go/workflows/security.yml.j2src/agentready/templates/bootstrap/go/workflows/tests.yml.j2tests/unit/test_assessors_go.py
| top_lang = max(lang_counts, key=lambda k: (lang_counts[k], k)) | ||
| top_count = lang_counts[top_lang] | ||
|
|
||
| if top_count == 0: | ||
| return None | ||
|
|
||
| # Check if any other candidate is close enough to contest | ||
| close_langs = { | ||
| lang for lang, count in lang_counts.items() if count >= top_count * 0.7 | ||
| } | ||
| if len(close_langs) > 1: | ||
| manifest_winners = [ | ||
| lang | ||
| for lang in sorted(close_langs) | ||
| if any( | ||
| (repository.path / m).exists() | ||
| for m in self._LANG_ROOT_MANIFESTS.get(lang, []) | ||
| ) | ||
| ] | ||
| if len(manifest_winners) == 1: | ||
| return manifest_winners[0] | ||
|
|
||
| return top_lang |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '95,125p' src/agentready/assessors/base.py
python - <<'PY'
lang_counts = {"Go": 10, "Python": 10, "TypeScript": 10}
current = max(lang_counts, key=lambda k: (lang_counts[k], k))
expected = min(lang_counts, key=lambda k: (-lang_counts[k], k))
print({"current": current, "expected": expected})
PYRepository: ambient-code/agentready
Length of output: 1110
🏁 Script executed:
# Check if there are tests for the _primary_language method
find . -name "*.py" -type f | xargs grep -l "_primary_language\|lang_counts" | head -20Repository: ambient-code/agentready
Length of output: 295
🏁 Script executed:
# Look for any documentation or comments about language precedence
rg -A5 -B5 "_primary_language|LANG_ROOT_MANIFESTS" --type pyRepository: ambient-code/agentready
Length of output: 8314
🏁 Script executed:
# Check if there are any existing tests for tie scenarios
find . -name "*test*.py" -o -name "test_*.py" | xargs grep -l "primary_language\|lang_counts" 2>/dev/nullRepository: ambient-code/agentready
Length of output: 209
🏁 Script executed:
# Check the test that uses _primary_language to understand expected behavior
sed -n '1,50p' tests/unit/test_fixers.py | head -100Repository: ambient-code/agentready
Length of output: 1454
🏁 Script executed:
# Search for any test of _primary_language itself
rg -A20 "test_generate_fix_uses_primary_language" tests/Repository: ambient-code/agentready
Length of output: 1367
🏁 Script executed:
# Check if there are any unit tests specifically for _primary_language
find . -name "*.py" -path "*/tests/*" -exec grep -l "_primary_language" {} \;Repository: ambient-code/agentready
Length of output: 95
🏁 Script executed:
# Get the full test_generate_fix_uses_primary_language test
rg -A50 "def test_generate_fix_uses_primary_language" tests/unit/test_fixers.pyRepository: ambient-code/agentready
Length of output: 1846
🏁 Script executed:
# Check if there are any unit tests for BaseAssessor._primary_language itself
rg -B5 -A30 "_primary_language" tests/ --type pyRepository: ambient-code/agentready
Length of output: 2198
🏁 Script executed:
# Check the base.py implementation more completely to understand the fallback behavior
sed -n '90,145p' src/agentready/assessors/base.pyRepository: ambient-code/agentready
Length of output: 2058
🏁 Script executed:
# Test what happens with an exact tie scenario - verify the exact behavior
python3 <<'PY'
# Simulate the scenario with exact tie and no clear manifest winner
lang_counts = {"Go": 10, "Python": 10, "TypeScript": 10}
close_langs = {lang for lang, count in lang_counts.items() if count >= 10 * 0.7}
print("Close languages (within 70%):", sorted(close_langs))
print("Current max() selection:", max(lang_counts, key=lambda k: (lang_counts[k], k)))
print("Expected first alphabetical:", min(lang_counts, key=lambda k: (-lang_counts[k], k)))
# Now show the lexicographic ordering behavior
keys = list(lang_counts.keys())
print("\nLexicographic ordering of language names:")
for lang in sorted(keys, reverse=True):
print(f" {lang}")
PYRepository: ambient-code/agentready
Length of output: 266
Exact-count ties still select the reverse alphabetical language.
On line 103, max(lang_counts, key=lambda k: (lang_counts[k], k)) uses reverse lexicographic ordering for ties. When counts are equal (e.g., Go: 10, Python: 10, TypeScript: 10), this picks TypeScript instead of Go. The 70% manifest check mitigates this in many cases, but fails when zero or multiple manifest files exist, falling back to the wrong language for assessment dispatch.
Fix
- top_lang = max(lang_counts, key=lambda k: (lang_counts[k], k))
+ top_lang = min(lang_counts, key=lambda k: (-lang_counts[k], k))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentready/assessors/base.py` around lines 103 - 125, The current
tie-break uses max(lang_counts, key=lambda k: (lang_counts[k], k)) which selects
the reverse-lexicographic language on equal counts; change the logic to first
compute top_count = max(lang_counts.values()) and then build the top candidates
list = [lang for lang, c in lang_counts.items() if c == top_count] and set
top_lang = sorted(top_candidates)[0] (i.e., pick the alphabetically first) so
equal-count ties choose the earliest alphabetical language; keep the subsequent
close_langs, manifest_winners, _LANG_ROOT_MANIFESTS and repository.path checks
intact so the 70% manifest logic still applies.
| prev_idx = symbol_idx - 1 | ||
| while prev_idx >= 0 and not lines[prev_idx].strip(): | ||
| prev_idx -= 1 | ||
| if prev_idx < 0: | ||
| return False | ||
|
|
||
| prev_line = lines[prev_idx].strip() | ||
|
|
||
| if prev_line.startswith("//"): | ||
| return True | ||
|
|
||
| if prev_line.endswith("*/"): | ||
| return True | ||
|
|
||
| return False |
There was a problem hiding this comment.
Blank lines are still being treated as part of a Go doc comment.
This walks past empty lines, so // comment\n\nfunc Exported() is counted as documented. That inflates godoc coverage even though the comment is not immediately attached to the declaration.
Suggested fix
- prev_idx = symbol_idx - 1
- while prev_idx >= 0 and not lines[prev_idx].strip():
- prev_idx -= 1
- if prev_idx < 0:
+ prev_idx = symbol_idx - 1
+ if prev_idx < 0 or not lines[prev_idx].strip():
return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentready/assessors/documentation.py` around lines 1200 - 1214, The
current logic walks past empty lines so a comment separated by blank lines (e.g.
"// comment\n\nfunc Exported()") is treated as attached; change the behavior so
a blank line immediately before the symbol disqualifies documentation. Remove
the while loop that decrements prev_idx over empty lines and instead set
prev_idx = symbol_idx - 1, immediately check if prev_idx < 0 or if
lines[prev_idx].strip() == "" then return False; otherwise proceed to set
prev_line = lines[prev_idx].strip() and keep the existing checks for
prev_line.startswith("//") and prev_line.endswith("*/") in the functions using
symbol_idx, prev_idx, lines, and prev_line.
|
Thank you, guys, for addressing issue #378. Looking forward to an improved score. |
🤖 AgentReady Code ReviewPR Status: 6 issues found (2 🔴 Critical, 4 🟡 Major) 🔴 Critical Issues (Confidence ≥90) — Assessment Accuracy1. Godoc blank-line skip contradicts Go doc conventionAttribute: 2.2 Inline Documentation (Tier 2, 3%) Issue: The docstring says "no blank line between" but the # Current (incorrect): skips blank lines
prev_idx = symbol_idx - 1
while prev_idx >= 0 and not lines[prev_idx].strip():
prev_idx -= 1Remediation: # Fix: blank line breaks the association
prev_idx = symbol_idx - 1
if prev_idx < 0 or not lines[prev_idx].strip():
return False2. golangci-lint config detection matches disabled lintersAttribute: 3.1 Cyclomatic Complexity (Tier 3, 2%) Issue: The regex This was flagged in both CodeRabbit reviews and remains unaddressed. Remediation: # Parse the config properly to check enable/disable state
# For YAML configs: check linters.enable contains the linter
# For TOML configs: check [linters] enable list🟡 Major Issues (Confidence 80-89) — Manual Review Required3. Bootstrap template uses
|
| Category | Count | Score Impact |
|---|---|---|
| 🔴 Critical (auto-fix recommended) | 2 | Inflated Go godoc + complexity scores |
| 🟡 Major (manual review) | 4 | Weight rebalancing affects all repos |
| Total | 6 |
- Auto-Fix Candidates: 2 critical issues ([P0] Create Automated Demo #1 godoc blank-line, [P0] Fix Critical Security & Logic Bugs from Code Review #2 golangci-lint config parsing) — both are assessment accuracy bugs that produce false positives
- Manual Review: 4 major issues require judgment — weight rebalancing ([P0] Report Header with Repository Metadata #4) is the highest-risk non-Go change
- Previously Addressed: 9 of 10 CodeRabbit findings from earlier reviews were fixed in commits
168ac29and72d8e81 - AgentReady Assessment: Run
agentready assess .after fixes to verify no self-score regression
🤖 Generated with Claude Code
- If this review was useful, react with 👍. Otherwise, react with 👎.
jwm4
left a comment
jwm4
left a comment
There was a problem hiding this comment.
🤖 Follow-up: Re-validation after 72d8e81
The new commit addresses every issue raised in our review (and kami619's). We re-ran against the same four repositories to verify.
| Repo | Before | After | Key changes |
|---|---|---|---|
| gin-gonic/gin | 48.3 Bronze | 52.5 Bronze | test_execution fail→pass, inline_documentation fail→pass |
| cli/cli | 60.6 Silver | 60.9 Silver | inline_documentation 14.8%→38.8% |
| prometheus/prometheus | 63.9 Silver | 65.1 Silver | type_annotations 85→95, inline_documentation 42→64 |
| models-as-a-service | 58.8 Bronze | 59.5 Bronze | stable, no regressions |
All fixes confirmed working. The inline_documentation score for cli/cli is still a fail at 38.8%, but that now reflects genuine documentation gaps in the repo rather than a measurement bug — which is the right outcome.
No new issues found. This looks ready to merge.
This review was generated by Claude Code under the supervision of Bill Murdock.
|
🎉 This PR is included in version 2.36.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 participants
Summary
not_applicableor used wrong language paths for Go reposgo.modin subdirectories (e.g., multi-service repos)Assessors extended
*_test.godetection,go test/coverage/race scoring, subdirectory Makefile scanninginterface{}/anyoveruse penaltygo.mod,cmd/,internal/,pkg/) with monorepo aggregationgocyclotool + golangci-lintcyclop/gocycloconfig detection in subdirsgo.modfilesdoc.godetectionInfrastructure
BaseAssessor._primary_language(): dispatches by file count with root manifest tiebreaker for mixed-language reposBaseAssessor._find_go_module_roots(): findsgo.modat root and in one-level subdirectoriesGo.arsrc: directory exclusion list for Go project layout detectiongovulncheckgo vetadded to CI typecheck patterns, enhanced gitignore patterns_has_golangci_lint()searches module root dirs for.golangci.toml/.golangci.jsonvariantsReal-world testing
Test plan
tests/unit/test_assessors_go.py)black,isort,ruff checkall pass🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements