From e9384aa8fefc8491a6ba5b8fd3d3fd0def02ac1d Mon Sep 17 00:00:00 2001 From: Michael Black Date: Tue, 26 May 2026 16:10:36 -0500 Subject: [PATCH] keep comments concise --- bskyweb/cmd/bskyweb/embedmeta.go | 19 ++---- bskyweb/cmd/bskyweb/jsonld.go | 105 +++++++++++------------------ bskyweb/cmd/bskyweb/jsonld_test.go | 42 ++++-------- bskyweb/cmd/bskyweb/labels.go | 5 +- bskyweb/cmd/bskyweb/labels_test.go | 6 +- bskyweb/cmd/bskyweb/render_test.go | 40 ++++------- bskyweb/cmd/bskyweb/server.go | 44 +++++------- 7 files changed, 94 insertions(+), 167 deletions(-) diff --git a/bskyweb/cmd/bskyweb/embedmeta.go b/bskyweb/cmd/bskyweb/embedmeta.go index d54b20a2e5..84b8f8a4da 100644 --- a/bskyweb/cmd/bskyweb/embedmeta.go +++ b/bskyweb/cmd/bskyweb/embedmeta.go @@ -4,15 +4,12 @@ import ( appbsky "github.com/bluesky-social/indigo/api/bsky" ) -// Helpers for extracting Open Graph / Twitter Card metadata from post -// embeds. These produce data for og:* and twitter:* meta tags only — they -// are not used by the schema.org JSON-LD output (which lives in jsonld.go). -// -// The og:video tags exist as a separate code path because JSON-LD does not -// currently emit a VideoObject (deferred — requires `duration` from the -// appview that is not exposed today). +// Helpers for extracting Open Graph metadata from post embeds. These feed +// og:* / twitter:* meta tags only; the schema.org JSON-LD output lives in +// jsonld.go. og:video has its own path because JSON-LD does not yet emit +// VideoObject (deferred — needs `duration` from the appview). -// videoMeta is the metadata needed for og:video meta tags. +// videoMeta holds og:video meta tag data. type videoMeta struct { URL string Type string @@ -21,10 +18,8 @@ type videoMeta struct { HasSize bool } -// extractVideoMeta returns og:video metadata for the post if it has a video -// embed, otherwise the zero value. Respects the same embedHidden gate as -// extractPostMedia (in jsonld.go) so og:video and og:image suppression stay -// in sync. +// extractVideoMeta returns og:video metadata, or the zero value if there's +// no video embed. Respects embedHidden so og:video and og:image stay in sync. func extractVideoMeta(pv *appbsky.FeedDefs_PostView, embedHidden bool) videoMeta { if pv == nil || pv.Embed == nil || embedHidden { return videoMeta{} diff --git a/bskyweb/cmd/bskyweb/jsonld.go b/bskyweb/cmd/bskyweb/jsonld.go index 79d5d4ad40..269f5b7047 100644 --- a/bskyweb/cmd/bskyweb/jsonld.go +++ b/bskyweb/cmd/bskyweb/jsonld.go @@ -56,7 +56,7 @@ type discussionForumPosting struct { SharedContent *sharedContent `json:"sharedContent,omitempty"` } -// comment is the schema.org Comment shape used inside +// comment is the schema.org Comment shape used in // DiscussionForumPosting.comment[]. The comment property does not accept // DiscussionForumPosting, so replies map to Comment. type comment struct { @@ -85,24 +85,18 @@ type profilePage struct { HasPart []discussionForumPosting `json:"hasPart,omitempty"` } -// maxComments is the maximum number of top-level replies emitted in -// DiscussionForumPosting.comment[]. Bounded to keep SSR HTML payload small. +// maxComments caps DiscussionForumPosting.comment[] to keep SSR HTML small. const maxComments = 10 -// maxRecentPosts is the maximum number of recent posts emitted on a profile -// page in ProfilePage.hasPart[]. +// maxRecentPosts caps ProfilePage.hasPart[]. const maxRecentPosts = 10 -// authorFeedFetchLimit is how many entries to request from getAuthorFeed -// when populating ProfilePage.hasPart. We oversample because the -// posts_no_replies filter still returns reposts (which we drop client-side -// — only the author's own posts go in hasPart). A 3x oversample is a safe -// margin even for profiles that repost frequently. +// authorFeedFetchLimit oversamples getAuthorFeed because posts_no_replies +// still returns reposts; we drop those client-side. 3x is a safe margin. const authorFeedFetchLimit = 3 * maxRecentPosts -// bskyPostURL returns the canonical handle-form URL for a post, given the -// post's author handle and record key. Returns "" if the handle is unusable -// (empty or handle.invalid) or rkey is empty. +// bskyPostURL returns the canonical handle-form post URL, or "" if handle +// or rkey is unusable. func bskyPostURL(handle, rkey string) string { if handle == "" || handle == "handle.invalid" || rkey == "" { return "" @@ -110,9 +104,7 @@ func bskyPostURL(handle, rkey string) string { return fmt.Sprintf("https://bsky.app/profile/%s/post/%s", handle, rkey) } -// bskyPostURLFromATURI is a convenience wrapper for callers that hold an -// at-uri rather than a record key directly. Returns "" if the URI cannot be -// parsed. +// bskyPostURLFromATURI is bskyPostURL for callers holding an at-uri. func bskyPostURLFromATURI(handle, atURI string) string { parsed, err := syntax.ParseATURI(atURI) if err != nil { @@ -121,8 +113,8 @@ func bskyPostURLFromATURI(handle, atURI string) string { return bskyPostURL(handle, parsed.RecordKey().String()) } -// bskyProfileURL returns the canonical handle-form URL for a profile. -// Returns "" if the handle is unusable. +// bskyProfileURL returns the canonical handle-form profile URL, or "" if +// the handle is unusable. func bskyProfileURL(handle string) string { if handle == "" || handle == "handle.invalid" { return "" @@ -130,10 +122,9 @@ func bskyProfileURL(handle string) string { return fmt.Sprintf("https://bsky.app/profile/%s", handle) } -// extractPostMedia returns the image thumbnail URLs for the post (if any). -// All URLs are reused verbatim from the appview response — the same strings -// that go into og:image meta tags — so Google sees byte-identical media -// references. Callers derive thumbnailUrl from urls[0]. +// extractPostMedia returns thumbnail URLs for the post's image or video +// embed, byte-identical to what we put in og:image. Callers derive +// thumbnailUrl from urls[0]. func extractPostMedia(pv *appbsky.FeedDefs_PostView, embedHidden bool) []string { if pv == nil || pv.Embed == nil || embedHidden { return nil @@ -157,8 +148,7 @@ func extractPostMedia(pv *appbsky.FeedDefs_PostView, embedHidden bool) []string return nil } -// imageThumbs returns the thumb URLs for a slice of image embed views, or -// nil if the slice is empty. +// imageThumbs returns the thumb URLs, or nil if empty. func imageThumbs(images []*appbsky.EmbedImages_ViewImage) []string { if len(images) == 0 { return nil @@ -170,9 +160,8 @@ func imageThumbs(images []*appbsky.EmbedImages_ViewImage) []string { return urls } -// extractQuotedPostURL returns the canonical handle-form URL of a quoted -// post, if the embed is a viewable record (not blocked / detached / not -// found / non-post record like a feed generator or list). +// extractQuotedPostURL returns the canonical URL of a quoted post, or "" +// if the embed is blocked / not-found / detached / a non-post record. func extractQuotedPostURL(pv *appbsky.FeedDefs_PostView) string { if pv == nil || pv.Embed == nil { return "" @@ -188,14 +177,13 @@ func extractQuotedPostURL(pv *appbsky.FeedDefs_PostView) string { } vr := rec.Record.EmbedRecord_ViewRecord if vr == nil || vr.Author == nil { - // Skip _ViewBlocked, _ViewNotFound, _ViewDetached, and non-post records. return "" } return bskyPostURLFromATURI(vr.Author.Handle, vr.Uri) } -// extractSharedContentURL returns the URL of an external link embedded in -// the post (or in the media slot of a record-with-media embed). +// extractSharedContentURL returns the URL of an external link embed (also +// from the media slot of a record-with-media embed). func extractSharedContentURL(pv *appbsky.FeedDefs_PostView) string { if pv == nil || pv.Embed == nil { return "" @@ -212,9 +200,8 @@ func extractSharedContentURL(pv *appbsky.FeedDefs_PostView) string { return "" } -// buildAuthor constructs a Person object. Organization classification for -// custom-domain or organization-style accounts is a future enhancement; for -// now, every author is emitted as Person. +// buildAuthor constructs a Person. Organization classification for +// custom-domain accounts is a future enhancement. func buildAuthor(author *appbsky.ActorDefs_ProfileViewBasic) *personOrOrg { if author == nil { return nil @@ -236,9 +223,9 @@ func buildAuthor(author *appbsky.ActorDefs_ProfileViewBasic) *personOrOrg { return p } -// postEmbedHidden checks self-labels and post-view labels for any label that -// causes embeds to be omitted. Mirrors logic in WebPost handler so the -// JSON-LD shape stays consistent with og:image emission. +// postEmbedHidden reports whether any post-view label or self-label asks +// embeds to be omitted. WebPost uses this so og:image and JSON-LD media +// suppression stay in sync. func postEmbedHidden(pv *appbsky.FeedDefs_PostView, hideLabels map[string]bool) bool { if pv == nil { return false @@ -263,8 +250,8 @@ func postEmbedHidden(pv *appbsky.FeedDefs_PostView, hideLabels map[string]bool) return false } -// postRecordText returns the expanded post text (with shortened links -// expanded back to full URLs) or "" if the record is missing or malformed. +// postRecordText returns the post's expanded text, or "" if the record is +// missing or malformed. func postRecordText(pv *appbsky.FeedDefs_PostView) string { if pv == nil || pv.Record == nil { return "" @@ -276,10 +263,8 @@ func postRecordText(pv *appbsky.FeedDefs_PostView) string { return ExpandPostText(rec) } -// buildPostStats returns the standard like/comment/share interaction stat -// triple. CommentAction count uses ReplyCount to match what Google expects -// in InteractionCounter; commentCount is emitted separately on -// DiscussionForumPosting. +// buildPostStats returns the like / comment / share interaction triple. +// commentCount is emitted separately on DiscussionForumPosting. func buildPostStats(pv *appbsky.FeedDefs_PostView) []interactionStat { if pv == nil { return nil @@ -297,10 +282,10 @@ func buildPostStats(pv *appbsky.FeedDefs_PostView) []interactionStat { } } -// buildPostNode constructs a DiscussionForumPosting (in nested form, no -// @context, no envelope). Used both for top-level posts and for entries in -// hasPart / comment arrays. Returns the zero value if pv or pv.Author is nil -// — callers should treat that as "skip this entry". +// buildPostNode constructs a DiscussionForumPosting in nested form (no +// envelope, no @context). Used for top-level posts and for entries in +// hasPart / comment arrays. Returns the zero value if pv or pv.Author is +// nil; callers should treat that as "skip". func buildPostNode(pv *appbsky.FeedDefs_PostView, replies []*appbsky.FeedDefs_ThreadViewPost_Replies_Elem, hideLabels map[string]bool) discussionForumPosting { if pv == nil || pv.Author == nil { return discussionForumPosting{} @@ -349,7 +334,6 @@ func buildPostNode(pv *appbsky.FeedDefs_PostView, replies []*appbsky.FeedDefs_Th } reply := buildReplyNode(r.FeedDefs_ThreadViewPost.Post, hideLabels) if reply.Type == "" { - // nil-Author guard tripped; skip. continue } node.Comment = append(node.Comment, reply) @@ -358,8 +342,8 @@ func buildPostNode(pv *appbsky.FeedDefs_PostView, replies []*appbsky.FeedDefs_Th return node } -// buildReplyNode builds a schema.org Comment for a reply. Returns the zero -// value if pv or pv.Author is nil; callers should treat that as "skip". +// buildReplyNode builds a schema.org Comment for a reply. Returns the +// zero value if pv or pv.Author is nil. func buildReplyNode(pv *appbsky.FeedDefs_PostView, hideLabels map[string]bool) comment { if pv == nil || pv.Author == nil { return comment{} @@ -382,28 +366,21 @@ func buildReplyNode(pv *appbsky.FeedDefs_PostView, hideLabels map[string]bool) c } } -// buildPostJSONLD marshals the top-level WebPage envelope for a post page. -// This is what gets injected into , and a unicode character. + // Includes ", \, newline, , and a unicode char. tricky := "hello \"world\" \\ <\\>\n 🎉" pv := makePostView("alice.bsky.social", "did:plc:alice", "abc123", tricky) out, err := buildPostJSONLD(pv, nil, "u", hideEmbedLabels) if err != nil { t.Fatal(err) } - // Must round-trip through the JSON parser. main := unmarshalLD(t, out)["mainEntity"].(map[string]any) if main["text"] != tricky { t.Errorf("text round-trip failed: got %q want %q", main["text"], tricky) } - // Defensive: literal "" must not appear in the output, since - // that would break out of would break out of the script tag. if strings.Contains(out, "") { t.Errorf("output contains literal , would break HTML embedding") } @@ -322,9 +319,7 @@ func TestBuildPostJSONLD_Comments(t *testing.T) { pv := makePostView("alice.bsky.social", "did:plc:alice", "abc123", "main") *pv.ReplyCount = 14 - // 14 replies: 12 valid, 1 not-found, 1 blocked. The cap is maxComments=10 - // so we expect exactly 10 valid entries in comment[], with the - // non-thread variants filtered out. + // 12 valid replies + 1 not-found + 1 blocked. Cap is maxComments=10. const validReplies = 12 var replies []*appbsky.FeedDefs_ThreadViewPost_Replies_Elem for i := 0; i < validReplies; i++ { @@ -355,15 +350,12 @@ func TestBuildPostJSONLD_Comments(t *testing.T) { if len(comments) != maxComments { t.Errorf("expected %d comments (capped from %d valid), got %d", maxComments, validReplies, len(comments)) } - // First comment must be the first reply (FIFO order, not the not-found - // one which sits at the end). + // FIFO order: first valid reply, not the not-found at the end. first := comments[0].(map[string]any) if first["identifier"] != "at://did:plc:rep00/app.bsky.feed.post/reply00" { t.Errorf("first comment should be reply00, got %v", first["identifier"]) } - // Each comment must be schema.org Comment (not DiscussionForumPosting — - // that type is invalid for the comment property) and must NOT have - // nested comment[] / isBasedOn / sharedContent. + // Comment type, no nested comment[] / isBasedOn / sharedContent. for i, c := range comments { cm := c.(map[string]any) if cm["@type"] != "Comment" { @@ -390,9 +382,8 @@ func TestBuildPostJSONLD_HandleInvalidAuthor(t *testing.T) { out, _ := buildPostJSONLD(pv, nil, fallback, hideEmbedLabels) envelope := unmarshalLD(t, out) main := envelope["mainEntity"].(map[string]any) - // With no usable handle, mainEntity.url falls back to the canonical URL - // the caller provided (DID-form request URI), so the envelope URL and - // the post URL agree on a single string. + // mainEntity.url falls back to the caller's canonical URL so envelope + // and post URLs always agree. if main["url"] != fallback { t.Errorf("mainEntity.url should fall back to canonical URL, got %v", main["url"]) } @@ -403,21 +394,18 @@ func TestBuildPostJSONLD_HandleInvalidAuthor(t *testing.T) { t.Errorf("envelope.url and mainEntity.url disagree: %v vs %v", envelope["url"], main["url"]) } - // identifier (AT-URI) is still present and stable across handle changes. + // identifier (AT-URI) stays stable across handle changes. if main["identifier"] != pv.Uri { t.Errorf("identifier should still be the AT-URI, got %v", main["identifier"]) } - // Author URL is omitted (no usable handle to construct a profile URL). + // Author URL omitted (no usable handle). author := main["author"].(map[string]any) if _, present := author["url"]; present { t.Errorf("handle.invalid author should not produce author.url") } } -// TestBuildPostJSONLD_EnvelopeURLMatchesMainEntity asserts the P0.3 -// invariant that the WebPage envelope and the inner DiscussionForumPosting -// always carry the same URL — both for happy-path (handle-form canonical) -// and the handle.invalid fallback case. +// envelope.url and mainEntity.url must always agree. func TestBuildPostJSONLD_EnvelopeURLMatchesMainEntity(t *testing.T) { cases := []struct { name, handle, did, rkey, canonical string @@ -454,8 +442,7 @@ func TestBuildPostJSONLD_EnvelopeURLMatchesMainEntity(t *testing.T) { } func TestBuildPostJSONLD_NilAuthor(t *testing.T) { - // Defensive: appview is contractually required to send an Author, but we - // shouldn't panic if a malformed payload sneaks through. + // Defensive: don't panic if Author is nil. pv := makePostView("alice.bsky.social", "did:plc:alice", "abc123", "hi") pv.Author = nil if _, err := buildPostJSONLD(pv, nil, "u", hideEmbedLabels); err == nil { @@ -464,8 +451,7 @@ func TestBuildPostJSONLD_NilAuthor(t *testing.T) { } func TestBuildPostJSONLD_NilAuthorReply(t *testing.T) { - // A reply with a nil author should be silently dropped from comment[] - // rather than producing an entry with no @type. + // Reply with nil Author should be dropped, not emitted with empty @type. pv := makePostView("alice.bsky.social", "did:plc:alice", "abc123", "main") *pv.ReplyCount = 2 diff --git a/bskyweb/cmd/bskyweb/labels.go b/bskyweb/cmd/bskyweb/labels.go index 53860f88a2..4055d4b9c8 100644 --- a/bskyweb/cmd/bskyweb/labels.go +++ b/bskyweb/cmd/bskyweb/labels.go @@ -7,9 +7,8 @@ import ( // Helpers for inspecting profile / post labels. // profileRequiresAuth reports whether the profile has self-applied the -// `!no-unauthenticated` label, indicating the user only wants their content -// shown to signed-in viewers. SSR responses for these profiles must omit -// post text, descriptions, and other content beyond minimal identity. +// !no-unauthenticated label. SSR responses for these profiles omit post +// content beyond minimal identity. func profileRequiresAuth(pv *appbsky.ActorDefs_ProfileViewDetailed) bool { if pv == nil { return false diff --git a/bskyweb/cmd/bskyweb/labels_test.go b/bskyweb/cmd/bskyweb/labels_test.go index 20d4daaed1..d55e31b207 100644 --- a/bskyweb/cmd/bskyweb/labels_test.go +++ b/bskyweb/cmd/bskyweb/labels_test.go @@ -56,10 +56,8 @@ func TestProfileRequiresAuth(t *testing.T) { want: false, }, { - // Helper does not currently honor a Neg flag — matches existing - // inline behavior in WebPost / WebProfile / WebProfileRSS. If - // negation semantics are needed for self-labels, all four call - // sites need to change together. + // Negation isn't honored — matches prior inline behavior in + // WebPost / WebProfile / WebProfileRSS. name: "negated label still triggers (matches prior behavior)", pv: &appbsky.ActorDefs_ProfileViewDetailed{ Did: "did:plc:alice", diff --git a/bskyweb/cmd/bskyweb/render_test.go b/bskyweb/cmd/bskyweb/render_test.go index d4bb1464a6..0f9a069d92 100644 --- a/bskyweb/cmd/bskyweb/render_test.go +++ b/bskyweb/cmd/bskyweb/render_test.go @@ -11,19 +11,13 @@ import ( "github.com/flosch/pongo2/v6" ) -// renderTemplate executes a template by name with the given context and -// returns the rendered output. Uses the same renderer plumbing as the live -// server so we exercise the real template loader. -// -// post.html and profile.html both extend base.html, which {% include %}s -// templates/scripts.html — a file generated by the React/Vite web build. -// When that file is absent (e.g. running `go test` in a fresh checkout -// without a prior `yarn build-web`), template loading fails. Skip rather -// than fail in that case. +// renderTemplate executes a template using the live renderer and returns +// the output. base.html includes templates/scripts.html, which is generated +// by `yarn build-web`; skip the test if it's missing. func renderTemplate(t *testing.T, name string, ctx pongo2.Context) string { t.Helper() if _, err := bskyweb.TemplateFS.ReadFile("templates/scripts.html"); err != nil { - t.Skip("templates/scripts.html not present (run yarn build-web first); skipping render tests") + t.Skip("templates/scripts.html not present (run yarn build-web first)") } r := NewRenderer("templates/", &bskyweb.TemplateFS, false) tmpl, err := r.TemplateSet.FromCache(name) @@ -39,8 +33,7 @@ func renderTemplate(t *testing.T, name string, ctx pongo2.Context) string { var jsonLDRe = regexp.MustCompile(`(?s)`) -// extractJSONLD pulls out the body of the