fix(review): fall back to the generic slop detail when the rationale neutralizes to empty - #1381
fix(review): fall back to the generic slop detail when the rationale neutralizes to empty#1381minion1227 wants to merge 3 commits into
Conversation
…neutralizes to empty slopFindingFromOpinion built the advisory detail with `safeRationale ?? fallback`, but toPublicSafe returns "" (not only null) when a non-empty rationale neutralizes away — a control-character-only rationale survives parseSlopOpinion's trim() yet collapses to "" in neutralizePublicMarkdown. Since `"" ?? x` keeps "", the fallback sentence was skipped and the public advisory detail began with a malformed " Observations: …". Use `||` so an empty body also takes the fallback, matching the `!safeRationale` drop-guard directly above, which already treats "" and null alike. Add a regression test covering the control-char rationale path (fails before, passes after).
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Important Gittensory found maintainer review notesPublic GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed. Readiness score: 66/100
Signal definitions
Review context
Maintainer notes
Contributor next steps
💰 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1381 +/- ##
=======================================
Coverage 95.37% 95.37%
=======================================
Files 192 192
Lines 20852 20852
Branches 7540 7540
=======================================
Hits 19887 19887
Misses 383 383
Partials 582 582
🚀 New features to boost your workflow:
|
JSONbored
left a comment
There was a problem hiding this comment.
Closing this under the same enforcement, and I'll be straight with you about why.
This account is permanently blocked from contributing to our repositories — for plagiarism. You submitted two verbatim copies of another contributor's open PRs as your own work:
- #1377 copied kiannidev's #1376 (identical source change; tests reworded to obscure the copy), filed ~90 minutes later on the same issue.
- #1378 copied kiannidev's #1373 byte-for-byte, filed ~2 hours later on the same issue.
Copying someone else's diff to claim the credit is a hard violation for us — two strikes, you're out. Our Code of Conduct now spells this out: plagiarism and reward-farming earn a permanent block from contributing, enforced across JSONbored/gittensory, JSONbored/metagraphed, and JSONbored/awesome-claude.
To be fair to the work: this one — the empty-rationale slop fallback — was a sharp, original catch, and I traced it as correct. That doesn't change the outcome. The ban is on the account, so there's no merit pass for an individual PR. Closed and labeled slop. Final.
Summary
No issue linked — this is a small, self-evident correctness fix to an existing sanitizer path, kept to one operator plus a regression test. The repo's
linkedIssuePolicyispreferred(not required) andissueDiscoveryPolicyisdiscouraged, so a direct PR with this rationale is the intended path rather than filing a discovery issue.slopFindingFromOpinion(src/services/ai-slop.ts) built the public advisory detail withconst detailBody = safeRationale ?? "An AI maintainer-assist pass flagged…".toPublicSafe(src/services/ai-review.ts) is typedstring | null, and it returns the empty string""(not onlynull) when a non-empty rationale neutralizes away: a control-character-only rationale survivesparseSlopOpinion's.trim()(JStrim()does not strip C0/C1 control chars) butneutralizePublicMarkdowncollapses it to"". Since"" ?? fallbackevaluates to"", the generic fallback sentence was skipped and the public advisorydetailbegan with a malformed" Observations: …"— a leading space and no explanatory body.if (!safeRationale && safeSignals.length === 0) return null;) already treats""andnullalike via!safeRationale; only thedetailBodyline used??, so it was the inconsistent one.||so an empty body also takes the fallback. Purely additive —safeRationaleisstring | null, whose only falsy values ("",null) should both fall back; any real rationale is truthy and still used verbatim.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥97% coverage of the lines AND branches you changed (aim for 98%+ on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
||arms), and the full suite passes with adequate per-test time.Safety
" Observations: …"fragment.)UI Evidencesection below. (N/A — backend-only, no visible change.)UI Evidence
N/A — backend-only change to the AI-slop advisory builder; no visible UI, frontend, docs, or extension change.
Notes
test/unit/ai-slop.test.ts): drivesslopFindingFromOpinionwith a control-character rationale (String.fromCharCode(1)) that neutralizes to"", plus one surviving signal, and asserts the detail contains the generic fallback sentence, has no leading space, and still includes the signal. Verified it fails on the pre-fix??(detail is" Observations: mostly reformatting.") and passes after the||fix. The pre-existingrationale: ""test continues to pass via thenullpath, confirming no regression.