Skip to content

refactor(review-enrichment): dedupe REES analyzer limit constants - #4164

Merged
JSONbored merged 1 commit into
mainfrom
refactor/rees-analyzer-shared-limits-4155-nits
Jul 8, 2026
Merged

refactor(review-enrichment): dedupe REES analyzer limit constants#4164
JSONbored merged 1 commit into
mainfrom
refactor/rees-analyzer-shared-limits-4155-nits

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • The gittensory-orb review on feat(review): add REES complexity and Go/Python error-defect analyzers #4155 (feat(enrichment): static analysis + complexity analyzer (lint/semgrep over the diff) #1477, merged) flagged as non-blocking nits: "The magic numbers 25/2000/10 are duplicated as literals across registry.ts, rees-analyzers.ts, and analyzer-metadata.json ... worth a shared constant given how many copies now exist" and "this PR is the third analyzer to duplicate the same numbers verbatim." Tracing the codebase found it's much broader than "third" — 28 analyzer files repeat const MAX_FINDINGS = 25; verbatim and 18 repeat const MAX_LINE_CHARS = 2000;, plus registry.ts's descriptors repeat the same two literals 28 and 18 times respectively.
  • Added review-enrichment/src/analyzers/limits.ts exporting DEFAULT_MAX_FINDINGS = 25 / DEFAULT_MAX_LINE_CHARS = 2000. Every file with the plain default now does const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; (imported) instead of redeclaring the literal — zero usage-site churn, since the local identifier name is unchanged. Analyzer-specific overrides (asset-weight's 50, heavy-dependency's 15, migration-safety/loose-range's 20, undocumented-export/provenance's 30) are deliberately untouched — those are real per-analyzer tuning, not a duplicate worth centralizing.
  • registry.ts's 50 descriptor limits: {...} objects: replaced the exact maxFindings: 25 (28×) and maxLineChars: 2000 (18×) occurrences with the imported constants, via precise word-boundary matching that never touched a genuinely different value (15/20/30/50).
  • maxDepth: 4 and maxComplexity: 10 in registry.ts were also duplicating a literal that each analyzer already exports as its own named constant (DEFAULT_MAX_DEPTH in deep-nesting.ts, DEFAULT_MAX_COMPLEXITY in complexity.ts) — imported those instead of repeating the numbers a second time.
  • Regenerated review-enrichment/analyzer-metadata.json + apps/gittensory-ui/src/lib/rees-analyzers.ts + the .env.example generated block via npm run rees:metadata — confirmed byte-identical (only the source's use of named constants changed, not the underlying values, so nothing downstream drifted).

Separately, the same review noted complexity.ts and error-swallow.ts "themselves trip the repo's own deep-nesting threshold (depth 5 vs. 4)." Fixed both:

  • complexity.ts: extracted the innermost if (pending) {...} else {...} block (continue-tracking vs. start-tracking a function) into a new top-level advancePendingFunction helper, called from a single flat if in the scan loop instead of a nested if/else.
  • error-swallow.ts: same shape, split into two helpers (advancePendingCatch for the continue-tracking branch, tryStartPendingCatch for the immediate-finding-or-start-tracking branch), both returning a uniform { pending, finding, findingLine } result so the scan loop's own body collapses to a ternary + one flat if.
  • Verified empirically, not just by inspection: fed each file's actual diff (pre- and post-refactor) through the sibling deep-nesting.ts analyzer's own exported scanPatchForDeepNesting function. The original diff (from merged PR feat(review): add REES complexity and Go/Python error-defect analyzers #4155) reports {"depth":5,"threshold":4} for both files — reproducing the exact finding the review described. The refactored diff reports zero findings for both.
  • Both extractions are pure refactors with no behavior change — verified by running the full pre-existing test suite (unchanged assertions) before and after: 45/45 tests across complexity.test.ts/error-swallow.test.ts/deep-nesting.test.ts pass identically.

Closes #4163.

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 (Closes #4163).

Validation

  • git diff --check
  • npm run rees:test (the exact root-level command CI runs: npm ci --prefix review-enrichment + npm --prefix review-enrichment test, which itself chains build + sourcemap validation + generate-analyzer-metadata.mjs --check + the full node test suite) — 1216/1216 tests passing, metadata check clean (no drift after regeneration).
  • npm --prefix review-enrichment run build (tsc -p tsconfig.json) — clean, both before and after the rebase onto latest main.
  • Empirical nesting-depth verification (see Summary above) — both files' diffs confirmed to trip depth 5 before this change and produce zero findings after, using the repo's own scanPatchForDeepNesting.
  • Not Codecov-measured: review-enrichment/** is outside src/**/packages/**, so no codecov/patch obligation — confirmed via .claude/skills/contributing-to-gittensory/reference.md.
  • npm audit --audit-level=moderate — 0 vulnerabilities (review-enrichment's own npm ci reported none; no new dependency was added anywhere in this PR).

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 — review-enrichment is a standalone service with its own test suite; no src/api or MCP surface touched.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — apps/gittensory-ui/src/lib/rees-analyzers.ts is a generated data file, regenerated byte-identical, not hand-edited.)
  • Visible UI changes include a UI Evidence section below with screenshots. (N/A — no visible UI surface changed.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

Not applicable — this PR only touches review-enrichment/src/analyzers/** plus the three files its own metadata generator regenerates (confirmed byte-identical).

Factor the 25/2000 default findings-cap and line-length limits, repeated
verbatim across 28 and 18 analyzer files respectively plus their registry.ts
descriptors, into a shared review-enrichment/src/analyzers/limits.ts module.
Analyzer-specific overrides (asset-weight's 50, heavy-dependency's 15, etc.)
are left untouched. Also import complexity.ts's and deep-nesting.ts's own
already-exported maxComplexity/maxDepth constants into registry.ts instead of
repeating those two literals too.

Extract complexity.ts's and error-swallow.ts's innermost pending/flush
conditional into named helper functions -- both files' own diffs tripped the
repo's own deep-nesting analyzer at depth 5 vs. the policed threshold of 4,
confirmed via scanPatchForDeepNesting before and after this change.

Regenerated analyzer-metadata.json / rees-analyzers.ts / .env.example via
npm run rees:metadata (byte-identical -- only the source's use of named
constants changed, not the underlying values).

Closes #4163.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 8, 2026
@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Important

🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪

🔍 Gittensory is reviewing…

AI analysis is in progress. This comment will update when the review is complete.

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

@JSONbored JSONbored self-assigned this Jul 8, 2026
@JSONbored
JSONbored merged commit b453658 into main Jul 8, 2026
8 checks passed
@JSONbored
JSONbored deleted the refactor/rees-analyzer-shared-limits-4155-nits branch July 8, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

refactor(review-enrichment): dedupe REES analyzer limit magic numbers into shared constants

1 participant