Skip to content

fix(rules): recognize .mts/.cts/.mjs/.cjs/.kts/.scala/.groovy in isCodePath - #9368

Closed
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:fix-9322-iscodepath-extensions
Closed

fix(rules): recognize .mts/.cts/.mjs/.cjs/.kts/.scala/.groovy in isCodePath#9368
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:fix-9322-iscodepath-extensions

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

What

isCodePath in src/rules/advisory.ts — the extension gate annotatablePullRequestFiles uses to
decide which changed files buildCheckRunAnnotations may annotate — recognized .ts/.tsx/.js/.jsx,
.kt, .java, etc., but was missing the module-variant extensions .mts/.cts/.mjs/.cjs and the JVM
extensions .kts/.scala/.groovy.

isCodeFile/SOURCE_FILE_EXTENSION (packages/loopover-engine/src/signals/test-evidence.ts:23)
already admit all seven. Because annotatablePullRequestFiles gates on isCodePath before the
isCodeFile missing-test filter runs, a PR touching only those extensions was dropped from
annotatableFiles and never received the Missing test evidence annotation the parity contract
intends.

Change

Add mts, cts, mjs, cjs, kts, scala, groovy to the isCodePath regex — purely additive.
No existing extension is removed or narrowed; the signature, callers, and the rest of
buildCheckRunAnnotations's behavior are unchanged.

Validation

  • New test in test/unit/rules.test.ts (mirrors the existing Vue/Svelte/Astro and C++ parity tests):
    a source file at each of .mts/.cts/.mjs/.cjs/.kts/.scala/.groovy, shipped without a test, is now
    flagged Missing test evidence — proving isCodePath admits it. All four isCodePath parity tests
    pass.
  • npx vitest run test/unit/rules.test.ts green; the changed line is a single regex literal executed
    by the passing tests.

Closes #9322

@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 27, 2026 14:34
@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 27, 2026
@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-27 14:43:09 UTC

2 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This is a narrow, purely additive fix to the `isCodePath` regex in `src/rules/advisory.ts` (advisory.ts:554), adding `.mts/.cts/.mjs/.cjs/.kts/.scala/.groovy` so files with these extensions aren't dropped from `annotatableFiles` before the missing-tests check runs — closing the stated parity gap with `isCodeFile`/`SOURCE_FILE_EXTENSION`. The regex alternation ordering (e.g. `kt` before `kts`, `ts` before `tsx`) is safe because JS regex backtracks on the `$` anchor, so all seven new extensions are actually reachable. The accompanying test in `test/unit/rules.test.ts` mirrors the existing Vue/Svelte/Astro/Dart parity tests and exercises the real `buildCheckRunAnnotations` path with files at each new extension, which is a genuine (not fabricated) regression test for the claimed fix.

Nits — 3 non-blocking
  • The 3-line inline comment above `isCodePath` in advisory.ts is fine but could be a single line per this file's terser one-liner comments elsewhere in the same function block.
  • The two undetailed CI failures (validate-tests, validate) can't be diagnosed from what's given, and this branch is 1 commit behind the default — worth rebasing before merge to rule out drift as the cause.
  • Rebase onto the current default branch to see if `validate`/`validate-tests` go green, since the branch is only 1 commit behind and the diff itself doesn't show an obvious cause for either failure.

CI checks failing

  • validate
  • validate-tests

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9322
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: 83 registered-repo PR(s), 38 merged, 17 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 83 PR(s), 17 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The regex in isCodePath is updated additively to include all seven missing extensions (.mts/.cts/.mjs/.cjs/.kts/.scala/.groovy) without removing existing ones, and a new regression test covering all seven extensions asserting 'Missing test evidence' annotations is added adjacent to the existing parity tests, matching the required style.

Review context
  • Author: jeffrey701
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Rust, TypeScript, Clojure, JavaScript
  • Official Gittensor activity: 83 PR(s), 17 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests)). 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.

@loopover-orb loopover-orb Bot closed this Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
11233 3 11230 16
View the top 2 failed test(s) by shortest run time
test/unit/selfhost-pg-retention.test.ts > runRetentionPrune + processJob on the Postgres backend (#977) > processJob prune-retention deletes eligible rows and records a success audit event on Postgres
Stack Traces | 0.00477s run time
AssertionError: expected 3 to be +0 // Object.is equality

- Expected
+ Received

- 0
+ 3

 ❯ test/unit/selfhost-pg-retention.test.ts:119:44
test/unit/selfhost-pg-retention.test.ts > pruneExpiredRecords on the Postgres backend (#977) > deletes across multiple bounded batches and stops at the per-table cap, same as the SQLite path
Stack Traces | 0.01s run time
AssertionError: expected +0 to be 4 // Object.is equality

- Expected
+ Received

- 4
+ 0

 ❯ test/unit/selfhost-pg-retention.test.ts:76:33
View the full list of 1 ❄️ flaky test(s)
test/unit/salvageability.test.ts > resolveAiReviewSalvageableHold > defaults: no configured floor uses the gate default, and a confidence-less blocker counts as certainty (at/above floor)

Flake rate in main: 100.00% (Passed 0 times, Failed 19 times)

Stack Traces | 0.00567s run time
AssertionError: expected undefined to be defined
 ❯ test/unit/salvageability.test.ts:88:18

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

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.

fix(rules): advisory.ts's isCodePath is missing .mts/.cts/.mjs/.cjs/.kts/.scala/.groovy that isCodeFile recognizes

1 participant