Skip to content

feat(review): parse inline finding category with safe default (#2147) - #4231

Merged
JSONbored merged 2 commits into
JSONbored:mainfrom
bohdansolovie:feat/inline-finding-category-parse-2147
Jul 8, 2026
Merged

feat(review): parse inline finding category with safe default (#2147)#4231
JSONbored merged 2 commits into
JSONbored:mainfrom
bohdansolovie:feat/inline-finding-category-parse-2147

Conversation

@bohdansolovie

Copy link
Copy Markdown
Contributor

Summary

  • Extract pure inline-finding-category-parse.ts with parseInlineFindingCategory — valid enum literals pass through; unknown or absent values default to maintainability
  • Wire parser default into toInlineFindings so parsed model output always carries a fixed category field
  • Update ai-review.test.ts expectations for absent/unknown categories; add dedicated parser unit tests

Fixes #2147

Test plan

  • inline-finding-category-parse.test.ts — each valid enum literal, unknown/absent/non-string → maintainability
  • ai-review.test.tsparseModelReview keeps valid categories and defaults unknown/absent to maintainability
  • npx tsc -p tsconfig.json --noEmit passes locally
  • CI green including codecov/patch ≥99%

Made with Cursor

…red#2147)

Normalize model-emitted category values to the fixed enum in a pure parser
module so absent or unknown categories default to maintainability instead of
being dropped during parseModelReview.

Co-authored-by: Cursor <cursoragent@cursor.com>
@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

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-08 19:07:25 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/services/ai-review.ts (matched src/services/**).

Review summary
This PR adds a pure `parseInlineFindingCategory` normalizer (new file, fully shown) and wires it into `parseModelReview`'s `toInlineFindings` in ai-review.ts so every parsed inline finding always carries a `category`, defaulting unknown/absent/non-string values to `maintainability` instead of leaving the field off. The change is narrowly scoped, matches the linked issue (#2147), and the diff shows both the parser's two branches (pass-through valid enum vs. default) exercised by new unit tests and by updated `ai-review.test.ts` expectations covering valid/unknown/absent/non-string cases via realistic model JSON payloads — not fabricated states. No schema/migration is implicated since `category` is an in-memory field on `InlineFinding`.

Nits — 6 non-blocking
  • src/services/ai-review.ts: the diff removes the direct `isFindingCategory` import from this file in favor of the one re-exported via `inline-finding-category-parse.ts` — confirm no other call site in the (not fully shown) file still references `isFindingCategory` directly, since that would be a build break the truncated diff wouldn't reveal.
  • The `validate` and `validate-code` CI checks are currently FAILED on this commit — worth resolving before merge even though it's not visible as a code defect in the diff itself.
  • src/services/ai-review.ts:659 comment says rendering 'may apply classifyFindingCategory when absent on hand-built findings' — since parsed model findings are now never uncategorized, double check there's no now-redundant/conflicting fallback logic in the renderer for the parsed-finding path.
  • inline-finding-category-parse.ts: consider re-exporting `FINDING_CATEGORIES`/`isFindingCategory` semantics via a single doc comment rather than duplicating the 'never leaves a finding uncategorized' claim across both this file and the ai-review.ts comment, to avoid the two drifting out of sync later.
  • Add a one-line note in the PR description confirming `isFindingCategory` has no other remaining call sites in ai-review.ts post-diff, since that file's full content wasn't inlined for review.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2147
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: 385 registered-repo PR(s), 226 merged, 6 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bohdansolovie; Gittensor profile; 385 PR(s), 6 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Partially addressed
The PR delivers the parser/default piece well (new parseInlineFindingCategory helper, wiring into toInlineFindings, and matching tests for valid/unknown/absent cases), but the diff shows no changes to the INLINE_FINDINGS_SUFFIX prompt (ai-review.ts:497) or to composeInlineFindings (ai-review.ts:841), and there are no tests confirming category is preserved through compose, so two of the five listed

Review context
  • Author: bohdansolovie
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, C++, JavaScript
  • Official Gittensor activity: 385 PR(s), 6 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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

…bored#2147)

Align parseModelReview and runGittensoryAiReview expectations with the new
maintainability default when the model omits or mis-emits category.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JSONbored
JSONbored marked this pull request as ready for review July 8, 2026 19:04
@JSONbored
JSONbored self-requested a review as a code owner July 8, 2026 19:04
@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.76%. Comparing base (19d111a) to head (13df173).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4231   +/-   ##
=======================================
  Coverage   93.76%   93.76%           
=======================================
  Files         390      391    +1     
  Lines       36553    36554    +1     
  Branches    13389    13388    -1     
=======================================
+ Hits        34274    34275    +1     
  Misses       1623     1623           
  Partials      656      656           
Files with missing lines Coverage Δ
src/review/inline-finding-category-parse.ts 100.00% <100.00%> (ø)
src/services/ai-review.ts 96.58% <100.00%> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored
JSONbored merged commit e1bc7f7 into JSONbored:main Jul 8, 2026
8 checks passed
@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. and removed gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jul 8, 2026
@JSONbored JSONbored added gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. and removed gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 9, 2026
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. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

feat(review): add category enum to AI finding schema + parser default

2 participants