From 5cb196940dd405038c33f3325e859b016b8114b8 Mon Sep 17 00:00:00 2001 From: SoXX Date: Tue, 2 Jun 2026 21:23:59 +0200 Subject: [PATCH] chore: remove SDK_ISSUES.md The umbrella audit (#4) had no open SDK issues remaining; the file is no longer load-bearing. --- SDK_ISSUES.md | 268 -------------------------------------------------- 1 file changed, 268 deletions(-) delete mode 100644 SDK_ISSUES.md diff --git a/SDK_ISSUES.md b/SDK_ISSUES.md deleted file mode 100644 index 79c6bdd..0000000 --- a/SDK_ISSUES.md +++ /dev/null @@ -1,268 +0,0 @@ -# SDK issues FA Droid - -Bugs whose root cause is in the **FA SDK** (`go-fa-api`, at -`/var/home/soxx/git/go-fa-api`, `replace`d in `go.mod`) not in this app. - -The SDK is a **separate module, maintained and fixed by a separate AI** -that does not see this app's code or conversation context. This file is the -handoff: every entry is a self-contained bug report meant to be copy-pasted -to that AI as-is. - -App-side bugs (frontend / go-service) live in `ISSUES.md`; fixed issues in -`SOLVED.md`. Features with UI but no backend are in `STUBS.md`; Wails/Android -stack traps in `PITFALLS.md`. - -**Status legend:** `[ ]` open · `[~]` diagnosed handoff brief drafted · -`[x]` fixed in the SDK (then moved to `SOLVED.md`) -**Numbering:** issue numbers are shared across `ISSUES.md`, `SOLVED.md` and -this file draw the next free number from the same sequence. - ---- - -## SDK handoff brief required fields - -Every entry MUST carry a handoff brief detailed enough to be copy-pasted to -the SDK AI as a standalone bug report. Do not write "sdk issue?" and stop — -that is not actionable for someone without this repo open. - -A handoff brief must answer all seven of: - -1. **SDK entry point** the exact exported function involved - (e.g. `Client.Fav(ctx, SubmissionID)` in `actions.go`). Name the file. -2. **How the app calls it** which `internal/services/*.go` wrapper - invokes it, and with what arguments. -3. **Observed behaviour** what the SDK call returns: an error (quote it - verbatim), a wrong value, or a silent no-op. -4. **Expected behaviour** what FurAffinity should do server-side and - what the SDK should return. -5. **Reproduction** concrete inputs (submission ID, username) and the - steps that trigger it. -6. **Suspected layer** HTML parsing (FA changed its markup?), request - shape (wrong form fields / missing CSRF / wrong URL), auth/cookies, or - rate-limiting. Point at the likely file: parsers are `*_parser.go`, - form posts are in `actions.go` / `*_post.go` / `*_send.go`. -7. **What is NOT the SDK** explicitly rule out the app layer so the SDK - AI does not chase a frontend bug. State what you verified. - -If you cannot yet fill all seven fields, the issue stays `[~]` and the brief -lists exactly which diagnostics are still needed never hand over a -half-brief as if it were complete. - ---- - -## Issues - -### #4 [~] Audit the FA SDK for bugs -- **Where:** `go-fa-api` (sibling module, `replace`d in `go.mod`). -- **Task:** Standing umbrella review the SDK surface used by - `internal/services/` for incorrect parsing, missing endpoints, or wrong - request shapes. -- **Cause tag:** `sdk` -- **Progress:** The first audit pass produced #1, #5, #9, #10 and #14 **all - now fixed and verified** (#1/#5/#9/#10 in app v0.0.7, #14 in v0.0.10; see - `SOLVED.md`). #15 was then fixed too (SDK gave `WithResolvedAvatars` a - count limit; app v0.0.14). #17 (priority rate limiting) was implemented too - — verified in app v0.0.16. **No open SDK issues currently.** Keep this - entry as the umbrella for future audit passes; file each concrete finding - as its own numbered issue. - -_No open SDK issues. Fixed ones are recorded in `SOLVED.md`._ - ---- - -## Add new SDK issues below - - - -### #18 [x] Rate limiter: upgrade from 2-level to N-level priority - -**Implemented in the SDK update** `options.go` now exposes `WithPriority` -and a multi-level `Priority` type; `WithPrioritizedRateLimiting` honours it. -FA Droid follow-up (assign tiers to reads/writes/preload/crawl) is tracked as -app-side work, not an SDK issue. - - -- **Component:** `go-fa-api` `ratelimit.go`, `options.go`. -- **Today:** the limiter has exactly two priorities. `WithBackgroundPriority(ctx)` - marks a request background; everything else is foreground. Background - requests wait until no foreground request is queued. Enabled via - `WithPrioritizedRateLimiting(true)`. -- **Need:** a consumer (FA Droid) wants *three+* tiers, not two: - 1. user-interactive (the page on screen, user write actions), - 2. speculative neighbor preload (likely-next submissions), - 3. bulk background crawl (inbox/watchlist warming). - With only two levels, neighbor preload must either compete with the live - page (tier 1) or interleave with the bulk crawl (tier 2) neither is right. -- **Proposed API:** keep `WithBackgroundPriority` working (maps to the lowest - level) and add `WithPriority(ctx, Priority)` where `Priority` is an ordered - enum, e.g. `PriorityInteractive`, `PriorityNormal` (default), - `PriorityLow`, `PriorityBackground`. The limiter serves waiting goroutines - highest-priority-first; the token emission rate is unchanged. -- **Compatibility:** default (no marker) must remain `PriorityNormal`; - `WithBackgroundPriority` must remain equivalent to `PriorityBackground`. -- **Why it matters to the app:** FA Droid will then assign tier 1 to - current-page reads + the write-action queue worker, tier `PriorityLow` to - neighbor preload, and `PriorityBackground` to the inbox crawler. - -### #21 [x] GetSubmission doesn't expose the viewer's favorite state - -- **Where:** `go-fa-api` `submission.go` (`Submission` struct + `parseSubmission`). -- **Symptom:** A submission the logged-in user has favorited shows the - favorite heart as *empty* on the app's submission detail page. There is no - way to know a submission's favorite state from a `GetSubmission` result. -- **Expected:** `GetSubmission` should report whether the authenticated viewer - has favorited the submission, so the UI can render the correct heart state. -- **Cause tag:** `sdk` -- **SDK handoff brief:** - 1. **Entry point:** `Client.GetSubmission(ctx, SubmissionID)` in - `submission.go`, which builds the result via `parseSubmission(id, doc)`. - The returned `Submission` struct (`submission.go:17-38`) has fields - ID, Title, Author, PostedAt, Rating, Category, Type, Species, Gender, - Description, DescriptionText, Tags, FileURL, ThumbURL, Width, Height, - Stats, Folders, Prev, Next and **nothing indicating favorite state**. - 2. **How the app calls it:** `SubmissionService.getCached` in - `internal/services/submission.go` calls `GetSubmission`, then - `dto.FromSubmission` (`internal/dto/types.go`) copies the struct to the - wire DTO. The frontend (`SubmissionView.svelte`) does - `favorited = !!sub.favorited`. - 3. **Observed behaviour:** `GetSubmission` returns a `*Submission` with no - favorite-state field. It is not a wrong value or an error the datum is - simply absent from the SDK's public type. - 4. **Expected behaviour:** When `/view/{id}/` is fetched with valid `a`/`b` - cookies, FA renders either a `+Fav` (`/fav/{id}/...`) or a `−Fav` - (`/unfav/{id}/...`) anchor exactly one, matching the viewer's current - state. The SDK should surface this on `Submission`, e.g. a new - `Favorited bool` field (final name your call), set true when the page - shows the `/unfav/` link. On an anonymous (no-cookie) fetch neither link - is present → `Favorited` false, which is correct. - 5. **Reproduction:** With cookies set, favorite submission X on FA, then - call `GetSubmission(ctx, X)` the result cannot express that X is - favorited. - 6. **Suspected layer:** HTML parsing `parseSubmission`. The SDK *already* - has the exact parser: `findFavLinks(doc, subID) (favURL, unfavURL string)` - in `actions.go` (used by `toggleFavorite` for its idempotency check). - `unfavURL != ""` means "currently favorited." `parseSubmission` just needs - to run that check and set the new field. No new scraping logic required. - 7. **What is NOT the SDK:** The app side is verified ready and correct. - `SubmissionView.svelte` already reads `sub.favorited` and types it - (`favorited?: boolean`); `getCached` correctly busts and re-fetches the - submission cache after a fav write (via `WriteService` / the action - queue). The value never reaches the app only because the SDK `Submission` - struct has no field to carry it `dto.FromSubmission` has nothing to map. -- **Related (please also check):** the same gap likely exists for *watch* - state `findWatchLinks` exists in `actions.go`, but the `User` struct may - not expose whether the viewer watches that user. Worth fixing in the same - pass. -- **When it lands (FA Droid follow-up):** add `Favorited bool` to - `dto.Submission` (`json:"favorited"`), map it in `dto.FromSubmission`, and - regenerate bindings. The frontend already consumes `sub.favorited`, so no UI - change is needed. -- **Status:** done SDK update added `Submission.Favorited`; app wired it in - v0.0.22. - -### #23 [ ] SubmissionInbox yields only the first page (~72 items) - -- **Where:** `go-fa-api` `inbox.go` (`SubmissionInbox` + `parseSubmissionInboxPage`). -- **Symptom:** The new-submission inbox shows only ~72 items even when the - account has thousands pending. A user with ~4718 inbox submissions sees one - page. -- **Expected:** `SubmissionInbox` should walk every cursor page until FA stops - rendering a "Next 72" link. -- **Cause tag:** `sdk` -- **SDK handoff brief:** - 1. **Entry point:** `Client.SubmissionInbox(ctx, ListOptions)` in `inbox.go`, - whose iterator follows `parseSubmissionInboxPage`'s returned `nextURL`. - 2. **How the app calls it:** `InboxService.StreamSubmissions` - (`internal/services/inbox.go`) runs one goroutine that does - `for sub, err := range client.SubmissionInbox(ctx, fa.ListOptions{})` - — `ListOptions{}` means `MaxPages: 0` (unbounded), so the app does not - cap the crawl. - 3. **Observed behaviour:** the `range` completes after ~72 items the - iterator yields one page then ends. Verified on-device: the app's crawl - channel (fed one item per `yield`) closes after exactly one ~72-item - chunk, so `StreamSubmissions` reports `HasMore: false` immediately after - page 1. - 4. **Expected behaviour:** FA's `/msg/submissions/` paginates via a - "Next 72" link encoding a from-id cursor; the iterator should follow it - across all ~66 pages for a 4718-item inbox. - 5. **Reproduction:** logged-in client with a large submission inbox; - `count := 0; for range client.SubmissionInbox(ctx, ListOptions{}) { count++ }` - yields ~72, not the true total. - 6. **Suspected layer:** HTML parsing `parseSubmissionInboxPage`. Its next- - cursor selector is `div.messagecenter-navigation a.button.more`; if FA - changed that markup the parser returns `nextURL == ""` and the iterator - stops after page 1. Check the selector against current `/msg/submissions/` - HTML (and the cursor-URL construction). - 7. **What is NOT the SDK:** app side verified. `StreamSubmissions` ranges the - iterator fully on a single goroutine and streams everything it yields; it - passes `ListOptions{}` (no `MaxPages` cap). On-device logging shows the - crawl ends after one chunk the iterator simply stops yielding. - -### #24 [x] Request logger drops `context.Context` (breaks trace propagation) - -**Fixed in the SDK** `transport.go`'s `logRequest` now emits its record via -`logger.InfoContext(req.Context(), "fa.request", …)` instead of `logger.Info`, -so a context-aware `slog.Handler` recovers the caller's active span and the -HTTP span nests under the RPC span. A regression test, -`TestTransport_LogRequest_PropagatesRequestContext` in `transport_test.go`, -installs a context-capturing handler and asserts a sentinel value threaded -through `req.Context()` reaches the slog record guarding against a silent -revert to `Info`. - - -- **Where:** `go-fa-api` the HTTP transport's request logging (`transport.go`, - the `logRequest` helper / wherever `slog` records an outgoing request). -- **Type:** Enhancement, not a bug the SDK works correctly; this is a - one-line change the app needs for distributed tracing. -- **Symptom:** The app (FA Droid, WI-10) added OpenTelemetry spans. An app RPC - opens an `rpc` span and threads its `context.Context` into the SDK call. The - SDK's per-request `slog` line is currently emitted with `logger.Info(...)`, - which carries **no context** so the app's `slog` handler cannot recover the - active span, and each HTTP span becomes an unparented root instead of a child - of the RPC span. -- **Expected:** the request log record should carry the request's context so a - context-aware `slog.Handler` can read the active span from it. -- **The fix (one line):** change the request-logging call from - `logger.Info("fa.request", …)` to - `logger.InfoContext(req.Context(), "fa.request", …)` (use the `*http.Request`'s - own `Context()`). `slog` already supports `InfoContext`; no API change, no new - dependency. -- **Cause tag:** `sdk` -- **SDK handoff brief:** - 1. **SDK entry point:** the HTTP transport `RoundTrip` / request path in - `transport.go` specifically the `slog` call that logs each outgoing FA - request (`logRequest`, or inline). All SDK client methods route through it. - 2. **How the app calls it:** every `internal/services/*.go` wrapper calls a - `Client.*` method with a `context.Context` that now carries an OTel span; - that ctx reaches the `*http.Request` (`req.Context()`). - 3. **Observed behaviour:** the request `slog` record is created without a - context, so `Handler.Handle` receives `context.Background()`. - 4. **Expected behaviour:** the record carries `req.Context()`, so a - context-aware handler can extract the active span. - 5. **Reproduction:** with WI-10's `diag` slog handler installed, every HTTP - span has `parentSpanId == ""` even when the call was made inside an RPC - span. After the `InfoContext` change, the HTTP span nests under the RPC - span on the same trace id. - 6. **Suspected layer:** request shape / logging purely the logging call, - no parsing or auth involved. - 7. **What is NOT the SDK behaviour-wise:** no functional SDK behaviour - changes request execution, parsing, retries, rate-limiting are all - untouched. This only changes which `slog` method is used so context flows. -- **Note:** WI-10 applied this change to the *local* `replace`d working copy of - `go-fa-api` so FA Droid v0.0.33 has working rpc→http span linkage. It is - **not committed in the SDK repo**. Until it is upstreamed, a clean SDK - checkout will degrade gracefully HTTP spans become valid but unparented - roots (no crash, no data loss, only the rpc↔http link is lost).