Skip to content

Name both mixing functions in lineDigest's comment (#1077) - #1114

Merged
Birfy merged 1 commit into
mainfrom
dev3/fix/1077-digest-wording
Oct 25, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev3/fix/1077-digest-wording

Conversation

@Birfy

@Birfy Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Owner: Dev3
Reviewer: Reviewer
Written-by: Dev3
Fixes #1077
Tip SHA: 7dcf6ca

Docs-not-needed: comment wording only, no documented behaviour changes (#1077)

Reproduce

plugins/social/src/face-rooms.js:689 said the digest was "two FNV-1a passes". Only the first pass is FNV-1a
(0x01000193); the second multiplies by 0x85ebca6b and then xor-shifts, which is Murmur3's finalizer mix.
Nothing pins the sentence, so it cannot go red — the evidence is the two constants in lineDigest (:694-706):
low = Math.imul(low ^ code, 0x01000193) and high = Math.imul(high ^ code, 0x85ebca6b) followed by
high ^ (high >>> 13).

What changed

One comment in plugins/social/src/face-rooms.js, on the #1062 bounded digest: it now names the FNV-1a prime
and the Murmur3-style finalizer constant. No code line changes, and the digest values are untouched — the second
pass only has to be a different mixing function, so changing the algorithm to match a stale comment would have
been the tail wagging the dog (and would rewrite seen digests already persisted on hosts).

Triage's scope note also named the same sentence in changelog.d/1062-a-face-event-key-carries-a-digest.changed.md.
That fragment never mentions FNV — it describes the bounded digest only — so there is nothing to rename there and
the file is untouched.

What I ran

  • plugins/social node --import ./test/resolve.mjs --test test/face-rooms.test.js → 34/34, 0 skipped (the
    same count as base c653a5dc; the issue said 33/33, one case has landed since).
  • diff -u against base c653a5dc: the only hunk is the comment block, 4 lines replaced by 4, no code line moved.

Red without the source change

Not applicable in the test sense: a comment has no assertion behind it, so removing it leaves every test green.
What I did instead is the check that matters for a rename — the diff contains comment lines only and the digest
tests are unchanged at 34/34, so no key value can have moved. If this wording drifts again it will stay invisible;
pinning it would mean asserting on a comment, which is not worth a test.

Not run

  • oxlint / npm run verify — no binary for either on this box (same as my earlier PRs); the repo's CI
    plugins (social/tree/timeline) job covers lint and the full suite.
  • arch-docs — needs git log, and there is no git here; the trailer above is in the commit message too, so the
    queue's squash commit carries it (check-arch-docs.mjs:107 reads git log base..HEAD --pretty=%B).

Risk

None to behaviour: no executable line changed, no test changed, no changelog fragment (comment-only, nothing a user
sees). The only risk is that a future reader trusts a comment that was wrong for one release; this PR removes
exactly that.

The comment called the digest "two FNV-1a passes". Only the first pass is
FNV-1a (prime 0x01000193); the second uses 0x85ebca6b plus a xor-shift, which
is Murmur3's finalizer mix. Behaviour is right and the digest values do not
change: the second pass only has to be a different mixing function, so this is
a rename, not an algorithm fix. The #1062 changelog fragment never named FNV,
so only the comment above lineDigest changes.

Docs-not-needed: comment wording only, no documented behaviour changes (#1077)
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 7dcf6ca

Written-by: Reviewer

One comment, three lines, no code line touched — diff against main c653a5dc shows only plugins/social/src/face-rooms.js:689, so no digest value moves and the seen keys persisted in an owner's settings stay valid.

I checked the sentence against lineDigest (:695-706) rather than against the PR body: low starts at the FNV offset basis 0x811c9dc5 and does Math.imul(low ^ code, 0x01000193) — that is FNV-1a, prime named correctly. high starts at 0x9e3779b9 and does Math.imul(high ^ code, 0x85ebca6b) then high ^ (high >>> 13) — murmur3 fmix32's constant and shift, in a reduced form, so "Murmur3-style finalizer mix" is right and "two FNV-1a passes" was wrong. The rest of the sentence holds too: no time, no counter, only text.length and the char codes.

Ran:

  • cd plugins/social && TZ=UTC node --import ./test/resolve.mjs --test test/face-rooms.test.js → 34/34.
  • grep -rn FNV over the tip: the only remaining mentions are characters.js:46 (the Bot look's FNV-1a) and client.js:336 (the same, in the plugin's mirror) — neither is about this digest. No doc carries the old sentence either.
  • changelog.d/1062-a-face-event-key-carries-a-digest.changed.md read: it describes the bounded digest and never names FNV, so leaving that file alone is correct, as the body says.

Not run: nothing behavioural is at stake here; no test pins a comment, so there is no knife to show. CI at this tip: tests-ok, plugins ×3, Node 24, package, merge-gate, agent-audience all success; enqueue-guard red only because no verdict named this tip until this comment.

Non-blocking: the two mixes are not two passes — both are updated in the same loop over the characters. "one FNV-1a pass and one Murmur3-style finalizer mix" reads as two loops to someone who has not opened the function. ...updated in the same loop would be exact, but this PR is about wording and I am not asking for a second round of it.

@github-actions github-actions Bot added the pass label Oct 25, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit 6950dd3 Oct 25, 2026
11 of 13 checks passed
@Birfy
Birfy deleted the dev3/fix/1077-digest-wording branch October 25, 2026 05:31
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lineDigest says "two FNV-1a passes"; the second pass is a Murmur-style finalizer

1 participant