Skip to content

feat(review): add REES complexity and Go/Python error-defect analyzers - #4155

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
feat/rees-static-complexity-analyzer-1477
Jul 8, 2026
Merged

feat(review): add REES complexity and Go/Python error-defect analyzers#4155
loopover-orb[bot] merged 1 commit into
mainfrom
feat/rees-static-complexity-analyzer-1477

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Adds a new complexity REES analyzer (review-enrichment/src/analyzers/complexity.ts): an approximate cyclomatic-complexity walker. For a newly-added function whose opening line is visible in the diff (named function declarations or a const/let/var-assigned arrow function — the same structural detection size-smell.ts uses for "big-function"), it counts if/for/while/case/catch/&&/||/?? token occurrences across the function's ADDED body lines and reports 1 + that count (the McCabe formula on the visible slice). This is explicitly not a whole-function true McCabe count — REES has no full-file content, only diff hunks — and is genuinely distinct from deep-nesting.ts (feat(enrichment): deep-nesting / arrow-anti-pattern heuristic analyzer #2030), which measures brace nesting depth (a readability smell its own header disclaims as a complexity metric), not decision-point density.
  • Extends the existing error-swallow analyzer (rather than adding a third analyzer file) with:
    • Go's if err != nil { ... } check (including the very common if-with-initializer form if err := f(); err != nil { ... }), reusing the exact same empty/unused-binding/return-null classification the JS/TS catch path already has, plus a panic() exclusion mirroring the existing throw exclusion.
    • A Python bare except: detector (flake8 E722-equivalent) — a new bare-except finding kind, since catching every exception (including SystemExit/KeyboardInterrupt) is a defect independent of the handler body.
  • Registers both in registry.ts/types.ts (BriefFindings.complexity, widened ErrorSwallowFinding.kind) and render.ts, exactly like every sibling analyzer (floating-promise.ts/deep-nesting.ts were the primary analogues traced). Adds complexity to both REES_ANALYZER_NAMES copies (src/review/enrichment-analyzer-names.ts and packages/gittensory-engine/src/review/enrichment-analyzer-names.ts — the latter is what the engine's .gittensory.yml review.enrichment.* parser validates against, so a repo owner can toggle complexity the same way as any other analyzer). errorSwallow already had a key, so no new entry was needed for the Go/Python extension.
  • Regenerates the generated analyzer metadata (review-enrichment/analyzer-metadata.json, apps/gittensory-ui/src/lib/rees-analyzers.ts, the .env.example generated block) via npm run rees:metadata.
  • Updates review-enrichment/Dockerfile's line-1 comment, which said "analyzers add CLI tools later (feat(enrichment): static analysis + complexity analyzer (lint/semgrep over the diff) #1477)" — this PR deliberately does not add any external linter/AST-parser/CLI tool, so the comment was stale relative to what actually shipped.
  • Unrelated drive-by fix: error-swallow.ts and its test file were already committed with CRLF line endings (confirmed via git cat-file on the pre-existing HEAD blob — not introduced by this PR). Since I materially edit both files here, git diff --check fails on every added line unless the file is normalized, so I converted both to LF to match the rest of the codebase. No other CRLF files in the repo were touched.

Why the cheap path, and why not duplicate churn-hotspot

Per the scoped-down plan for #1477: the original issue asked for real external linters (eslint/ruff/golangci-lint/semgrep) baked into the runtime image. This PR intentionally does not do that — REES stays no-checkout/no-external-process. churn-hotspot.ts (#1513, already shipped) is not precedent for a broader one-time full-file fetch here: it fetches commit history metadata, which cannot exist in a diff in any form at all, so a fetch is its only option. Complexity is partially derivable from the diff text itself (the added lines are real source), so the cheap in-hunk approximation — not a full-file fetch — is the right scope, and is documented as such in the analyzer's own header comment.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Closes #1477

Validation

  • git diff --check
  • npm run actionlint (part of the full npm run test:ci run below; no workflow files changed)
  • npm run typecheck
  • npm run test:coverage locally — full unsharded run, 1 file failed then passed in isolation (test/unit/selfhost-ai.test.ts's real-subprocess timing test — confirmed pre-existing/unrelated flake under load, not touched by this diff)
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — 17 new tests for complexity.ts (decision-point counting, function detection, per-function partitioning of sibling functions, comment-line exclusion, the "signature not in diff" scope limit, custom threshold, findings cap) and 9 new Go/bare-except cases for error-swallow.ts (empty/unused-binding/return-nil/propagated/logged/panicked, the nil-pointer-vs-err disambiguation, the if-with-initializer form, bare-except vs named-exception). Ran the full local gate via npm run test:ci (green) plus review-enrichment's own suite directly (npm run test:node, 1216/1216 passing). review-enrichment/** is not Codecov-measured (only src/**/packages/gittensory-engine/src/** are, per vitest.config.ts's coverage include), so I additionally confirmed via coverage/lcov.info that the one line I added to each of the two enrichment-analyzer-names.ts copies is 100%-line-covered by the existing tests that already import those modules.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no such changes.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no public API/OpenAPI surface changed; this is REES-internal.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no manual UI change (the generated rees-analyzers.ts docs data file is regenerated output only).
  • Visible UI changes include a UI Evidence section — N/A, no visible UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — no CHANGELOG.md edit.

Notes

  • Traced analogues before writing anything: floating-promise.ts, deep-nesting.ts, error-swallow.ts, size-smell.ts, and churn-hotspot.ts (per the issue's own guidance).

Adds two cheap, diff-hunk-only REES analyzers per the scoped-down plan for
#1477: a new complexity.ts approximates cyclomatic complexity (1 + a count of
if/for/while/case/catch/&&/||/?? tokens) for a newly-added function whose
opening line is visible in the diff, distinct from deep-nesting.ts's brace-
depth readability heuristic. error-swallow.ts is extended (rather than a new
file) to also flag Go's if-err-check swallow/discard/return-nil shapes and a
Python bare except naming no exception type, since both fit its existing
error-handling-block model. Registered in registry.ts/types.ts/render.ts and
the REES_ANALYZER_NAMES lists exactly like every sibling analyzer, gated by
the existing per-repo review.enrichment toggle. Regenerates the analyzer
metadata (UI docs page, .env.example block) and updates the Dockerfile's
now-stale line-1 comment. Also normalizes error-swallow.ts/test.ts from
pre-existing CRLF to LF line endings (unrelated to this feature, but required
to keep git diff --check green once real edits touch those files).
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 1aca8f2 Commit Preview URL

Branch Preview URL
Jul 08 2026, 08:10 AM

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.72%. Comparing base (0336d4e) to head (1aca8f2).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4155   +/-   ##
=======================================
  Coverage   93.72%   93.72%           
=======================================
  Files         384      384           
  Lines       36230    36230           
  Branches    13279    13279           
=======================================
  Hits        33955    33955           
  Misses       1618     1618           
  Partials      657      657           
Files with missing lines Coverage Δ
...ory-engine/src/review/enrichment-analyzer-names.ts 100.00% <ø> (ø)
src/review/enrichment-analyzer-names.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 8, 2026
@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-08 08:21:12 UTC

14 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR extends the existing error-swallow analyzer with Go if-err and Python bare-except detection, and adds a new complexity analyzer that approximates cyclomatic complexity over diff-visible function bodies. Wiring is complete and consistent across all six analyzer registration points (registry.ts, types.ts, render.ts, both REES_ANALYZER_NAMES copies, analyzer-metadata.json, rees-analyzers.ts) and the analyzer-registry.test.ts expected-list is updated to match. The regex-based Go if-err detection correctly disambiguates from unrelated nil-pointer checks (requires an err-like identifier) and correctly handles the if-with-initializer form via the `;\s*` alternative; test coverage for both new analyzers and the extended error-swallow paths looks thorough and exercises the real code paths rather than fabricated scenarios.

Nits — 6 non-blocking
  • review-enrichment/src/analyzers/complexity.ts and error-swallow.ts themselves trip the repo's own deep-nesting threshold (depth 5 vs. 4) per the external brief — worth a quick refactor of the nested pending/flush logic for consistency with what the analyzer polices elsewhere.
  • The magic numbers 25/2000/10 are duplicated as literals across registry.ts, rees-analyzers.ts, and analyzer-metadata.json (already true of prior analyzers, but worth a shared constant given how many copies now exist) — not introduced by this diff's pattern, just growing.
  • functionNameFromLine only recognizes `function` declarations and const/let/var arrow assignments, so a class/object method's newly-added function body won't get a complexity score — this mirrors size-smell.ts's existing scope limit and is explicitly disclosed in the header/tests, so it's a known gap rather than a new one.
  • GO_ERR_CHECK_OPEN_RE technically runs (unfiltered by extension) inside the shared detectErrorSwallow/parseCompleteCatchLine helpers even for non-Go files; in practice it can't match because JS/TS has no `nil` literal, but a comment noting the filtering happens only at scanPatchForErrorSwallow's isScannablePath layer would help future maintainers avoid assuming per-language dispatch happens deeper in the call chain.
  • Consider factoring the 25/2000/10 literals into named exported constants imported by all four registration sites, since this PR is the third analyzer to duplicate the same numbers verbatim.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #1477
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 52 registered-repo PR(s), 43 merged, 486 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 52 PR(s), 486 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Partially addressed
The PR adds a diff-only approximate cyclomatic-complexity analyzer and extends error-swallow, but explicitly declines the issue's core requirement to run real per-language tools (eslint, ruff, golangci-lint, semgrep) and to pin/bake that toolchain into the Dockerfile — the Dockerfile comment and analyzer header state analyzers are 'pure-JS diff-hunk heuristics with no external CLI tools,' directly

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 52 PR(s), 486 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit f5c5c52 into main Jul 8, 2026
12 checks passed
@loopover-orb
loopover-orb Bot deleted the feat/rees-static-complexity-analyzer-1477 branch July 8, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(enrichment): static analysis + complexity analyzer (lint/semgrep over the diff)

1 participant