Skip to content

fix(signals): classify Rust prost/tonic underscore protobuf stubs as generated#3782

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jimcody1995:fix/signals-rust-protobuf-underscore-stubs
Jul 6, 2026
Merged

fix(signals): classify Rust prost/tonic underscore protobuf stubs as generated#3782
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jimcody1995:fix/signals-rust-protobuf-underscore-stubs

Conversation

@jimcody1995

Copy link
Copy Markdown
Contributor

Summary

Extend isGeneratedFile to recognize Rust prost/tonic underscore protobuf spellings (*_pb.rs, *_grpc_pb.rs) alongside the existing .pb.rs / .grpc.pb.rs dot-infix 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 — 10124 tests pass, 94%+ coverage
  • 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:

  • None skipped.

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: prost/tonic underscore protobuf outputs now classify as generated via classifyChangedFile, so replay/snapshot diff analysis and slop signals do not treat *_pb.rs / *_grpc_pb.rs 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. Inserts Rust underscore regexes in the Perl→JS block (lines ~112–114) and tests/table entries in non-overlapping hunks vs open #3763 (Objective-C .pb.objc.* / .pb.m near lines ~93–96). Zero overlap with #3774/#3773 (check-miner-package), #3724 (local-branch Dart scoring), #3771/#3712 (visual shot), #3766 (ops-wire), #3760/#3755/#3721/#3698 (queue processors), or renovate deps PRs.

Made with Cursor

…generated

Recognize *_pb.rs and *_grpc_pb.rs alongside the existing .pb.rs matcher so
replay/snapshot diff classification treats prost/tonic underscore output as
machine-generated.

Fixes JSONbored#3010

Co-authored-by: Cursor <cursoragent@cursor.com>
@jimcody1995
jimcody1995 requested a review from JSONbored as a code owner July 6, 2026 09:12
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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 (ede8961) to head (a10585e).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3782   +/-   ##
=======================================
  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.

@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

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-06 11:31:03 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a narrow, well-scoped fix: it adds two regexes (`/_pb\.rs$/`, `/_grpc_pb\.rs$/`) to `isGeneratedFileFrom` in src/signals/path-matchers.ts so prost/tonic's underscore-style Rust protobuf stubs (`*_pb.rs`, `*_grpc_pb.rs`) classify as generated, matching the existing dot-infix `.pb.rs`/`.grpc.pb.rs` handling for other ecosystems. The change is correct and tested with both positive cases (`service_pb.rs`, `service_grpc_pb.rs`) and negative cases (`lib.rs`, `main.rs`) via `isGeneratedFile` and `classifyChangedFile`, and it links to open issue #3010 per the PR description. The one notable detail is that the second regex is redundant: any string ending in `_grpc_pb.rs` already ends in `_pb.rs`, so the first regex alone would match both cases.

Nits — 4 non-blocking
  • src/signals/path-matchers.ts: `/_grpc_pb\.rs$/` is redundant since any `*_grpc_pb.rs` path already satisfies `/_pb\.rs$/` immediately above it — harmless but dead pattern; consider dropping it or adding a comment explaining why it's kept for symmetry/readability with the other ecosystems' explicit grpc regex.
  • src/signals/path-matchers.ts:82-83: the updated comment now mentions 'the Rust plugin emits `.pb.rs`' twice in the same sentence (once from the original text, once in the new addition) — worth tightening to avoid the duplicate clause.
  • Consider consolidating the redundant `_grpc_pb.rs` regex into a single comment noting it's kept only for explicitness/symmetry with the dot-infix Rust entry, so a future reader doesn't wonder if it's load-bearing.
  • The test table entries and unit test assertions look sufficient — no further test additions needed for this scope.
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: 132 registered-repo PR(s), 89 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jimcody1995; Gittensor profile; 132 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 is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript
  • Official Gittensor activity: 132 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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit ae151e9 into JSONbored:main Jul 6, 2026
8 checks passed
JSONbored added a commit that referenced this pull request Jul 11, 2026
… targets (#3010)

Adds exportReplaySnapshot: given a repo and commit T, produces a
detached git-worktree checkout at T (never mutates the caller's own
checkout/branch) plus a context bundle (commit history up to and
including T by ancestry, tags reachable from T, and the README as it
existed at T, matched case-insensitively via git ls-tree rather than
a guessed filename list).

Fail-fast validation: every exported commit's and tag's date is
checked against T's own commit date, since ancestry-walking excludes
unreachable commits by construction but does not guarantee dates are
monotonic (a tag can be created long after the commit it points to;
committer dates aren't strictly increasing along the DAG in general).

Persistence is commit-keyed (UNIQUE on repo_full_name, commit_sha) in
the local store -- re-exporting the same (repo, T) pair returns the
cached row rather than re-running git, which is both how "byte-
reproducible" holds trivially and avoids redundant work.

Reuse: this issue's own text points at "existing discovery/analyze
git-history utilities" as the starting point. Grepped both packages
before writing this and found no such utility (opportunity-fanout.js
reads GitHub API issue updated_at, not git commit/tag history at
all). The genuinely reusable piece -- worktree-allocator.ts's
injected-exec convention and its removeWorktree -- is reused directly
via @jsonbored/gittensory-engine rather than inventing a third
"inject the git subprocess" abstraction.

Note on this issue's history: previously closed by #3782, an
unrelated PR (Rust protobuf stub classification) that cited this
issue number to satisfy the linked-issue gate without implementing
the actual feature. Reopened and implemented for real here.

Closes #3010.
JSONbored added a commit that referenced this pull request Jul 11, 2026
… targets (#3010)

Adds exportReplaySnapshot: given a repo and commit T, produces a
detached git-worktree checkout at T (never mutates the caller's own
checkout/branch) plus a context bundle (commit history up to and
including T by ancestry, tags reachable from T, and the README as it
existed at T, matched case-insensitively via git ls-tree rather than
a guessed filename list).

Fail-fast validation: every exported commit's and tag's date is
checked against T's own commit date, since ancestry-walking excludes
unreachable commits by construction but does not guarantee dates are
monotonic (a tag can be created long after the commit it points to;
committer dates aren't strictly increasing along the DAG in general).

Persistence is commit-keyed (UNIQUE on repo_full_name, commit_sha) in
the local store -- re-exporting the same (repo, T) pair returns the
cached row rather than re-running git, which is both how "byte-
reproducible" holds trivially and avoids redundant work.

Reuse: this issue's own text points at "existing discovery/analyze
git-history utilities" as the starting point. Grepped both packages
before writing this and found no such utility (opportunity-fanout.js
reads GitHub API issue updated_at, not git commit/tag history at
all). The genuinely reusable piece -- worktree-allocator.ts's
injected-exec convention and its removeWorktree -- is reused directly
via @jsonbored/gittensory-engine rather than inventing a third
"inject the git subprocess" abstraction.

Note on this issue's history: previously closed by #3782, an
unrelated PR (Rust protobuf stub classification) that cited this
issue number to satisfy the linked-issue gate without implementing
the actual feature. Reopened and implemented for real here.

Closes #3010.
JSONbored added a commit that referenced this pull request Jul 11, 2026
… targets (#3010) (#5113)

* feat(miner-governor): freeze/snapshot mechanism for historical replay targets (#3010)

Adds exportReplaySnapshot: given a repo and commit T, produces a
detached git-worktree checkout at T (never mutates the caller's own
checkout/branch) plus a context bundle (commit history up to and
including T by ancestry, tags reachable from T, and the README as it
existed at T, matched case-insensitively via git ls-tree rather than
a guessed filename list).

Fail-fast validation: every exported commit's and tag's date is
checked against T's own commit date, since ancestry-walking excludes
unreachable commits by construction but does not guarantee dates are
monotonic (a tag can be created long after the commit it points to;
committer dates aren't strictly increasing along the DAG in general).

Persistence is commit-keyed (UNIQUE on repo_full_name, commit_sha) in
the local store -- re-exporting the same (repo, T) pair returns the
cached row rather than re-running git, which is both how "byte-
reproducible" holds trivially and avoids redundant work.

Reuse: this issue's own text points at "existing discovery/analyze
git-history utilities" as the starting point. Grepped both packages
before writing this and found no such utility (opportunity-fanout.js
reads GitHub API issue updated_at, not git commit/tag history at
all). The genuinely reusable piece -- worktree-allocator.ts's
injected-exec convention and its removeWorktree -- is reused directly
via @jsonbored/gittensory-engine rather than inventing a third
"inject the git subprocess" abstraction.

Note on this issue's history: previously closed by #3782, an
unrelated PR (Rust protobuf stub classification) that cited this
issue number to satisfy the linked-issue gate without implementing
the actual feature. Reopened and implemented for real here.

Closes #3010.

* fix(miner-governor): clean up a leaked worktree on failure, exclude unverifiable lightweight tags

Two real defects found by AI review on the original PR:

- exportReplaySnapshot created the worktree, then ran 4 more git reads
  plus the freshness validation with no cleanup on failure. A thrown
  error after the worktree existed left it on disk at the deterministic
  planReplaySnapshotPath location with no store row saved, so a retry
  for the same (repo, commit) pair hit git worktree add's own "path
  already exists" refusal instead of the real underlying error,
  permanently masking it. Wrapped the post-add steps in try/catch:
  clean up (best-effort) before rethrowing the ORIGINAL error, never a
  cleanup error.

- readReachableTags used %(creatordate) from git tag --merged, but a
  lightweight tag has no tag object of its own, so %(creatordate) for
  one silently falls back to the POINTED-TO commit's date -- git has no
  record of when a lightweight tag was actually created at all. That
  meant a lightweight tag added long after T, but pointing at an
  ancestor of T, could never trigger validateSnapshotFreshness's check
  (its reported date is always <= T's by construction of --merged).
  Since this can't be verified, lightweight tags are now excluded from
  the export entirely -- %(objecttype) is "tag" only for an annotated
  tag's own tag object, "commit" for a lightweight tag's direct target.
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

1 participant