From f6c9dd6fbbe1b6646e653b617c16591e59f8f3f0 Mon Sep 17 00:00:00 2001 From: Michael Black Date: Tue, 9 Jun 2026 15:50:26 -0500 Subject: [PATCH] claude feedback --- bskyweb/cmd/bskyweb/jsonld.go | 41 +++++++++++++++++++++++-- bskyweb/cmd/bskyweb/jsonld_test.go | 48 ++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/bskyweb/cmd/bskyweb/jsonld.go b/bskyweb/cmd/bskyweb/jsonld.go index 2f66177188..ca0f542545 100644 --- a/bskyweb/cmd/bskyweb/jsonld.go +++ b/bskyweb/cmd/bskyweb/jsonld.go @@ -143,6 +143,31 @@ func bskyPostURLFromATURI(handle, atURI string) string { return bskyPostURL(handle, parsed.RecordKey().String()) } +// bskyPostURLFromATURIWithDIDFallback returns the handle-form post URL +// when the handle is usable, otherwise falls back to the DID-form URL +// derived from the AT-URI's authority. Returns "" only when the AT-URI is +// unparseable or has no record key. Used for nested fields (e.g. +// VideoObject.embedUrl) where omitting on handle.invalid would weaken the +// emitted structured data. +func bskyPostURLFromATURIWithDIDFallback(handle, atURI string) string { + parsed, err := syntax.ParseATURI(atURI) + if err != nil { + return "" + } + rkey := parsed.RecordKey().String() + if rkey == "" { + return "" + } + if url := bskyPostURL(handle, rkey); url != "" { + return url + } + did := parsed.Authority().String() + if did == "" { + return "" + } + return fmt.Sprintf("https://bsky.app/profile/%s/post/%s", did, rkey) +} + // bskyProfileURL returns the canonical handle-form profile URL, or "" if // the handle is unusable. func bskyProfileURL(handle string) string { @@ -221,6 +246,8 @@ func buildVideoObject(pv *appbsky.FeedDefs_PostView, embedURL, postText string, Type: "VideoObject", ContentURL: v.Playlist, EmbedURL: embedURL, + // uploadDate uses IndexedAt (not record CreatedAt) for consistency + // with DiscussionForumPosting.datePublished on the parent post. UploadDate: pv.IndexedAt, } if v.Thumbnail != nil { @@ -437,6 +464,12 @@ func buildPostNode(pv *appbsky.FeedDefs_PostView, replies []*appbsky.FeedDefs_Th postURL := bskyPostURLFromATURI(pv.Author.Handle, pv.Uri) postText := postRecordText(pv) + // videoEmbedURL prefers the handle-form URL but falls back to DID-form + // for handle.invalid authors so VideoObject.embedUrl is never empty. + videoEmbedURL := postURL + if videoEmbedURL == "" { + videoEmbedURL = bskyPostURLFromATURIWithDIDFallback(pv.Author.Handle, pv.Uri) + } node := discussionForumPosting{ Type: "DiscussionForumPosting", @@ -446,7 +479,7 @@ func buildPostNode(pv *appbsky.FeedDefs_PostView, replies []*appbsky.FeedDefs_Th Text: postText, Image: images, ThumbnailURL: thumb, - Video: buildVideoObject(pv, postURL, postText, embedHidden), + Video: buildVideoObject(pv, videoEmbedURL, postText, embedHidden), DatePublished: pv.IndexedAt, InteractionStat: buildPostStats(pv), } @@ -511,6 +544,10 @@ func buildReplyNode(pv *appbsky.FeedDefs_PostView, hideLabels map[string]bool) c } postURL := bskyPostURLFromATURI(pv.Author.Handle, pv.Uri) postText := postRecordText(pv) + videoEmbedURL := postURL + if videoEmbedURL == "" { + videoEmbedURL = bskyPostURLFromATURIWithDIDFallback(pv.Author.Handle, pv.Uri) + } return comment{ Type: "Comment", URL: postURL, @@ -519,7 +556,7 @@ func buildReplyNode(pv *appbsky.FeedDefs_PostView, hideLabels map[string]bool) c Text: postText, Image: images, ThumbnailURL: thumb, - Video: buildVideoObject(pv, postURL, postText, embedHidden), + Video: buildVideoObject(pv, videoEmbedURL, postText, embedHidden), DatePublished: pv.IndexedAt, } } diff --git a/bskyweb/cmd/bskyweb/jsonld_test.go b/bskyweb/cmd/bskyweb/jsonld_test.go index 002cc74978..f8c6011fff 100644 --- a/bskyweb/cmd/bskyweb/jsonld_test.go +++ b/bskyweb/cmd/bskyweb/jsonld_test.go @@ -1337,3 +1337,51 @@ func TestBuildProfileJSONLD_HasPartVideo(t *testing.T) { t.Errorf("hasPart video embedUrl = %v", video["embedUrl"]) } } + +// handle.invalid authors must still produce a non-empty embedUrl on the +// VideoObject. The handle-form URL is unusable, so we fall back to the +// DID-form URL derived from the AT-URI authority. Without this fallback, +// omitempty drops embedUrl and Google's video indexer loses the canonical +// page reference. +func TestBuildPostJSONLD_VideoHandleInvalidEmbedURL(t *testing.T) { + playlist := "https://video.bsky.app/p.m3u8" + pv := makePostView("handle.invalid", "did:plc:alice", "abc123", "watch", + withVideoFull(videoEmbedOpts{ + playlist: playlist, alt: "scenic clip", + })) + canonical := "https://bsky.app/profile/did:plc:alice/post/abc123" + out, _ := buildPostJSONLD(pv, nil, canonical, hideEmbedLabels, hideReplyLabels) + main := unmarshalLD(t, out)["mainEntity"].(map[string]any) + video, ok := main["video"].(map[string]any) + if !ok { + t.Fatalf("video missing on handle.invalid post") + } + if video["embedUrl"] != canonical { + t.Errorf("embedUrl = %v, want %v", video["embedUrl"], canonical) + } + if video["contentUrl"] != playlist { + t.Errorf("contentUrl = %v, want %v", video["contentUrl"], playlist) + } +} + +// Same fallback applies to videos on replies whose author is handle.invalid. +func TestBuildPostJSONLD_VideoHandleInvalidEmbedURL_Reply(t *testing.T) { + pv := makePostView("alice.bsky.social", "did:plc:alice", "abc123", "main") + *pv.ReplyCount = 1 + playlist := "https://video.bsky.app/bob.m3u8" + reply := makePostView("handle.invalid", "did:plc:bob", "rep1", "watch", + withVideoFull(videoEmbedOpts{ + playlist: playlist, alt: "bob's clip", + })) + out, _ := buildPostJSONLD(pv, buildReplies(reply), "u", hideEmbedLabels, hideReplyLabels) + main := unmarshalLD(t, out)["mainEntity"].(map[string]any) + c := main["comment"].([]any)[0].(map[string]any) + video, ok := c["video"].(map[string]any) + if !ok { + t.Fatalf("reply video missing") + } + want := "https://bsky.app/profile/did:plc:bob/post/rep1" + if video["embedUrl"] != want { + t.Errorf("reply video embedUrl = %v, want %v", video["embedUrl"], want) + } +}