Skip to content

fix(miner): wire recordOwnSubmission's write side into the real attempt pipeline - #5678

Merged
JSONbored merged 1 commit into
mainfrom
miner-record-own-submission
Jul 14, 2026
Merged

fix(miner): wire recordOwnSubmission's write side into the real attempt pipeline#5678
JSONbored merged 1 commit into
mainfrom
miner-record-own-submission

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

While researching what maintainer-only work would get AMS closer to a genuinely validated end-to-end release, I found that governor-state.js's recordOwnSubmission/listRecentOwnSubmissions store has a real schema and tested store methods, but nothing in the real pipeline ever called the write side. That means listRecentOwnSubmissions always returns empty in production, which silently makes resolveOwnRejectionHistory (#5655's own-rejection-history trigger, merged via #5657) a no-op — it passes its own unit tests because those inject fake submission data, but a real deployment never has any real history to check.

  • attempt-cli.js now calls recordOwnSubmission on every real "submitted" outcome, with:
    • a real fingerprint, computed by a new fingerprintFromChangedFiles helper (gittensory-engine's self-plagiarism.ts) over the loop's own real handoffPacket.changedFiles — sorted, deduped, comma-joined so it feeds fingerprintSimilarity's own token-set contract correctly
    • the real PR number (reusing the same parsePrNumberFromExecResult result the claim-conflict check already computes)
    • an honest absence (no record) when the handoff packet reports no changed files at all — never a fabricated placeholder fingerprint
  • Fail-open: a recordOwnSubmission failure never fails an otherwise-successful attempt, mirroring the existing attempt_outcome_summary write's own non-fatal handling.

This also unblocks two of the Governor's other documented-but-inactive defense layers (reputationHistory, selfPlagiarismCandidate/selfPlagiarismRecentSubmissions — both explicitly gated as optional/skip-when-absent in chokepoint.ts, and both currently always-absent for the same underlying reason), and a related durability gap in loop-cli.js's own in-memory convergence tracking. I've filed those as separate, appropriately-scoped follow-ups rather than bundling them here:

Test plan

  • packages/gittensory-engine node:test suite (543/543 pass) — new fingerprintFromChangedFiles coverage: sort/dedupe/order-independence/empty-input/blank-entry/Jaccard-integration
  • Root vitest mirror for the same (test/unit/self-plagiarism.test.ts)
  • test/unit/miner-attempt-cli.test.ts (57/57 pass) — records with a real fingerprint+PR number, skips on no changed files, records with a null PR number when unrecoverable, never fails the attempt on a recordOwnSubmission throw, skips on a non-submitted outcome, and exercises the real (non-DI) recordOwnSubmission default against an isolated tmp store
  • npm run test:ci green
  • npm run test:coverage (unsharded) — 100% branch coverage on every new/changed line, verified directly against lcov.info (including the ?? recordOwnSubmission fallback branch, which needed its own dedicated test)
  • npm run typecheck, npm run build:miner, npm run test:miner-pack, npm audit --audit-level=moderate all clean

…pt pipeline

governor-state.js's recordOwnSubmission/listRecentOwnSubmissions store existed
with a real schema and tested store methods, but nothing ever called the write
side -- meaning listRecentOwnSubmissions always returned empty in production,
silently making resolveOwnRejectionHistory's own-rejection-history trigger a
no-op despite passing its unit tests (which inject fake data). This records a
real fingerprint (gittensory-engine's new fingerprintFromChangedFiles, over the
loop's own real handoff-packet changed-files set) and real PR number on every
real "submitted" outcome, so the next attempt on a repo has real history to
check.

See follow-up issues #5675, #5676, and #5677 for the reputationHistory/
selfPlagiarismCandidate chokepoint wiring and loop-cli.js's convergence-history
dedup this unblocks.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
loopover-ui 1ee5f65 Commit Preview URL

Branch Preview URL
Jul 14 2026, 01:15 AM

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (b579eb6) to head (1ee5f65).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5678   +/-   ##
=======================================
  Coverage   95.04%   95.04%           
=======================================
  Files         577      577           
  Lines       45987    45998   +11     
  Branches    14708    14708           
=======================================
+ Hits        43707    43720   +13     
+ Misses       1527     1525    -2     
  Partials      753      753           
Flag Coverage Δ
shard-1 43.98% <45.45%> (+<0.01%) ⬆️
shard-2 35.75% <100.00%> (+0.06%) ⬆️
shard-3 32.38% <0.00%> (-0.01%) ⬇️
shard-4 32.56% <0.00%> (-0.01%) ⬇️
shard-5 31.77% <0.00%> (-0.01%) ⬇️
shard-6 44.42% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../gittensory-engine/src/governor/self-plagiarism.ts 100.00% <100.00%> (ø)
packages/gittensory-miner/lib/attempt-cli.js 97.68% <100.00%> (+0.68%) ⬆️

... and 1 file with indirect coverage changes

🚀 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 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-14 01:44:23 UTC

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

⏸️ Suggested Action - Manual Review

Review summary
This wires recordOwnSubmission into attempt-cli.js's real submitted-outcome path, computing an honest fingerprint from the loop's real handoffPacket.changedFiles via a new, well-tested fingerprintFromChangedFiles helper, and fails open on write errors exactly like the existing summary-event write. The change is narrowly scoped, DI-friendly (options.recordOwnSubmission override plus a real-store fallback test), and closes a genuine gap where resolveOwnRejectionHistory was silently a no-op in production. The diff traces cleanly end-to-end: import added, call site added, helper added and exported, tests cover the omitted-changed-files case, the null-PR case, and the fail-open case.

Nits — 5 non-blocking
  • The 40+ line comment block above the recordOwnSubmission call in attempt-cli.js:456-467 duplicates most of the PR description; consider trimming it to the non-obvious invariant (fail-open, honest-absence) and letting commit history carry the rest.
  • attempt-cli.js is now flagged at ~487 lines (over the repo's own 400-line file-size convention) per the external size-smell note; worth splitting the new submission-recording block into a small helper if the file keeps growing.
  • fingerprintFromChangedFiles lowercases nothing itself but fingerprintSimilarity's normalizeFingerprint does — worth a one-line comment noting case-folding happens downstream so a future reader doesn't assume this helper is case-sensitive on its own.
  • Consider extracting the recordOwnSubmission block (attempt-cli.js:460-479) into a small named function (e.g. recordSubmissionHistory) for readability given the file-size concern.
  • If repoFullName casing ever varies across callers, confirm fingerprintSimilarity's own normalization (lowercase) is the only place that matters, since fingerprintFromChangedFiles preserves original case.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 36 merged, 321 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 321 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 45 PR(s), 321 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 14, 2026
@JSONbored
JSONbored merged commit ebb540d into main Jul 14, 2026
19 checks passed
@JSONbored
JSONbored deleted the miner-record-own-submission branch July 14, 2026 02:16
JSONbored pushed a commit that referenced this pull request Jul 14, 2026
…utation throttle (#5685)

buildAttemptGovernorContext always left governor.reputationHistory undefined, so the chokepoint's
self-reputation throttle (already built + tested in the engine) never saw a real track record and
was silently skipped. Now that #5678 landed the recordOwnSubmission write path, wire the real data:

- WRITE: on a resolved terminal PR outcome (loop-cli.js's disposition poll), update governor-state.js's
  reputation history -- decided always increments, unfavorable only on a closed-without-merge
  (rejection-state-machine.js's isRejectedPr, matching #5655's own-rejection classification).
- READ: attempt-cli.js reads the per-repo loadReputationHistory and passes it into
  buildAttemptGovernorContext, which forwards it into the chokepoint context -- the same
  optional-parameter-with-honest-fallback pattern convergenceInput uses (#5654). Omitted stays an
  honest absence (the throttle stage is skipped), never a fabricated clean history.

Out of scope: reputation-throttle.ts's calculator/thresholds are unchanged.

Closes #5675
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant