ledger-signals: extend duplicateDirections to see still-open PRs - #33
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Gj1odM4UYS1miHuiHnfbN
|
Resolved a merge conflict against Side observation while re-verifying: Generated by Claude Code |
learningSignals()'s duplicateDirections only scanned rows already merged into LEDGER.md, so a direction repeated across several still-open draft PRs (demonstrated live tonight: PR #15 and PR #27, both unmerged, both targeting the same zeroMergeStreak defect) went undetected. Add an opt-in pendingFindings option folded into the same normalization/count pipeline, plus a --pending CLI flag on `ledger signals`, so a future night can feed in open-PR findings and get the same detector to see them. Default (omitted) behavior is unchanged. Nightly Dream Cycle, 2026-08-26. DEEP=ledger-signals, SCAN=witness,verify. Full report: docs/dream-cycle/2026-08-26-ledger-signals-report.md Issue: #32 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Gj1odM4UYS1miHuiHnfbN
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Gj1odM4UYS1miHuiHnfbN
58fee1c to
0dfec6f
Compare
#96) Landing the long-stale dream-cycle backlog (#21/#29/#33/#35/#42/#46) required rebasing branches whose conflicts were mostly additive, but a mechanical union of the conflict hunks cut across syntactic boundaries in several places and left main red. This repairs all of it: - ledger/src/index.ts: `pendingFindings` and the closing brace of LearningSignalsOptions landed inside daysBetween()'s body; field moved back into the interface and the function restored. - ledger|compile|cli test files: five `it(...)` blocks lost their closing `});` at the seam between the two sides' test sets. - cli/src/index.test.ts: duplicated import block collapsed. - cli/src/index.ts: `ledger signals` emitted TWO JSON objects (one for --merged, one for --pending) instead of one combined result, so JSON.parse(out) threw for every signals test; both option sets now feed a single learningSignals() call. Duplicated help entry collapsed. - witness/src/index.ts: #35's `termination` and #42's `trace-replay` each export their own `canonicalJson` (TS2308). They are not interchangeable -- trace-replay's does WeakSet cycle detection and takes `unknown`, termination's is typed to JsonValue -- so trace-replay is now re-exported explicitly without it rather than silently promoting either. Verified: typecheck clean, 585/585 vitest, `npm run check` 80/81 (the one failure is the macOS-only /tmp symlink case in mission.test.mjs, which this repo's Linux-only CI does not exercise). Claude-Session: https://claude.ai/code/session_01Ff2xRKvYrqXJhefvcapfE1
Nightly Dream Cycle, 2026-08-26. DEEP=ledger-signals, SCAN=witness,verify. Full report:
docs/dream-cycle/2026-08-26-ledger-signals-report.md. Issue: #32.Hypothesis
Given
duplicateDirectionsonly scans ledger rows already merged intoLEDGER.mdon main, whenlearningSignals()gains an optionalpendingFindings: string[]parameter folded into the same first-6-word normalization/counting pipeline already used for merged rows, then a direction repeated across merged rows + still-open PRs' findings (>= 3 combined) should be flagged — while the default (omitted) path stays byte-identical. Frozen before implementation.Candidate
+130/-9 across 5 files (1 new report, 4 edits:
packages/ledger/src/index.ts+ its tests,packages/cli/src/index.ts+ its tests). One conceptual change: an additivependingFindingsoption onlearningSignals()+ a--pending "f1|f2"CLI flag onledger signals. No removal, no behavior change when the option/flag is omitted.Evaluation Receipt
Real evaluator:
npm test(vitest, this repo's ownbenchentrypoint).Baseline reconfirmed via
git stashimmediately before finalizing (96/96, exact match). Live receipt:Baseline
Parent commit
074fd1fe6ceac4bebc993be6b6b0f0f69cc750f5, 96/96 tests passing, clean build (npm ci && npm run build, no wasm/NAPI packages in this workspace).Darwin Lineage
Not run —
DARWIN=not-applicable. Small, pure, deterministic string-processing change; no evolvable population. Same judgment as the 2026-08-13/2026-08-24 precedents.Evidence
Confirmed live via GitHub MCP tonight:
LEDGER.mdon main has 1 row while GitHub has 9 open, unmerged dream-cycle draft PRs (#15, #17, #19, #21, #23, #24, #27, #29, #30), 0 merged since #7/#13. PR #15 (2026-08-16) and PR #27 (2026-08-24) independently propose closely related fixes for the samezeroMergeStreak-can't-see-real-merges defect — a real, demonstrated duplicate-direction case the existing detector cannot see, because neither finding ever landed inLEDGER.md.Honesty correction (post-critique): the draft report initially implied this candidate "would have caught #15 vs #27." Verified directly (
directionKeyon the real title text) that it would not — their real titles don't share a 6-word lexical prefix. The candidate extends recall (pending findings become visible to the same detector), not the detector's underlying matching precision, which is a known, pre-existing limitation unchanged by this PR. See the full report for the corrected, honest framing.Reward-Hack Check
Independent critic: a fresh subagent (Agent tool, no access to this candidate's authoring context) reviewed the diff adversarially. Initial verdict: BLOCKED on two findings:
parsePendingFindingssplit on,— the real motivating PR developer-experience: thread real merge state into zero-merge learning signal (cli, tui) #27 title contains a comma and would fracture into a bogus extra finding. Not covered by the original tests. Fixed: delimiter switched to|(matchesLEDGER.md's own field separator), regression test added.Both fixes verified: rebuilt, reran
npm test(104/104, 0 regressions), re-ranparsePendingFindingsagainst the real PR #27 title to confirm the comma is preserved. Re-reviewed after fixes: CLEAR. No pre-existing test weakened/removed; default path verified byte-identical via a dedicated regression test; no new I/O/network/shell-exec/credential surface; no threshold/gate/safety constant touched; scope limited to the 4 intended source+test files.Security Review
Pure string parsing (
parsePendingFindings,directionKey,bumpDirection) — no eval, no shell interpolation, no external I/O, no LLM calls.--pending's input is always caller-supplied, never attacker-controllable runtime input.Regression Analysis
0 pre-existing tests modified or removed. All 96 baseline tests still pass unchanged; 8 new tests added (3 in
packages/ledger, 5 inpackages/cli, including the post-critique comma-preservation regression test).ADR
None — additive, opt-in extension to an existing, already-tested signal-computation function, not a new architectural decision. Same judgment as the PR #15/#27 precedents for adjacent changes.
Gist
No
gh gist create/ghCLI available in this environment. Report committed atdocs/dream-cycle/2026-08-26-ledger-signals-report.mdinstead.GIST=LOCAL.Issue
#32
Witness
Verify:
sha256sum docs/dream-cycle/2026-08-26-ledger-signals-report.md, thenprintf '%s%s' "<that hash>" "074fd1fe6ceac4bebc993be6b6b0f0f69cc750f5" | sha256summust equal the witness above. Confirmed tonight viadream-machine witness verify(✓ VALID).Merge Policy
Draft — human review required. This PR does not carry the
automerge-safelabel: it modifies a decision-support signal (duplicateDirections) that future nights' STEP 1.1 relies on, which is exactly the low-confidence-blast-radius case this repo's guarded-auto-merge policy (.github/workflows/automerge.yml) keeps human-review-only. The session never applies that label itself and never merges.Also flagged for human triage, not fixed by this PR: 9 dream-cycle draft PRs are open and unmerged (#15, #17, #19, #21, #23, #24, #27, #29, #30). This candidate makes the symptom (undetected duplicate directions) more visible for a future night that wires
--pendingin, but the actual fix — merging or closing the backlog — is a human decision outside any single nightly candidate's scope.Generated by Claude Code