Skip to content

feat(enrichment): add focused-test (.only) analyzer - #3757

Closed
davion-knight wants to merge 1 commit into
JSONbored:mainfrom
davion-knight:feat/focused-test-analyzer
Closed

feat(enrichment): add focused-test (.only) analyzer#3757
davion-knight wants to merge 1 commit into
JSONbored:mainfrom
davion-knight:feat/focused-test-analyzer

Conversation

@davion-knight

Copy link
Copy Markdown
Contributor

Summary

A describe.only / it.only / test.only accidentally committed to a test file silently skips every other test in that file — CI stays green while most of the suite no longer runs. It's an easily-missed footgun that no current analyzer catches: the existing debug-leftover analyzer deliberately skips test files (it targets debugger/console/print in non-test source).

This adds a new pure, local focusedTest analyzer — the test-file counterpart of debug-leftover:

  • Flags a .only chained onto a known test-block function (describe / context / suite / it / test / specify) in added lines of changed test files, line-cited via hunk headers.
  • No false positives by construction: the function-name restriction means an unrelated stream.only( never matches; string-literal content is stripped before matching (via the shared codeOnly), so a .only inside a string isn't flagged; and only isTestPath files are scanned.
  • Pure compute, no network. defaultEnabled like its sibling quality analyzers; purely additive.
  • Wired into the analyzer registry, the response type, and the render surface, with the generated analyzer metadata / UI catalog / .env.example regenerated via npm --prefix review-enrichment run metadata.

No linked issue

A self-contained new local analyzer touching only review-enrichment/ (plus the three generated catalog artifacts). It mirrors the accepted pattern and precedent of the sibling quality analyzers (debug-leftover, floating-promise, error-swallow, todo-marker, …). No external behavior changes beyond surfacing the new advisory finding.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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.
  • No linked issue — see the No linked issue section above.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage — no src/** lines changed (the analyzer lives under review-enrichment/; the only non-review-enrichment edits are the generated apps/gittensory-ui/src/lib/rees-analyzers.ts catalog and .env.example, neither measured by Codecov), so codecov/patch has no diff to gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:version-audit
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New behavior has tests — a new review-enrichment/test/focused-test.test.ts covers every branch: each test-block function, whitespace tolerance, ordinary tests / non-test .only / embedded-identifier / no-call-paren negatives, string-literal stripping, non-test-file skip, hunk line-citation across context/removed lines, the maxFindings cap, the multi-file entrypoint, and the render section (present with findings, omitted when empty). npm run rees:test passes (1194 tests; analyzer-metadata check clean); the analyzer-registry stable-order test is updated for the new descriptor.

Validated green against the full GitHub CI validate-code check set — actionlint, db:migrations:check, db:schema-drift:check, cf-typegen:check, selfhost:validate-observability, typecheck, test:coverage, test:workers, build:mcp, test:mcp-pack, build:miner, rees:test, ui:openapi:check, ui:openapi:settings-parity, ui:version-audit, ui:lint, ui:typecheck, ui:test, ui:build. Branch rebased on latest main.

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. The rendered finding is advisory, content-only (file:line), and gate-neutral.
  • No auth, cookie, CORS, GitHub App, Cloudflare, or session changes (N/A).
  • No API/OpenAPI/MCP behavior change (N/A — the generated UI analyzer catalog only gains the new entry).
  • No UI changes beyond the generated rees-analyzers.ts catalog entry (no route/component change; screenshots N/A).
  • No docs/changelog changes needed.

Notes

Modeled end-to-end on the sibling debug-leftover analyzer (#2015): same detect* + scanPatchFor* + scan* shape, same hunk line-citation, same codeOnly literal stripping, same descriptor/render/registry wiring and generated-artifact regeneration. The two are complementary — debug-leftover scans non-test source, focusedTest scans test files — and share no overlap.

A `describe.only` / `it.only` / `test.only` committed to a test file silently
skips every other test in that file, so CI stays green while most of the suite
no longer runs — an easily-missed footgun a reviewer should see.

Add a pure, local `focusedTest` analyzer: the test-file counterpart of the
debug-leftover analyzer (which scans NON-test source). It flags a `.only`
chained onto a known test-block function (`describe`/`context`/`suite`/`it`/
`test`/`specify`) in added lines of changed test files, line-cited via hunk
headers. String literals are stripped before matching, so a `.only` inside a
string is not flagged, and the function-name restriction keeps an unrelated
`stream.only(` from matching. Wired into the registry, response type, and
render surface, with regenerated analyzer metadata / UI catalog / env example.
Off nothing — additive, defaultEnabled like its siblings.
@davion-knight
davion-knight requested a review from JSONbored as a code owner July 6, 2026 07:41
@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 6, 2026
@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-06 07:45:12 UTC

9 files · 1 AI reviewer · 3 blockers · readiness 93/100 · CI green · clean

🛑 Suggested Action - Reject/Close

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
  • AI reviewers agree on a likely critical defect: The PR's own checklist admits 'No linked issue' — per repo policy every external contributor PR must close or clearly link an eligible open issue, and while the code comments reference 'epic(enrichment): REES modular analyzer runtime roadmap #1499 quality-signal family' as precedent, that is not a formal link/close and should be confirmed with the maintainer before merging as-is. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This adds a new pure, local `focusedTest` analyzer that flags `.only` chained onto describe/context/suite/it/test/specify in added lines of changed test files, mirroring the existing `debug-leftover` analyzer's structure and wired consistently through the registry, types, render surface, and generated metadata/catalog/env files. The regex correctly avoids false positives via ` ` word-boundary anchoring (verified `submit.only(` and `latest.only(` don't match) and `codeOnly` strips string-literal content before matching. The hunk-line tracking in `scanPatchForFocusedTest` correctly increments `newLine` only for added/context lines and skips removed lines, and the accompanying test file exercises whitespace tolerance, non-test-file skipping, maxFindings capping, and line citation across mixed hunks.

Blockers

  • The PR's own checklist admits 'No linked issue' — per repo policy every external contributor PR must close or clearly link an eligible open issue, and while the code comments reference 'epic(enrichment): REES modular analyzer runtime roadmap #1499 quality-signal family' as precedent, that is not a formal link/close and should be confirmed with the maintainer before merging as-is.
Nits — 5 non-blocking
  • `review-enrichment/src/analyzers/focused-test.ts` around line 55 nests to depth 5 in `scanPatchForFocusedTest` (hunk match → inHunk check → line-type branches → length check → kind check); consider extracting the added-line handling into a small helper to flatten it.
  • `MAX_FINDINGS = 25` / `MAX_LINE_CHARS = 2000` duplicate the same literals used in `debug-leftover.ts` and other sibling analyzers; consider a shared constant if this pattern keeps repeating across the quality-analyzer family.
  • The regex `\b(?:describe|context|suite|it|test|specify)\s*\.\s*only\s*\(` will also match `.only(` calls that are commented out (e.g. `// it.only(...)`), since `codeOnly` strips only string literals, not comments — worth confirming this is intentional (a commented-out focused test isn't actually skipping anything).
  • Consider having `codeOnly` (or a follow-up call) strip line comments too, so a commented-out `it.only(...)` doesn't produce a false positive — check `review-enrichment/src/analyzers/secret-log.ts` for what `codeOnly` currently does.
  • If the analyzer family under epic(enrichment): REES modular analyzer runtime roadmap #1499 has an established convention for exempting individual sibling-analyzer PRs from the issue-link requirement, document that precedent explicitly in the PR description rather than leaving the checklist item blank.

Why this is blocked

  • The PR's own checklist admits 'No linked issue' — per repo policy every external contributor PR must close or clearly link an eligible open issue, and while the code comments reference 'epic(enrichment): REES modular analyzer runtime roadmap #1499 quality-signal family' as precedent, that is not a formal link/close and should be confirmed with the maintainer before merging as-is.
  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 3 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 41 registered-repo PR(s), 28 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor davion-knight; Gittensor profile; 41 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: davion-knight
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Rust
  • Official Gittensor activity: 41 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (No linked issue detected; Maintainer requires a linked issue; AI reviewers agree on a likely critical defect: The PR's own checklist admits 'No linked issue' — per repo policy every external contributor PR must close or clearly link an eligible open issue, and while the code comments reference '#1499 quality-signal family' as precedent, that is not a formal link/close and should be confirmed with the maintainer before merging as-is.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant