Skip to content

fix(signals): classify alternate Objective-C protobuf stubs as generated - #3763

Closed
jimcody1995 wants to merge 2 commits into
JSONbored:mainfrom
jimcody1995:fix/signals-objc-protobuf-alt-generated
Closed

fix(signals): classify alternate Objective-C protobuf stubs as generated#3763
jimcody1995 wants to merge 2 commits into
JSONbored:mainfrom
jimcody1995:fix/signals-objc-protobuf-alt-generated

Conversation

@jimcody1995

Copy link
Copy Markdown
Contributor

Summary

Extend isGeneratedFile to recognize alternate Objective-C protobuf spellings (.pb.objc.{h,m} and .pb.m) alongside the existing .pbobjc.{h,m} / .pbrpc.{h,m} matchers. Includes positive/negative isGeneratedFile / classifyChangedFile assertions and representative cases-table entries.

Fixes #3010

Scope

Validation

  • git diff --check
  • npx vitest run test/unit/path-matchers.test.ts on Node 22
  • npm run typecheck
  • npm run test:coverage (path-matchers suite + full unit coverage; 10122/10123 tests pass — check-miner-package fails on main due to nested lib/calibration/* allowlist drift, skipped in PR CI because miner paths unchanged)
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • Unit tests cover isGeneratedFile, classifyChangedFile, and the representative cases table

If any required check was skipped, explain why:

  • test:miner-pack not run in PR CI (miner filter false); local failure is upstream allowlist drift on main, unrelated to this diff.

Safety

  • No secrets, auth, or UI changes.
  • N/A for UI Evidence.

UI Evidence

N/A — signals-only change with no visible UI.

Notes

Incremental path-matcher parity for #3010: alternate Objective-C protobuf outputs now classify as generated via classifyChangedFile, so replay/snapshot diff analysis and slop signals do not treat protoc .pb.objc.* / .pb.m stubs as hand-authored source.

Does not deliver the freeze/snapshot mechanism itself — only closes a generated-classification gap in the shared path-matcher module.

Conflict avoidance: Touches only src/signals/path-matchers.ts and test/unit/path-matchers.test.ts. Zero overlap with open PRs (#3760 queue repair cap, #3759 max-findings caps, #3758 review-effort test, #3757 focused-test analyzer, #3756 miner-discovery pipeline test, #3755 stuck-CI re-review fix, #3724 Dart part scoring, #3721 automated-review skip hold, #3712 visual shot bounds, #3698 ignored-author gate).

Made with Cursor

@jimcody1995
jimcody1995 requested a review from JSONbored as a code owner July 6, 2026 07:54
@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

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-06 07:57:18 UTC

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

Review summary
This PR adds two regexes to isGeneratedFileFrom (src/signals/path-matchers.ts:95-96) to recognize the alternate `.pb.objc.{h,m}` and `.pb.m` Objective-C protobuf spellings alongside the existing `.pbobjc.{h,m}`/`.pbrpc.{h,m}` matchers, with corresponding positive/negative unit tests and cases-table entries. The change is small, correct, and well-covered — negative tests for plain `.m`/`.h` confirm the new patterns don't over-match hand-authored Objective-C source. One minor note: the `/i` flag on the `.pb.objc` regex is redundant since `norm` is already lowercased by `normalize()`, and the PR's linked issue (#3010, miner freeze/snapshot mechanism) is a much larger feature that this incremental path-matcher fix does not deliver — the author is transparent about this but it's worth confirming the maintainers consider this sufficient issue linkage.

Nits — 5 non-blocking
  • src/signals/path-matchers.ts:95 — the `/i` case-insensitive flag on `/\.pb\.objc\.(h|m)$/i` is redundant since `norm` is already lowercased by `normalize()`/`normalizeForMatch()`; drop it for consistency with the adjacent `.pb.m` and `.pbobjc` regexes which don't use `/i`.
  • The two new alternations (`.pb.objc.{h,m}` and `.pb.m`) could be merged into a single regex like `/\.pb\.(?:objc\.)?m$/` alongside the h-only case, though the current two-line form is arguably more readable and matches the file's existing style of one regex per spelling variant.
  • Issue feat(miner): freeze/snapshot mechanism for historical replay targets #3010 is titled around a much larger freeze/snapshot mechanism; this PR only closes a narrow generated-file classification gap and explicitly disclaims delivering the feature — worth confirming this counts as adequate issue linkage under the repo's policy rather than partial/tangential coverage.
  • Consider consolidating the Objective-C protobuf regexes into one pattern for readability, per the nit above.
  • If more alternate protoc spellings surface later (e.g. other language plugins with non-standard layouts), consider a shared helper/table rather than continuing to append one-off regex lines, to keep isGeneratedFileFrom's growing regex list maintainable.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3010
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: 127 registered-repo PR(s), 88 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jimcody1995; Gittensor profile; 127 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: jimcody1995
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: TypeScript, JavaScript
  • Official Gittensor activity: 127 PR(s), 0 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

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.29%. Comparing base (2302554) to head (f146d16).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3763   +/-   ##
=======================================
  Coverage   93.29%   93.29%           
=======================================
  Files         317      317           
  Lines       32368    32368           
  Branches    11872    11872           
=======================================
  Hits        30199    30199           
  Misses       1537     1537           
  Partials      632      632           
Files with missing lines Coverage Δ
src/signals/path-matchers.ts 89.09% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jimcody1995 and others added 2 commits July 6, 2026 10:51
Recognize `.pb.objc.{h,m}` and `.pb.m` protoc spellings alongside existing
`.pbobjc.{h,m}` matchers so replay/snapshot diff classification treats them
as machine-generated output.

Fixes JSONbored#3010

Co-authored-by: Cursor <cursoragent@cursor.com>
norm is already lowercased; match adjacent regex style.

Co-authored-by: Cursor <cursoragent@cursor.com>

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please complete the entire issue, not a small slice. Closing.

@JSONbored JSONbored closed this Jul 6, 2026
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.

feat(miner): freeze/snapshot mechanism for historical replay targets

2 participants