Skip to content

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

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

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

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Closes #4274

Fresh PR replacing #4389, which the orb auto-closed for a base-branch conflict ("resolve and open a fresh PR") after the sibling miner PRs #4376 and #4390 merged and advanced the shared packages/gittensory-miner/README.md. Rebased onto current main; the three code files are unchanged, only the README entry was re-anchored after the newly-merged ledger paragraphs. No conflict.

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, so a later self-improve pass can pair a prediction (the prediction-ledger, #4263) with its realized outcome. In laptop mode there may be no webhook relay at all, so this records outcomes locally rather than via the server-side hosted-D1 path.

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

  • MINER_PR_OUTCOME_EVENT ("pr_outcome") + normalizePrOutcomePayload — a plain, JSON-round-trippable { prNumber, decision, reason, closedAt }; returns null on missing/invalid required fields.
  • 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 one REJECTION_REASONS reason bucket (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.

Testing

npx vitest run test/unit/miner-pr-outcome.test.ts
npm run typecheck

5/5 tests pass; typecheck clean. (packages/gittensory-miner/lib/** carries no coverage wall.)

The miner needs its OWN local record of the outcome (merged / closed) of its
OWN PRs, so a later self-improve pass can pair a prediction with its realized
outcome. In laptop mode there may be no webhook relay at all, so this records
outcomes locally via the existing event ledger rather than the server-side
hosted-D1 path.

- packages/gittensory-miner/lib/pr-outcome.js: normalizePrOutcomePayload (a
  strict, JSON-round-trippable normalizer — a `closed` decision may carry one
  REJECTION_REASONS reason bucket, a merged/unknown reason normalizes to null),
  recordPrOutcomeSnapshot (a thin dependency-injected writer with the same shape
  as recordManagePollSnapshot, so it's unit-testable without a real ledger),
  and readPrOutcomes (reduces the append-only ledger to the latest outcome per
  repo/PR). Deliberately reuses the MINER_PR_OUTCOME_EVENT = "pr_outcome" event
  type and REJECTION_REASONS vocabulary; no shared code with the server side.
- packages/gittensory-miner/lib/pr-outcome.d.ts: hand-written types.
- packages/gittensory-miner/README.md: one-line entry for the writer.
- test/unit/miner-pr-outcome.test.ts: normalization, write, and read coverage
  against an injected fake ledger.

Reopens the change from JSONbored#4389, which the orb auto-closed for a base-branch
conflict after sibling miner PRs (JSONbored#4376/JSONbored#4390) merged and advanced the shared
README; rebased onto current main, no conflict.

Closes JSONbored#4274
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
12495 4 12491 12
View the top 3 failed test(s) by shortest run time
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > engine version skew > uses default version readers against the real monorepo workspace
Stack Traces | 0.0208s run time
AssertionError: expected [ Array(1) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   "src/review/linked-issue-label-propagation.ts and .../src/review/linked-issue-label-propagation.ts have drifted apart (normalized comparison).
+ Edit both copies together or convert the host file to a thin engine re-export shim.",
+ ]

 ❯ test/unit/check-engine-parity-script.test.ts:192:31
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > runEngineParityMain returns 0 for the real monorepo workspace
Stack Traces | 0.0361s run time
AssertionError: expected 1 to be +0 // Object.is equality

- Expected
+ Received

- 0
+ 1

 ❯ test/unit/check-engine-parity-script.test.ts:205:48
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > the real repo's hand-duplicated pairs agree after normalization (regression guard)
Stack Traces | 0.0489s run time
AssertionError: expected [ Array(1) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   "src/review/linked-issue-label-propagation.ts and .../src/review/linked-issue-label-propagation.ts have drifted apart (normalized comparison).
+ Edit both copies together or convert the host file to a thin engine re-export shim.",
+ ]

 ❯ test/unit/check-engine-parity-script.test.ts:88:29
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > prints a clean summary and exits 0 for the real repo state when run as a subprocess
Stack Traces | 0.96s run time
Error: Command failed: .../gittensory/node_modules/.bin/tsx scripts/check-engine-parity.ts
Engine-parity check found 1 issue(s):
src/review/linked-issue-label-propagation.ts and .../src/review/linked-issue-label-propagation.ts have drifted apart (normalized comparison).
Edit both copies together or convert the host file to a thin engine re-export shim.

 ❯ test/unit/check-engine-parity-script.test.ts:210:20

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { status: 1, signal: null, output: [ null, '', 'Engine-parity check found 1 issue(s):\nsrc/review/linked-issue-label-propagation.ts and .../src/review/linked-issue-label-propagation.ts have drifted apart (normalized comparison).\nEdit both copies together or convert the host file to a thin engine re-export shim.\n' ], pid: 8228, stdout: '', stderr: 'Engine-parity check found 1 issue(s):\nsrc/review/linked-issue-label-propagation.ts and .../src/review/linked-issue-label-propagation.ts have drifted apart (normalized comparison).\nEdit both copies together or convert the host file to a thin engine re-export shim.\n' }

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@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

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-09 17:59:11 UTC

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This is a small, well-scoped addition of a local (non-server) PR-outcome writer/reader for the miner package, cleanly mirroring the existing recordManagePollSnapshot/indexLatestManageUpdates pattern (dependency-injected event ledger, plain normalizer, latest-wins reducer). Validation logic (repo shape, decision enum, reason bucket scoping to REJECTION_REASONS, JSON round-trippability via the underlying ledger) is correct and traceable to the real event-ledger.js and rejection-templates.js it builds on. No schema/migration changes are needed since it reuses the generic event ledger table, tests exercise the real ledger (not a fabricated payload), and the PR closes its linked issue (#4274) with a narrowly scoped, insert-only foundational module consistent with prior similar additions (event-ledger, prediction-ledger) in this repo.

Nits — 5 non-blocking
  • packages/gittensory-miner/lib/pr-outcome.js re-implements the owner/repo split-and-validate logic that already exists as normalizeOptionalRepoFullName in event-ledger.js; consider exporting and reusing it instead of duplicating the same slash-count check.
  • test/unit/miner-pr-outcome.test.ts never exercises the two early-continue branches in readPrOutcomes (event.repoFullName missing/blank, and normalizePrOutcomePayload returning null for a malformed stored payload) — worth a small test to hit ~97% branch coverage on that reducer.
  • pr-outcome.d.ts and pr-outcome.js accept any string for closedAt with no ISO-date shape check, unlike the numeric prNumber/decision validation; likely fine since it's opaque bookkeeping, but worth a one-line note if a downstream consumer will parse it as a Date.
  • Consider exporting normalizeOptionalRepoFullName from event-ledger.js so pr-outcome.js (and any future typed-event module) shares one repo-name validator instead of re-deriving it.
  • Add a readPrOutcomes test case with a malformed/foreign-type event mixed into the stream to lock in the defensive filtering behavior described in the code comments.

CI checks failing

  • validate
  • validate-code
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 packages/gittensory-miner/lib/pr-outcome.js with MINER_PR_OUTCOME_EVENT, normalizePrOutcomePayload, recordPrOutcomeSnapshot (DI-shaped like recordManagePollSnapshot and reusing REJECTION_REASONS for closed PRs), and readPrOutcomes mirroring indexLatestManageUpdates, plus unit tests covering the writer for both decisions, normalizer validation, and multi-event reduction, and a README no

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: not available
  • 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 (CI is failing (validate, validate-code)). 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.

@loopover-orb loopover-orb Bot closed this Jul 9, 2026
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