feat(signals): add slop path matchers and non-substantive-padding signal (#561) - #752
Conversation
|
gittensory · advisory review Reviewed 4 changed file(s) — two independent AI reviewers. Changed files: Suggested action: ✅ Safe to merge — both reviewers found no blocking issues. Reviewer A · Suggestions
Worth double-checking
Reviewer B · No blocking issues spotted. |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Note Gittensory Gate skippedPR closed before full evaluation. No late first comment was created.
💰 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. |
JSONbored
left a comment
There was a problem hiding this comment.
PR #752 — feat(signals): add slop path matchers and non-substantive-padding signal (#561)
Author: bittoby | CI: Fail
Highlights:
- Exactly a deterministic, side-effect-free signal builder (src/signals/path-matchers.ts), not noise.
- High-precision: fires only on high churn (≥40 lines) AND padding dominance (≥50%) AND negligible substantive source/test (≤15%). False-positive-averse.
- Excludes lockfiles/dependency manifests/docs so dep bumps and docs-only PRs can't trip it; no double-count with trivialWhitespaceChurn (tested).
- Public-safe: interpolates only integer counts; forbidden-language regex test passes. Coverage 98.65% stmts / 97.05% branches. Conventional title, linked #561.
Notes / before merge:
- mergeable_state: blocked = fix failing CI (test coverage below threshold).
…t/561-slop-path-matchers
…oby/gittensory into feat/561-slop-path-matchers
4486582 to
1a5fafc
Compare
1a5fafc to
91c60c3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #752 +/- ##
==========================================
+ Coverage 96.73% 96.74% +0.01%
==========================================
Files 94 95 +1
Lines 13851 13905 +54
Branches 5051 5075 +24
==========================================
+ Hits 13399 13453 +54
Misses 86 86
Partials 366 366 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@JSONbored Fixed all CI testing errors. |
JSONbored
left a comment
There was a problem hiding this comment.
PR #752 — feat(signals): add slop path matchers and non-substantive-padding signal (#561)
Verdict: MERGE
Resolves issue #561: FULLY
CI: Green (lint, test 1/2, workers, security, changes; ui in_progress, no failures). No migration/schema/OpenAPI change.
Highlights:
- Exactly the ask: pure deterministic path matchers in src/signals/path-matchers.ts (isGeneratedFile/isVendoredFile/isLockfile/isMinifiedFile/isDocsFile/isDependencyManifestFile) + single-category classifyChangedFile, siblings to isTestFile.
- New non_substantive_padding signal is false-positive-averse: requires high churn, >=50% padding dominance, and <=15% substantive before firing; lockfiles/manifests/docs recognized but never counted as padding, so dependency bumps and docs-only PRs can't trip it. Complementary to trivialWhitespaceChurn (no double-count).
- Thorough tests (categories, case/separator normalization, defensive null/undefined, precedence; fire/no-fire, threshold edges, public-safety). Finding text interpolates only integer counts.
Concerns / required changes:
- None.
Completeness vs issue: Fully resolves #561 — each matcher classifies representative paths correctly with deterministic public-safe output and >=97% coverage on new code, no scope creep or migration.
Summary
src/signals/path-matchers.ts(siblings toisTestFile):isGeneratedFile,isVendoredFile,isLockfile,isMinifiedFile,isDocsFile,isDependencyManifestFile, plus anisNonSubstantivePaddingFilecomposite and a single-categoryclassifyChangedFile.non_substantive_padding: fires when a high-churn diff is dominated by generated/vendored/minified output (files that carry code extensions and so slip pasttrivialWhitespaceChurn) while genuine source/test effort is negligible.trivialWhitespaceChurn(no double-count).Closes #561
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; global coverage stays at or above 97% for lines, statements, functions, and branches (aim for 98%+ branch coverage locally so CI variance does not fail near the threshold)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=moderatenpm run test:cipasses end to end. Coverage (src/**): statements 98.65%, branches 97.05%, functions 97.05%, lines 99.27% (all ≥ 97%).npm auditreports 0 vulnerabilities. New code is fully covered:src/signals/path-matchers.tsis 100% statements/branches; the newslop.tsbranches are covered bytest/unit/path-matchers.test.tsand the added cases intest/unit/slop.test.ts.If any required check was skipped, explain why:
npm run test:cigate was run locally and passed.Safety
detail/publicTextinterpolate only integer line counts, so the text is public-safe by construction; a unit test asserts the output contains no forbidden public terms.ui:openapi:checkclean).UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository. N/A — no visible UI, frontend, docs, or extension changes.SLOP_RUBRIC_MARKDOWNsignal list is updated to include the new signal).UI Evidence
Not applicable — this change is backend signal logic only (
src/signals/), with no visible UI, frontend, docs, or extension surface.Notes
SlopAssessmentInput.changedFiles.src/signals/path-matchers.ts(new matchers + classifier),src/signals/slop.ts(new signal + weight + rubric, wired intobuildSlopAssessment),test/unit/path-matchers.test.ts(new),test/unit/slop.test.ts(added padding-signal cases).