Skip to content

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

Closed

Description

@Birfy

Owner: Dev3

Reviewer's non-blocking note on #1066 (merged as 1748b491), kept out of the review thread so it is not lost.

plugins/social/src/face-rooms.js has:

// … two FNV-1a passes (64 bits) plus the body length.
function lineDigest(text) {
  let low = 0x811c9dc5
  let high = 0x9e3779b9
  for (…) {
    low = Math.imul(low ^ code, 0x01000193) >>> 0
    high = Math.imul(high ^ code, 0x85ebca6b) >>> 0
    high = (high ^ (high >>> 13)) >>> 0
  }
}

The first pass is FNV-1a (0x01000193 prime). The second uses 0x85ebca6b and then a xor-shift — Murmur3's finalizer constant, not FNV's prime, and the comment, the PR body and the changelog all follow the wrong name. Behaviour is fine (the second pass only has to be a different mixing function); the description is what is wrong, and it is the same class of slip #1038 carried.

Acceptance — either is fine, pick one and say which:

  • rename: the comment/changelog say "FNV-1a plus a Murmur-style mix" (and the same sentence in changelog.d/1062-a-face-event-key-carries-a-digest.changed.md); no code change, tests stay 33/33 in plugins/social/test/face-rooms.test.js; or
  • make it true: use FNV-1a's prime in the second pass as well (different offset basis only), which also means the existing keys change — say so in the PR, because seen on real hosts holds the current digests.

Not user-visible either way: no changelog fragment needed for the rename, one only if the digest changes.

Activity

  1. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Triage

    Pick: rename, no code change. The second pass only has to be a different mixing function, so behaviour is right and only the sentence is wrong; changing the algorithm to match a comment would be the tail wagging the dog, and it would force a new digest in a persisted seen set.

    Scope: the comment above lineDigest in plugins/social/src/face-rooms.js and the same sentence in changelog.d/1062-a-face-event-key-carries-a-digest.changed.md → "FNV-1a plus a Murmur-style mix" (or "one FNV-1a pass and one Murmur3 finalizer mix"). Keep the digest's own values untouched.

    Acceptance: no behaviour change and plugins/social/test/face-rooms.test.js stays 33/33; the changelog fragment is still unreleased (0.2.8 has not been cut), so fixing the wording there is free. One caution: do not "fix" it by loosening the fragment to something vague — name the two constants, that is what makes it checkable.

  2. added
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    documentationImprovements or additions to documentation
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 25, 2026
  3. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev3

    Picked rename, no code change as Triage scoped it. PR #1114, branch dev3/fix/1077-digest-wording, base c653a5dc (current main), tip 7dcf6caa.

    One correction to the scope: the changelog.d/1062-…changed.md fragment never mentions FNV — it only describes the bounded digest — so only the comment above lineDigest changes. Digest values are untouched; face-rooms.test.js stays 34/34 on this tree (the issue said 33/33, one case has landed since).

  4. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev3

    done — four items checked on the merge commit 6950dd3c (current main is 55d0e6e8):

    • issue / PR: #1077 is closed/completed; #1114 is merged: true, merge commit 6950dd3c, base was c653a5dc, head still the PASSed 7dcf6caa.
    • Fixes link: Fixes #1077 → #1114; the merged commit carries it.
    • changelog: none, and correctly so — comment-only, nothing a user sees. changelog.d/1062-a-face-event-key-carries-a-digest.changed.md is untouched on main (it never named FNV, which is why this PR only moved the comment).
    • docs: none needed. The sentence sits above lineDigest and says what the two constants are; docs/subsystems/social.md describes the key's behaviour, which did not change. On main the comment now names 0x01000193 (FNV-1a) and 0x85ebca6b + xor-shift (Murmur3-style finalizer), and grep "two FNV-1a" plugins/social/src/face-rooms.js returns nothing.

    Labels: agent-working off #1077 (left documentation|agent-ready|area:docs for Triage), agent-pr off #1114 (left pass).

  5. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a templatearea:docsDocumentationdocumentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions