Skip to content

feat(miner-manage): local pr_outcome record writer - #4389

Closed
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:feat/miner-pr-outcome-writer
Closed

feat(miner-manage): local pr_outcome record writer#4389
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:feat/miner-pr-outcome-writer

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

What

packages/gittensory-miner/lib/event-ledger.js is a generic append-only local ledger; manage-status.js shows the established pattern for a typed event layered on top of it (MANAGE_PR_UPDATE_EVENT + a normalizer + a thin writer). There was no equivalent for PR outcomes. This adds one — the miner's own local record of the outcomes of its own PRs.

New packages/gittensory-miner/lib/pr-outcome.js:

  • MINER_PR_OUTCOME_EVENT ("pr_outcome") + normalizePrOutcomePayload — a plain, JSON-round-trippable { prNumber, decision, reason, closedAt }.
  • recordPrOutcomeSnapshot(input, options) — a thin writer over the ledger with the same dependency-injection shape as recordManagePollSnapshot, so it's unit-testable without a real ledger file. A closed-not-merged PR may carry an optional reason from REJECTION_REASONS (rejection-templates.js), so this writer and the rejection-note renderer share one reason vocabulary.
  • readPrOutcomes(eventLedger, filter) — the read-side reducer (mirroring indexLatestManageUpdates) that reconstructs the current outcome per repo/PR from the append-only event stream.

This is deliberately the miner's local bookkeeping — distinct from the server-side recordPrOutcome (src/review/outcomes-wire.ts), which writes hosted-D1 ground truth from the App's webhook stream. Same concept name, different codebase layer, no shared code; a distinct constant name (MINER_PR_OUTCOME_EVENT) and a README note make that explicit.

Deliverables

  • pr-outcome.js with MINER_PR_OUTCOME_EVENT + payload normalizer, mirroring manage-status.js.
  • recordPrOutcomeSnapshot(input, options) thin writer (DI shape of recordManagePollSnapshot), reusing REJECTION_REASONS for the closed-PR reason bucket.
  • readPrOutcomes(eventLedger, filter) read-side reducer mirroring indexLatestManageUpdates.
  • Unit tests in test/unit/miner-pr-outcome.test.ts — writer (merged + closed), normalizer validation, and the read-side reduction over a multi-event history.
  • README note distinguishing this from the server-side recordPrOutcome.

Closes #4274

Add packages/gittensory-miner/lib/pr-outcome.js: a MINER_PR_OUTCOME_EVENT
constant + payload normalizer, a recordPrOutcomeSnapshot thin writer over the
local event ledger (dependency-injected, mirroring recordManagePollSnapshot),
and a readPrOutcomes read-side reducer (mirroring indexLatestManageUpdates) that
reconstructs the current outcome per repo/PR from the append-only stream. A
closed PR may carry an optional REJECTION_REASONS reason bucket. This is the
miner's own local record of its PRs' outcomes — distinct from the server-side
recordPrOutcome (src/review/outcomes-wire.ts).

Closes JSONbored#4274
@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 9, 2026 07:58
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.96%. Comparing base (ca5576f) to head (ee9c87b).
⚠️ Report is 38 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4389   +/-   ##
=======================================
  Coverage   93.96%   93.96%           
=======================================
  Files         401      401           
  Lines       36869    36869           
  Branches    13480    13480           
=======================================
  Hits        34644    34644           
  Misses       1569     1569           
  Partials      656      656           
🚀 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:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 9, 2026
@loopover-orb

loopover-orb Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-09 17:21:24 UTC

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · dirty

⏸️ Suggested Action - Manual Review

Review summary
This adds a thin, well-scoped local ledger writer for the miner's own PR outcomes, mirroring the existing manage-status.js pattern (typed event constant + normalizer + DI-shaped writer + read-side reducer). The normalizer correctly scopes `reason` to `closed` decisions with a known REJECTION_REASONS bucket, the writer validates repo/payload/ledger shape before appending, and readPrOutcomes correctly folds the append-only stream to latest-wins per repo/PR, matching indexLatestManageUpdates. Tests cover the normalizer's validation matrix, both writer paths (merged/closed), invalid-input rejection, and the read-side reduction with a re-open+merge sequence — this is solid, low-risk, additive code with no wiring into any caller yet.

Nits — 5 non-blocking
  • packages/gittensory-miner/lib/pr-outcome.js:64 — readPrOutcomes accepts a `filter` param (repo scope / `since` cursor) but no test exercises it; only the no-arg call path is covered.
  • packages/gittensory-miner/lib/pr-outcome.js:24 — an unknown `reason` string on a closed PR is silently normalized to null with no signal; worth a comment or debug log if that's expected to be rare/unexpected in practice rather than routine.
  • test/unit/miner-pr-outcome.test.ts — no test covers a non-string `closedAt` (e.g. a number or Date) being normalized to null via optionalString's type check.
  • External history notes issue feat(miner-manage): local pr_outcome record writer #4274 linkage as only 'partial' — worth confirming the PR body explicitly closes/references feat(miner-manage): local pr_outcome record writer #4274 per this repo's issue-scope requirement.
  • Add a test that passes a `filter` (e.g. `{ since }` or repo scope) through readPrOutcomes to lock in that it's forwarded correctly to eventLedger.readEvents, not just accepted and ignored.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4274
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: 96 registered-repo PR(s), 36 merged, 4 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 96 PR(s), 4 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Addressed
The PR adds pr-outcome.js with MINER_PR_OUTCOME_EVENT and a payload normalizer mirroring manage-status.js, a recordPrOutcomeSnapshot thin writer with the same DI shape as recordManagePollSnapshot that reuses REJECTION_REASONS, and a readPrOutcomes reducer mirroring indexLatestManageUpdates, plus unit tests covering merged/closed writes, normalizer validation, and multi-event reduction, and a READM

Review context
  • Author: jeffrey701
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, Clojure, JavaScript
  • Official Gittensor activity: 96 PR(s), 4 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 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (conflicts with the base branch — resolve and open a fresh PR). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(miner-manage): local pr_outcome record writer

1 participant