PR #463 follow-ups: degraded-tick fingerprint divergence, two stale poll.sh comments in dispatch.sh nf-ruh1 ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/473 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #463. None of these blocked the PR — it reached result=CONVERGED. Recorded so they are not re-found, not asked for.

1. The degraded-tick fingerprint diverges from poll.sh's — and the reason it was declined is backwards

tools/pr/poll.py:452 appends jqstr(data.get("headRefOid")) unconditionally, so a gh pr view that fails contributes the literal null to the fingerprint payload. poll.sh contributed *nothing* for that PR. (The same tick also has the shell contributing the raw gh api error body where poll.py — correctly, per #463 F3 — contributes nothing.)

This was raised as a residual on an earlier pass and declined with:

> compute_fingerprint appending jqstr(None) where the shell appended nothing is > stably wrong on both sides, so the ladder behaves identically, and "wrong the same > way" is the property the fingerprint actually needs. I'd rather not "fix" it into a > divergence from a poll.sh-written state file.

Measured at head 3e33471, four interleaved --check runs against one live tracker holding 463, 462 and a bogus 9999999:

``` SH NO-CHANGE 637235350 SH NO-CHANGE 637235350 PY NO-CHANGE 3668878516 PY NO-CHANGE 3668878516 ```

Each side is stable; they are not stable *as each other*. jqstr(None) is the divergence, not the protection against it. Payload diff, sorted, two lines:

``` < {"message":"Resource not accessible by integration",...,"status":"403"}412 # shell only > 412 # port > null # port only ```

Scope is narrow and that is why it stayed out of the gate: on a healthy tracker the payloads are byte-identical and both sides produce 2251854758. It fires only when tracked-prs.txt holds a PR gh cannot read, and costs one spurious CHANGED → one full digest → one ladder reset, at the handover tick only.

If it is worth closing: skip the headRefOid line when pr_view returned nothing, i.e. the same "a failed read contributes nothing" rule the shell had by construction. A three-line assertion on compute_fingerprint's sorted payload for a degraded PR would pin it — and would also have caught the earlier F1 (closing refs joined as one element) and the empty-closing case below, none of which any golden fixture happened to exercise.

2. Two poll.sh references left in dispatch.sh, one of them factually wrong

- tools/reviewer/dispatch.sh:171 — *"poll.sh greps this warning out of dispatch.log"*. This line arrived with the d2f7eb9 merge of #462, i.e. after #463's rename sweep ran, so the sweep could not have caught it. - tools/reviewer/dispatch.sh:267 — *"NOTE (#379): poll.sh matches the same line with a (?m) regex."* Both halves are stale: the file is poll.py, and poll.py:403 (first_verdict_line) uses a per-line startswith, not a regex. The next clause is a live coupling instruction ("if either moves, move both"), so a wrong description carries weight.

poll.py:100-107 names comment-rot as this file's recurring defect, and these are two more instances of it.

3. Cosmetic

- tools/pr/poll.py imports subprocess, zlib and typing.Iterable and uses none of them (zlib is a leftover from the approach _cksum's own docstring rejects). - tools/tests/test_nflib_gh.py:26 re-imports Gh one line after importing it at :25.

Nothing lints Python in this repo, so neither is caught automatically.

4. Carried forward from earlier passes on #463 (unchanged, still open)

- dispatch_interval() raises ValueError on a non-numeric NINEFOLD_DISPATCH_INTERVAL_MIN, and the port has no equivalent of the shell's catch-all *"⚠ STALE-DISPATCH: the liveness check itself FAILED (exit $rc)"*. - An issue whose gh issue view fails renders as #n [UNCOVERED] : null. - Empty closing diverges, same class as item 1: poll.sh:364 was `printf '%s\n' "$closing", which emits ONE BLANK LINE into the sort | cksum` stream when no open PR carries a Closes/Refs; lines.extend([]) emits nothing. Moot once poll.sh is deleted, and not reachable today. - NFR verdict tie-break. poll.sh used sort_by(.t) | last (stable → *last* of equal timestamps); max() returns the *first* maximal element. Only bites when two nfr posts share a second, and GitHub stamps to the second. - stderr on the degraded path. poll.sh had 2>/dev/null on exactly the calls that fail on a degraded tick (both compute_fp calls, the --check ledger call, gh issue view in the issue loop); lenient_run passes every failed call's stderr through. Terminal noise only — stdout, the parsed contract, is unaffected. Arguably an improvement; recorded because it is an undocumented difference.

5. Untested-but-verified surface

compute_fingerprint, run_digest, emit_tracked_prs, emit_issues and nflib.force_utf8's newline="\n" are driven by no unit test. All were exercised by the live differential on #463 (1093 lines, zero diff against poll.sh), which for this class of code is the stronger evidence — but nothing pins them in the suite, so a later refactor has no guard.

Dependencies

None.

Comments

No comments.

Add a comment