Repository navigation
fireflies: a transcript's id and a user's user_id are ULIDs, not 24 hex characters - #1103
Conversation
nuriyoo
left a comment
There was a problem hiding this comment.
Thanks for this, and welcome. The form is right. I measured it again on 2026-10-27 with the workspace's API key: the user's and both transcripts' ids are 26 characters of Crockford's base32, upper case, starting 01. The issue measured that a transcript id in lower case is object_not_found; a user id in lower case is not_in_team, and both as sent are found. This branch answers both lower-case spellings with nothing found too, since the two lookups compare ids as stored.
What I ran:
- The new test against the base's code: it fails, as it should. CI is green, and the branch merges into current
mainwithout conflicts, though it is 263 commits behind it. - A mutation run over the lines this PR adds to
synth.py,store.pyand the schema, againsttests/test_fireflies.py,test_store.py,test_acl.pyand the two importer test files, with hand mutations on top: a lower-case alphabet,Iin place ofZ, 76 or 96 bits from the digest, 25 or 27 characters, and a transcript seed or user salt changed. Every one breaks a test, apart from dropping the schema field'stypeordescription, which no test reads.
The inline comments:
_ulid'swhen_mshas no caller, and the docstring says the id names no moment where it names 1970-01-01T00:00:00Z. You asked at the end of the body whether the id should carry the record's date instead: zero is fine, for the reason the body gives, and the inline comment says where that reason shows.- The same "no time" in
fireflies_id's docstring and in the schema. - The test docstring carries the issue number, the old form and a second copy of the measurement, and the decode loop checks nothing the prefix check doesn't.
- A rewrap in
store.py.
One more test, for after you merge main. With upper-case ids, the lower-case spelling becomes something a client can send, and nothing here pins that it finds nothing: making the two lookups case-insensitive (COLLATE NOCASE on fireflies_transcript_by_id's and fireflies_user_by_served_id's queries) passes every test in tests/test_fireflies.py, test_acl.py and test_store.py. main has a table for exactly these answers, test_fireflies_an_id_naming_nothing_answers_with_reals_error, and two rows there fail each of those two changes made alone:
(hidden,) = {t["id"] for t in every} - {t["id"] for t in seen}
ava_id = ff_gql(client, "{ user { user_id } }", ava_h).json()["data"]["user"]["user_id"]
wanted = {
...
("ava@acme.com", '{ transcript(id: "%s") { id } }' % hidden): object_not_found,
("ava@acme.com", '{ user(id: "%s") { user_id } }' % ava_id.lower()): not_in_team,
("ava@acme.com", '{ transcript(id: "%s") { id } }' % seen[0]["id"].lower()): (
object_not_found
),The suggestions give the tree I tested: tests/test_fireflies.py, test_store.py, test_acl.py, both importer test files and test_synth.py pass (649), and ruff check, ruff format --check and scripts/gen_docs.py --check are clean. Merged into current main with the two rows above, those files plus the packaging, SDK, integration, MCP, LlamaIndex, cross-vendor and fidelity tests give 1,090 passed and 2 skipped (Docker and the HubSpot reader), and scripts/prose_check.py is clean.
| "transcript_id": { | ||
| "type": "string", | ||
| "description": "The API-facing id `transcript(id:)` looks a meeting up by. Default: a 24-character hex id derived from the doc_id." | ||
| "description": "The API-facing id `transcript(id:)` looks a meeting up by. Default: a ULID (26 Crockford base32 characters) derived from the doc_id; it carries no time." |
There was a problem hiding this comment.
As in fireflies_id's docstring: the id carries a time, zero.
| "description": "The API-facing id `transcript(id:)` looks a meeting up by. Default: a ULID (26 Crockford base32 characters) derived from the doc_id; it carries no time." | |
| "description": "The API-facing id `transcript(id:)` looks a meeting up by. Default: a ULID (26 Crockford base32 characters) derived from the doc_id, its time zero." |
| -- `synth.fireflies_user_id` is a ULID, 80 of whose bits vary -- the raw seed is stored as-is and a collision | ||
| -- (vanishingly unlikely at any real scale) fails the import loudly through this index rather than | ||
| -- resolving silently. |
There was a problem hiding this comment.
The first line runs to 113 characters; rewrapped at 100 like the comment's other lines.
| -- `synth.fireflies_user_id` is a ULID, 80 of whose bits vary -- the raw seed is stored as-is and a collision | |
| -- (vanishingly unlikely at any real scale) fails the import loudly through this index rather than | |
| -- resolving silently. | |
| -- `synth.fireflies_user_id` is a ULID, 80 of whose bits vary -- the raw seed is stored as-is and a | |
| -- collision (vanishingly unlikely at any real scale) fails the import loudly through this index | |
| -- rather than resolving silently. |
| def _ulid(seed: str, when_ms: int = 0) -> str: | ||
| """A ULID, 26 characters of Crockford's base32: a 48-bit millisecond time, then 80 random bits. | ||
|
|
||
| Fireflies serves ULIDs (measured 2026-10-27: 26 characters, this alphabet, upper case). The | ||
| time is left at zero, i.e. the id names no moment: a measured id's time sits *before* its own | ||
| meeting by an amount that varies (1 day 22 hours on one transcript, 4 hours 24 minutes on | ||
| another), so there is no rule to reproduce, and inventing an offset would make the id claim a | ||
| fact the corpus does not hold. The form is what a client reads; the timestamp is not. | ||
| """ | ||
| if not 0 <= when_ms < 1 << 48: | ||
| raise ValueError("a ULID's time is 48 bits of milliseconds, got %d" % when_ms) | ||
| value = (when_ms << 80) | int(_digest(seed)[:20], 16) |
There was a problem hiding this comment.
Two things here.
- No caller passes
when_ms, so the parameter and itsValueErrorcan't be reached; both callers want the zero time. If a later change wants a time, it can add the parameter with its first caller. - "the id names no moment" isn't quite what a client sees: a ULID library decodes these ids to 1970-01-01T00:00:00Z, the moment zero is. And "inventing an offset would make the id claim a fact the corpus does not hold" holds for zero as well. The reason zero is the better answer is the one the PR body gives: it keeps
fireflies_id(seed)a function of the seed alone, which the id-column table instore.py,tests/test_acl.pyand the erb importer's test all call it as. That answers the question at the end of the body too: zero is fine, keep it.
| def _ulid(seed: str, when_ms: int = 0) -> str: | |
| """A ULID, 26 characters of Crockford's base32: a 48-bit millisecond time, then 80 random bits. | |
| Fireflies serves ULIDs (measured 2026-10-27: 26 characters, this alphabet, upper case). The | |
| time is left at zero, i.e. the id names no moment: a measured id's time sits *before* its own | |
| meeting by an amount that varies (1 day 22 hours on one transcript, 4 hours 24 minutes on | |
| another), so there is no rule to reproduce, and inventing an offset would make the id claim a | |
| fact the corpus does not hold. The form is what a client reads; the timestamp is not. | |
| """ | |
| if not 0 <= when_ms < 1 << 48: | |
| raise ValueError("a ULID's time is 48 bits of milliseconds, got %d" % when_ms) | |
| value = (when_ms << 80) | int(_digest(seed)[:20], 16) | |
| def _ulid(seed: str) -> str: | |
| """A ULID, 26 characters of Crockford's base32: a 48-bit millisecond time, then 80 bits of | |
| ``seed``'s digest. | |
| Fireflies serves ULIDs (measured 2026-10-27: 26 characters, this alphabet, upper case). The | |
| time is zero, which a client decodes as 1970-01-01T00:00:00Z: a measured id's time sits | |
| *before* its own meeting by an amount that varies (1 day 22 hours on one transcript, 4 hours | |
| 24 minutes on another), so there is no rule to reproduce, and zero keeps the id a function of | |
| ``seed`` alone. | |
| """ | |
| value = int(_digest(seed)[:20], 16) |
| unique — ``transcript(id:)`` looks a meeting up by this one (see the store schema). Carries no | ||
| time; see ``_ulid`` for why. |
There was a problem hiding this comment.
Same point as on _ulid: the time is there, and it is zero.
| unique — ``transcript(id:)`` looks a meeting up by this one (see the store schema). Carries no | |
| time; see ``_ulid`` for why. | |
| unique — ``transcript(id:)`` looks a meeting up by this one (see the store schema). Its time is | |
| zero; see ``_ulid`` for why. |
| """#1089: real serves a ULID, 26 Crockford base32 characters, where this served 24 hex ones. | ||
|
|
||
| Measured against api.fireflies.ai on 2026-10-27: `users { user_id }` and | ||
| `transcripts { id user { user_id } }` answered 26-character ids drawn from Crockford's | ||
| alphabet (no I, L, O or U), upper case. A client tells the two forms apart by length and | ||
| characters alone, so the form is the whole of this test. | ||
| """ |
There was a problem hiding this comment.
The docstring carries the issue number and the history ("where this served 24 hex ones"), and its second paragraph repeats the measurement _ulid's docstring already records. Suggested: what the test checks.
| """#1089: real serves a ULID, 26 Crockford base32 characters, where this served 24 hex ones. | |
| Measured against api.fireflies.ai on 2026-10-27: `users { user_id }` and | |
| `transcripts { id user { user_id } }` answered 26-character ids drawn from Crockford's | |
| alphabet (no I, L, O or U), upper case. A client tells the two forms apart by length and | |
| characters alone, so the form is the whole of this test. | |
| """ | |
| """A served user id and transcript id are ULIDs as `synth._ulid` writes them, their first ten | |
| characters, the time, zero.""" |
| # 26 upper-case hex characters are also 26 characters of this alphabet, so decode: a ULID | ||
| # is 128 bits with the top two clear, and its first ten characters are the time, which | ||
| # this id deliberately leaves at zero. | ||
| bits = 0 | ||
| for char in one: | ||
| bits = bits * 32 + alphabet.index(char) | ||
| assert bits < 1 << 128, one |
There was a problem hiding this comment.
Once the first ten characters are 0, the value is below 2^125, so bits < 1 << 128 can't fail when the next assertion passes; the decode adds nothing the prefix check doesn't.
| # 26 upper-case hex characters are also 26 characters of this alphabet, so decode: a ULID | |
| # is 128 bits with the top two clear, and its first ten characters are the time, which | |
| # this id deliberately leaves at zero. | |
| bits = 0 | |
| for char in one: | |
| bits = bits * 32 + alphabet.index(char) | |
| assert bits < 1 << 128, one |
…ex characters Real Fireflies serves 26 characters of Crockford's base32 in both places, measured against api.fireflies.ai on 2026-10-27: three distinct ids, each decodable as a ULID whose time sits earlier than the meeting it names. Ours were a 24-character lowercase hex digest, which a client tells apart by length and alphabet alone. Both ids now come from one `_ulid(seed)` helper, which fills the 80 random bits from the digest and leaves the 48-bit time at zero: a client decodes that as 1970-01-01T00:00:00Z, and the measured ids' times are earlier than their own meeting by an amount that varies (1d22h on one transcript, 4h24m on another), so there is no offset to reproduce. Zero is also what keeps the id a function of the seed alone, which both ids have to be: a transcript's is the table's PRIMARY KEY and a user's is reversed through idx_fireflies_user_served. A lower-case spelling of either id finds nothing, as real does, and two rows in test_fireflies_an_id_naming_nothing_answers_with_reals_error pin it. `pytest`: 988 passed on the fireflies, store, ACL, synth and importer files. `ruff check .` and `ruff format --check .` clean, `scripts/gen_docs.py --check` clean.
f66ac68 to
f141513
Compare
|
All five inline points and the two rows are in, on
On the merged tree: The PR body is rewritten to match: the "I could put the date in the id if you prefer" paragraph is gone, since you settled it. |
…gest
The served ids' length, alphabet and zero time hold as well for a ULID of 76 bits of the digest
or of 16 of the 32 symbols. Decoding `synth._ulid("seed")` with the test's own alphabet and
comparing it with the first 80 bits of sha256 pins what `_ulid`'s docstring says the other 16
characters are.
nuriyoo
left a comment
There was a problem hiding this comment.
Thanks for this one, and for taking every point of the first round, the two lower-case rows included.
I put one commit on the branch, 3797cec9, rather than another round: the ids' length, alphabet and zero time hold as well for a ULID built from 76 bits of the digest or from 16 of the 32 symbols, so test_fireflies_ids_are_ulids now also decodes synth._ulid("seed") with its own alphabet and compares it with the first 80 bits of the seed's sha256, which is what _ulid's docstring says the other 16 characters are. I also rewrote the description to the template's sections and dropped the ORDER BY id paragraph: the hex ids ordered by the digest too, so a tie on created_ts was not chronological before this either.
On current main the full suite passes, and both Fireflies tests fail against main's code.
Closes #1089. A Fireflies transcript's
idand a user'suser_idare ULIDs, as real serves them: 26 characters of Crockford's base32, upper case.What changed
The ids.
fireflies_idandfireflies_user_idreturn the ULID_ulid(seed)inbacklot/synth.pywrites from the seed's digest, its time zero, in place of the 24 lower-case hex characters of that digest thatmainserves. The schema's description oftranscript_idand the twostore.pycomments on the ids give the new form.Lookups.
transcript(id:)anduser(id:)find an id as sent. Its lower-case spelling finds nothing:object_not_foundfor a transcript,not_in_teamfor a user.Why this is what the real API does
Measured against api.fireflies.ai on 2026-10-27:
{ users { user_id } transcripts(limit: 10) { id user { user_id } } }answered three ids, a user's and two transcripts'. Each is 26 characters of0–9and the upper-case letters butI,L,OandU, which is Crockford's base32.transcript(id:)anduser(id:)found each id as sent, and its lower-case spelling gotobject_not_foundfor the transcript andnot_in_teamfor the user.This server's time is zero;
_ulid's docstring gives the two measured offsets and why no rule reproduces them.Verification
Tests — on
uv sync --locked --extra dev. At1d897739, onmainat879d7cf6:The mutations are the ones
mutation-siteswrites for the branch's lines inbacklot/, and five more on_ulid's encoding: the digest's width, the character count, the 5-bit mask and two alphabets. They ran againsttests/test_fireflies.py,tests/test_store.py,tests/test_importer_erb.pyandtests/test_acl.py, and the schema's against the tests that read the schemas. Three live, each on a partmainalready has or on prose: theor ""infireflies_user_id, which the branch keeps frommain's line, and thetranscript_idproperty'stype, alsomain's, and description, which no test orscripts/gen_docs.pyreads.Optional surfaces that ran rather than skipped — none here: the 40 skips are
tests/test_sdk.py,tests/test_mcp.py,tests/test_s3.py,tests/test_llamaindex.py, and rows oftests/test_integrations.py,tests/test_fidelity.pyandtests/test_telemetry.py, whose extras.[dev]leaves out. CI installs every extra.Lint —
ruff check .,ruff format --check .,scripts/prose_check.py origin/mainandscripts/gen_docs.py --check: clean.A test fails without this change —
test_fireflies_ids_are_ulids,test_fireflies_an_id_naming_nothing_answers_with_reals_error.A new endpoint serving corpus content is ACL-scoped, proved for both an admin and a scoped token (no new endpoint; the ids are served by the resolvers that already served the hex ones)
Comments state what was measured, not the history of the fix