feat(miner): record feasibility-verdict reasons as rule-fired signals on the infeasible path#8569
Conversation
… on the infeasible path AMS calibration capture was one-sided: discovery records its eligibility exclusions as rule-fired signals, but the feasibility verdict recorded nothing, so feasibility rules could not be precision-scored (JSONbored#8107). Inside attempt-cli's existing !codingTaskSpec.ready branch only, records one RuleFiredEvent per avoidReasons entry (outcome "avoid") and per raiseReasons entry (outcome "raise"), with targetKey byte-identical to discovery's `${repoFullName}#issue-${issueNumber}` and no metadata. Adds an initSignalTrackingStore seam and a default backed by the shared local event ledger, mirroring discover-cli's recordEligibilityExclusionSignals verbatim -- including its best-effort discipline: store-init and per-write failures are swallowed so console output, JSON result, and the exit-4 contract are unchanged. Pure modules stay side-effect free; the ready path records nothing. Closes JSONbored#8543
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8569 +/- ##
==========================================
+ Coverage 89.58% 89.70% +0.11%
==========================================
Files 97 98 +1
Lines 22706 22970 +264
Branches 3872 3960 +88
==========================================
+ Hits 20341 20605 +264
Misses 2187 2187
Partials 178 178
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 21:23:46 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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.
|
Summary
AMS's calibration capture was one-sided: discovery records its four eligibility-exclusion reasons as rule-fired signals (
recordEligibilityExclusionSignals), but the feasibility verdict — the other deterministic gate an attempt passes through — recorded nothing, so feasibility rules could not be precision-scored the same way (#8107).Inside
attempt-cli.ts's existingif (!codingTaskSpec.ready)branch only, this records oneRuleFiredEventperfeasibility.avoidReasonsentry (outcome: "avoid") and perraiseReasonsentry (outcome: "raise"), withruleId= the reason verbatim,targetKey=`${repoFullName}#issue-${issueNumber}`(byte-identical to discovery's format), an ISOoccurredAt, and nometadata.Everything goes through
createSignalTrackingStore(canonicalsignal_rule_firedshape). Adds aninitSignalTrackingStoreseam — same name as discover's — defaulting tocreateSignalTrackingStore({ appendEvent, readEvents })over the shared local event ledger. Best-effort discipline is mirrored verbatim fromrecordEligibilityExclusionSignals: store-init failure (try/catch) and each per-write failure (.catch(() => undefined)) are swallowed, so the branch's console output, JSON result, and exit-4 contract are unchanged. The pure modules (coding-task-spec.ts,feasibility.ts) are untouched, and theready: truepath records nothing.Coverage
100% of the changed lines and branches — verified against the scoped simulation CI runs (
vitest run --coverage --coverage.all=false), zero uncovered lines or branches on the diff.Getting there surfaced a real inconsistency worth noting: four branches were initially unreachable because I passed
runAttempt's already-resolvednowMsto the helper, whereasdiscover-clipasses the rawoptionsso the helper's ownnowMs ?? Date.now()default is exercised. I corrected the call site to forward the raw seam values — this both closes the coverage gap and makes the "mirrorrecordEligibilityExclusionSignalsverbatim" requirement literally true.Tests (root vitest, Codecov-visible via the miner-lib source path — the #8479 precedent):
ruleId/outcome/targetKey/occurredAtasserted per event, order and counts matching the arrays;ready: true→ zero events recorded (capture is infeasible-only);initSignalTrackingStorethrows, and a store whoserecordRuleFiredrejects → exit code, JSON result, and console output identical to a run without the seam;Date.now()fallbacks run;I verified the recording is genuinely wired: reverting the capture call makes the "records one signal per reason" test fail while the swallow tests still pass (a no-op also yields exit 4 — correct).
Closes #8543
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
packages/loopover-miner/lib/attempt-cli.ts+ its root test suite. No workflow, MCP, UI, worker, or dependency surface is touched, soactionlint,build:mcp/test:mcp-pack,ui:*,test:workers, andnpm auditare not exercised by it.packages/loopover-miner/lib/**/*.tsand*.jsare invitest.config.ts'scoverage.include). Roottsc --noEmitand the miner packagetscare clean, andgit diff --checkpasses.test/unit/miner-attempt-cli.test.ts(#5185 ... broken appendAttemptLogEventandreports an unexpected allocator failure ...) fail on my Windows checkout because the sandbox cannot unlink an open sqlite handle (EBUSY). I verified they fail identically with my changes stashed (2 failed on cleanorigin/maintoo), so they are environmental and untouched by this PR; my six new cases pass.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — a miner CLI signal-capture change and its tests; no visible UI, frontend, docs, or extension change.
Notes
ready: falseoccurrence records — no dedup across repeated attempts at the same issue, since each attempt is a distinct decision instance (matching the issue's requirement and discovery's own non-deduped capture).