nandi/frq · 60bbd35

nandi/frqpublic⑂ Fork 0

⑂ 60bbd35

Faces, fetched off the thread that draws them

`Message.avatar` has existed since the port and nothing ever wrote to
it, so every row painted an initial. With DIDs now arriving from WHO,
there is finally something to look a face up by.

The face comes from the profile cache at render time rather than from
the message. A face belongs to a person and not to a line they said: a
profile that lands after someone's first message should appear on all of
them, which it cannot if it was copied onto a Message when the line
arrived.

Fetching is a worker thread. `fetch` blocks and was affordable for the
panel — a reader presses a face and waits — but a room of twelve is
twelve HTTPS round trips, and the caller is the thread that answers
every keystroke. `want` puts an actor on a channel and marks the cache
loading, so a second ask while the first is in flight is not a second
round trip; `collect` folds the answers in from the drain, on the one
thread that owns the cache. Nothing is shared but the channels. Opening
the profile panel uses the same path, so that no longer blocks either.

The worker is stopped and joined at exit. A thread still running while
the process tears down is a thread calling `newContext` after OpenSSL
has been unloaded under it — a SIGSEGV inside `net.newContext`, which is
how the suite reported it the first time this landed, after every test
in the file had passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandi committed 2026-09-19T10:19:42-07:00 Browse files

60bbd35 parent: 4b72ad1

modified nim/src/frq/profile.nim +94 -1 Viewed

@@ -18,7 +18,7 @@
18	18	## The Clojure has a fetch seam here because the two compilers disagreed about
19	19	## HTTP. Nim has one client, so the seam is gone and `fetch` simply asks.
20	20
21		-import std/[json, strutils, tables]
	21	+import std/[exitprocs, json, strutils, tables]
22	22	from std/unicode import runeLen, runeSubStr
23	23	import frq/[atproto, trace]
24	24
@@ -135,6 +135,96 @@ proc fetch*(actor: string) =
135	135	trace("profile", "could not fetch " & actor & ": " & e.msg)
136	136	cache[actor] = Profile(status: psFailed)
137	137
	138	+# ------------------------------------------------------------ in the background
	139	+
	140	+# Faces are wanted for everyone in a room at once, and a profile is an HTTPS
	141	+# round trip each. Blocking was affordable for the panel — a reader presses a
	142	+# face and waits — and is not affordable for a room of twelve, on the thread
	143	+# that also answers every keystroke.
	144	+#
	145	+# So: a worker takes actors off one channel and puts answers on another, and
	146	+# `collect` folds them into the cache on the thread that owns it. The cache
	147	+# itself is never touched from two threads; only the channels are.
	148	+
	149	+var
	150	+ requests: Channel[string]
	151	+ answers: Channel[string] ## "<actor>\x1f<json>", the json empty on failure
	152	+ fetcher: Thread[void]
	153	+ fetching: bool
	154	+
	155	+var stopping: bool
	156	+
	157	+proc fetcherBody() {.thread.} =
	158	+ {.gcsafe.}:
	159	+ while true:
	160	+ let actor = requests.recv()
	161	+ if actor.len == 0 or stopping: break
	162	+ var body = ""
	163	+ try:
	164	+ body = $getProfile(actor)
	165	+ except CatchableError as e:
	166	+ trace("profile", "could not fetch " & actor & ": " & e.msg)
	167	+ answers.send(actor & "\x1f" & body)
	168	+
	169	+proc want*(actor: string) =
	170	+ ## Ask for a profile, without waiting for it.
	171	+ ##
	172	+ ## Once per identity for the run: `psLoading` goes in the cache here, so a
	173	+ ## second ask for the same person while the first is in flight is not a
	174	+ ## second round trip.
	175	+ if actor.len == 0 or cache.hasKey(actor) or isAgent(actor): return
	176	+ cache[actor] = Profile(status: psLoading)
	177	+ if not fetching:
	178	+ fetching = true
	179	+ createThread(fetcher, fetcherBody)
	180	+ requests.send(actor)
	181	+
	182	+proc stopFetching() =
	183	+ ## Stop the worker and wait for it, at exit.
	184	+ ##
	185	+ ## Not tidiness: a thread that is still running when the process tears down
	186	+ ## is a thread calling `newContext` after OpenSSL has been unloaded under
	187	+ ## it, which is a SIGSEGV inside `net.newContext` reported against whatever
	188	+ ## ran last. The test suite found it the first time this landed.
	189	+ ##
	190	+ ## The sentinel is an empty actor. A request already in flight finishes
	191	+ ## first — at worst the HTTP timeout, and in practice the round trip that
	192	+ ## was already nearly done.
	193	+ if not fetching: return
	194	+ stopping = true
	195	+ requests.send("")
	196	+ joinThread(fetcher)
	197	+ fetching = false
	198	+
	199	+addExitProc(stopFetching)
	200	+
	201	+proc collect*(): bool =
	202	+ ## Fold whatever has come back into the cache. True where anything did, so
	203	+ ## a caller knows the screen has something new on it.
	204	+ while true:
	205	+ let (ok, msg) = answers.tryRecv()
	206	+ if not ok: break
	207	+ let sep = msg.find('\x1f')
	208	+ if sep < 0: continue
	209	+ let actor = msg[0 ..< sep]
	210	+ let body = msg[sep + 1 .. ^1]
	211	+ cache[actor] =
	212	+ if body.len == 0: Profile(status: psFailed)
	213	+ else:
	214	+ try:
	215	+ let j = parseJson(body)
	216	+ if j{"did"}.getStr().len > 0: parseProfile(j)
	217	+ else: Profile(status: psFailed)
	218	+ except CatchableError: Profile(status: psFailed)
	219	+ result = true
	220	+
	221	+proc avatarFor*(actor: string): string =
	222	+ ## The face to paint for this identity, or "" where there is not one yet.
	223	+ ## A lookup and never a fetch: this is called from the render path.
	224	+ if actor.len == 0: return ""
	225	+ let p = cache.getOrDefault(actor)
	226	+ if p.status == psReady: p.avatar else: ""
	227	+
138	228	proc forgetProfiles*() =
139	229	## For a test that wants a known starting point.
140	230	cache.clear()
@@ -160,3 +250,6 @@ proc truncate*(s: string, max: int): string =
160	250	# third time this has come up in this port.
161	251	let t = s.strip()
162	252	if t.runeLen <= max: t else: t.runeSubStr(0, max - 1) & "…"
	253	+
	254	+requests.open()
	255	+answers.open()

  ## The Clojure has a fetch seam here because the two compilers disagreed about
  ## HTTP. Nim has one client, so the seam is gone and `fetch` simply asks.
--import std/[json, strutils, tables]
++import std/[exitprocs, json, strutils, tables]
  from std/unicode import runeLen, runeSubStr
  import frq/[atproto, trace]
      trace("profile", "could not fetch " & actor & ": " & e.msg)
      cache[actor] = Profile(status: psFailed)
++# ------------------------------------------------------------ in the background
++
++# Faces are wanted for everyone in a room at once, and a profile is an HTTPS
++# round trip each. Blocking was affordable for the panel — a reader presses a
++# face and waits — and is not affordable for a room of twelve, on the thread
++# that also answers every keystroke.
++#
++# So: a worker takes actors off one channel and puts answers on another, and
++# `collect` folds them into the cache on the thread that owns it. The cache
++# itself is never touched from two threads; only the channels are.
++
++var
++  requests: Channel[string]
++  answers: Channel[string]   ## "<actor>\x1f<json>", the json empty on failure
++  fetcher: Thread[void]
++  fetching: bool
++
++var stopping: bool
++
++proc fetcherBody() {.thread.} =
++  {.gcsafe.}:
++    while true:
++      let actor = requests.recv()
++      if actor.len == 0 or stopping: break
++      var body = ""
++      try:
++        body = $getProfile(actor)
++      except CatchableError as e:
++        trace("profile", "could not fetch " & actor & ": " & e.msg)
++      answers.send(actor & "\x1f" & body)
++
++proc want*(actor: string) =
++  ## Ask for a profile, without waiting for it.
++  ##
++  ## Once per identity for the run: `psLoading` goes in the cache here, so a
++  ## second ask for the same person while the first is in flight is not a
++  ## second round trip.
++  if actor.len == 0 or cache.hasKey(actor) or isAgent(actor): return
++  cache[actor] = Profile(status: psLoading)
++  if not fetching:
++    fetching = true
++    createThread(fetcher, fetcherBody)
++  requests.send(actor)
++
++proc stopFetching() =
++  ## Stop the worker and wait for it, at exit.
++  ##
++  ## Not tidiness: a thread that is still running when the process tears down
++  ## is a thread calling `newContext` after OpenSSL has been unloaded under
++  ## it, which is a SIGSEGV inside `net.newContext` reported against whatever
++  ## ran last. The test suite found it the first time this landed.
++  ##
++  ## The sentinel is an empty actor. A request already in flight finishes
++  ## first — at worst the HTTP timeout, and in practice the round trip that
++  ## was already nearly done.
++  if not fetching: return
++  stopping = true
++  requests.send("")
++  joinThread(fetcher)
++  fetching = false
++
++addExitProc(stopFetching)
++
++proc collect*(): bool =
++  ## Fold whatever has come back into the cache. True where anything did, so
++  ## a caller knows the screen has something new on it.
++  while true:
++    let (ok, msg) = answers.tryRecv()
++    if not ok: break
++    let sep = msg.find('\x1f')
++    if sep < 0: continue
++    let actor = msg[0 ..< sep]
++    let body = msg[sep + 1 .. ^1]
++    cache[actor] =
++      if body.len == 0: Profile(status: psFailed)
++      else:
++        try:
++          let j = parseJson(body)
++          if j{"did"}.getStr().len > 0: parseProfile(j)
++          else: Profile(status: psFailed)
++        except CatchableError: Profile(status: psFailed)
++    result = true
++
++proc avatarFor*(actor: string): string =
++  ## The face to paint for this identity, or "" where there is not one yet.
++  ## A lookup and never a fetch: this is called from the render path.
++  if actor.len == 0: return ""
++  let p = cache.getOrDefault(actor)
++  if p.status == psReady: p.avatar else: ""
++
  proc forgetProfiles*() =
    ## For a test that wants a known starting point.
    cache.clear()
    # third time this has come up in this port.
    let t = s.strip()
    if t.runeLen <= max: t else: t.runeSubStr(0, max - 1) & "…"
++
++requests.open()
++answers.open()

modified nim/src/frq/reducer.nim +11 -2 Viewed

@@ -524,7 +524,7 @@ proc dispatch*(event: JsonNode) =
524	524	# pressed a face and is already waiting.
525	525	# A `did:key:` agent has no Bluesky profile to fetch, and the panel
526	526	# says so rather than showing a failure it caused itself.
527		- if who.len > 0 and not isAgent(who): fetch(who)
	527	+ if who.len > 0 and not isAgent(who): want(who)
528	528
529	529	of "profile.close": app.profileViewing = ProfileView()
530	530
@@ -768,13 +768,19 @@ proc drain*() =
768	768	let real = p.params[^1]
769	769	let sp = real.find(' ') # the hop count comes first
770	770	let did = if sp >= 0: real[sp + 1 .. ^1].strip() else: ""
771		- if did.startsWith("did:"): app.dids[who] = did
	771	+ if did.startsWith("did:"):
	772	+ app.dids[who] = did
	773	+ # And their face, in the background. A room of twelve is twelve
	774	+ # HTTPS round trips, which is affordable on a thread of its own and
	775	+ # is not affordable here.
	776	+ want(did)
772	777
773	778	of "330":
774	779	# WHOIS's `<nick> <account> :is authenticated as`. The same DID by a
775	780	# different road — one nick rather than a room of them.
776	781	if p.params.len >= 3 and p.params[2].startsWith("did:"):
777	782	app.dids[p.params[1]] = p.params[2]
	783	+ want(p.params[2])
778	784
779	785	of "MODE":
780	786	# A channel MODE, for the letters that change how someone is listed.
@@ -805,6 +811,9 @@ proc drain*() =
805	811	else:
806	812	trace("skip", p.command & " " & $p.params)
807	813
	814	+ # Faces that have come back since the last frame.
	815	+ discard collect()
	816	+
808	817	# A line arriving moves the marker in the room being looked at, and closing
809	818	# the window is not a moment this client gets told about — so the saving
810	819	# happens as it goes, throttled, rather than at an end that may never come.

        # pressed a face and is already waiting.
        # A `did:key:` agent has no Bluesky profile to fetch, and the panel
        # says so rather than showing a failure it caused itself.
--      if who.len > 0 and not isAgent(who): fetch(who)
++      if who.len > 0 and not isAgent(who): want(who)
    of "profile.close": app.profileViewing = ProfileView()
          let real = p.params[^1]
          let sp = real.find(' ')       # the hop count comes first
          let did = if sp >= 0: real[sp + 1 .. ^1].strip() else: ""
--        if did.startsWith("did:"): app.dids[who] = did
++        if did.startsWith("did:"):
++          app.dids[who] = did
++          # And their face, in the background. A room of twelve is twelve
++          # HTTPS round trips, which is affordable on a thread of its own and
++          # is not affordable here.
++          want(did)
      of "330":
        # WHOIS's `<nick> <account> :is authenticated as`. The same DID by a
        # different road — one nick rather than a room of them.
        if p.params.len >= 3 and p.params[2].startsWith("did:"):
          app.dids[p.params[1]] = p.params[2]
++        want(p.params[2])
      of "MODE":
        # A channel MODE, for the letters that change how someone is listed.
      else:
        trace("skip", p.command & " " & $p.params)
++  # Faces that have come back since the last frame.
++  discard collect()
++
    # A line arriving moves the marker in the room being looked at, and closing
    # the window is not a moment this client gets told about — so the saving
    # happens as it goes, throttled, rather than at an end that may never come.

modified nim/src/frq/screens/chat.nim +4 -1 Viewed

@@ -148,7 +148,10 @@ proc messageBody(s: State, room: Room, m: Message, highlit: bool): Node =
148	148	m.frm)
149	149	let open = "profile.open:" & m.frm & ":" & senderActor
150	150	var row = hbox(%*{"spacing": 6},
151		- avatar(m.avatar, m.frm, size = faceSize, onClick = open),
	151	+ # From the profile cache rather than the message: a face belongs to a
	152	+ # person, not to a line they said, and a profile that arrives after
	153	+ # their first message should appear on all of them.
	154	+ avatar(avatarFor(senderActor), m.frm, size = faceSize, onClick = open),
152	155	n("button", %*{"label": m.frm, "kind": "plain", "onClick": open}))
153	156	if m.at > 0:
154	157	row.children.add dimLabel(clockTime(m.at))

        m.frm)
      let open = "profile.open:" & m.frm & ":" & senderActor
      var row = hbox(%*{"spacing": 6},
--      avatar(m.avatar, m.frm, size = faceSize, onClick = open),
++      # From the profile cache rather than the message: a face belongs to a
++      # person, not to a line they said, and a profile that arrives after
++      # their first message should appear on all of them.
++      avatar(avatarFor(senderActor), m.frm, size = faceSize, onClick = open),
        n("button", %*{"label": m.frm, "kind": "plain", "onClick": open}))
      if m.at > 0:
        row.children.add dimLabel(clockTime(m.at))

modified nim/tests/tsession.nim +26 -1 Viewed

@@ -10,7 +10,7 @@
10	10	## reads what went out. No socket at either end.
11	11
12	12	import std/[json, sequtils, strutils, tables, unittest]
13		-import frq/[cells, model, reducer, rooms]
	13	+import frq/[cells, model, profile, reducer, rooms]
14	14	import frq/conn as tr
15	15
16	16	proc sent(): seq[string] =
@@ -153,3 +153,28 @@ suite "opening a profile":
153	153	test "and what the message itself knew still wins":
154	154	dispatch(%*{"id": "profile.open:bob:did:plc:fromtheaccounttag"})
155	155	check app.profileViewing.actor == "did:plc:fromtheaccounttag"
	156	+
	157	+suite "faces":
	158	+ setup:
	159	+ reset()
	160	+ forgetProfiles()
	161	+
	162	+ test "learning a DID starts the face on its way, without waiting for it":
	163	+ # `want` is not `fetch`: a room of twelve is twelve HTTPS round trips,
	164	+ # and this is the thread that answers every keystroke.
	165	+ say(":irc.freeq.at 352 alice #freeq ~u freeq/plc/ngokl2gn irc.freeq.at " &
	166	+ "nandi.uk H :0 did:plc:ngokl2gnmpbvuvrfckja3g7p")
	167	+ let (p, known) = entry("did:plc:ngokl2gnmpbvuvrfckja3g7p")
	168	+ check known
	169	+ check p.status == psLoading
	170	+
	171	+ test "and nothing is painted for one that has not arrived":
	172	+ say(":irc.freeq.at 352 alice #freeq ~u freeq/plc/ngokl2gn irc.freeq.at " &
	173	+ "nandi.uk H :0 did:plc:ngokl2gnmpbvuvrfckja3g7p")
	174	+ check avatarFor("did:plc:ngokl2gnmpbvuvrfckja3g7p") == ""
	175	+
	176	+ test "an agent is never asked about":
	177	+ # `did:key:` has no Bluesky profile, so a request for one can only 400.
	178	+ say(":irc.freeq.at 352 alice #freeq ~u freeq/key/z6Mkp5we irc.freeq.at " &
	179	+ "cartographer H :0 did:key:z6Mkp5wegrxZR62h54HwR329yz7TJ8Ccx4sh")
	180	+ check not entry("did:key:z6Mkp5wegrxZR62h54HwR329yz7TJ8Ccx4sh")[1]

  ## reads what went out. No socket at either end.
  import std/[json, sequtils, strutils, tables, unittest]
--import frq/[cells, model, reducer, rooms]
++import frq/[cells, model, profile, reducer, rooms]
  import frq/conn as tr
  proc sent(): seq[string] =
    test "and what the message itself knew still wins":
      dispatch(%*{"id": "profile.open:bob:did:plc:fromtheaccounttag"})
      check app.profileViewing.actor == "did:plc:fromtheaccounttag"
++
++suite "faces":
++  setup:
++    reset()
++    forgetProfiles()
++
++  test "learning a DID starts the face on its way, without waiting for it":
++    # `want` is not `fetch`: a room of twelve is twelve HTTPS round trips,
++    # and this is the thread that answers every keystroke.
++    say(":irc.freeq.at 352 alice #freeq ~u freeq/plc/ngokl2gn irc.freeq.at " &
++        "nandi.uk H :0 did:plc:ngokl2gnmpbvuvrfckja3g7p")
++    let (p, known) = entry("did:plc:ngokl2gnmpbvuvrfckja3g7p")
++    check known
++    check p.status == psLoading
++
++  test "and nothing is painted for one that has not arrived":
++    say(":irc.freeq.at 352 alice #freeq ~u freeq/plc/ngokl2gn irc.freeq.at " &
++        "nandi.uk H :0 did:plc:ngokl2gnmpbvuvrfckja3g7p")
++    check avatarFor("did:plc:ngokl2gnmpbvuvrfckja3g7p") == ""
++
++  test "an agent is never asked about":
++    # `did:key:` has no Bluesky profile, so a request for one can only 400.
++    say(":irc.freeq.at 352 alice #freeq ~u freeq/key/z6Mkp5we irc.freeq.at " &
++        "cartographer H :0 did:key:z6Mkp5wegrxZR62h54HwR329yz7TJ8Ccx4sh")
++    check not entry("did:key:z6Mkp5wegrxZR62h54HwR329yz7TJ8Ccx4sh")[1]