fix: forward feed context to PostCard on the profile (#48) #104

Open

starsetbyte wants to merge 4 commits from fix/profile-feed-context-attribution into main

pull from: fix/profile-feed-context-attribution

merge into: starsetbyte:main

starsetbyte:main

starsetbyte:docs/repo-cleanup-pass

starsetbyte:feat/moderation

starsetbyte:worktree-feat+moderation

Conversation 2 Commits 4 Files changed 15 +805 -34

starsetbyte commented 2026-07-26 12:48:19 +00:00

Owner

Copy link

Closes #48.

What was wrong

ProfileRoute's PostCard call never passed repostedBy or replyParentAuthor. Both default to null in PostCard, and the "Replying to X" / "Reposted by X" lines only render when non-null — so on the profile a reply was indistinguishable from a top-level post, and the author's own reposts rendered as posts by the original author.

That is the whole of the "Replies tab looks identical to Posts" report.

What was not wrong

The plumbing, which was my first suspicion and is worth writing down so nobody re-investigates it:

• ProfileTab.REPLIES sends posts_with_replies through authorFeedType into getAuthorFeed correctly.
• Each filter gets its own Room key, and the composite (feedType, uri) primary key keeps the tabs from evicting one another.
• The data already reached TimelineItem — FeedMappingTest has covered that round-trip since Phase 3.5.

FeedRoute passed both fields all along. ProfileRoute was the sole consumer that dropped them.

Expectation note

posts_with_replies is a superset of posts_no_replies, so the Replies tab legitimately still contains every row the Posts tab shows, plus replies. Replies are now visually marked, which may make that read fine as-is. Whether to relabel the tab, or move it to posts_and_author_threads, is deliberately left open here.

Tests

Two contract tests, both failing before the fix:

• profile post rows forward reply parent attribution to PostCard
• profile post rows forward repost attribution to PostCard

They scope their assertions to the PostCard( ... ) argument block by paren balancing, so they cannot be satisfied by the identifier appearing anywhere else in the file.

Suite: 712/712 green (was 710; these two are the delta).

Caveats for review

• These are source-inspection tests, matching the repo's existing contract-test idiom. They pin the call site but do not prove the rendered output. A real render assertion would need a PostCardGeometryDeviceTest-style androidTest, which CI cannot run — buildserv has no KVM.
• I ran testDebugUnitTest locally. assembleDebug and lint are unverified on my side; the main sources did compile as part of the test run, so a compile break is unlikely, but lint is genuinely untested until CI reports.
• Not yet confirmed on a device.

Still open, not addressed here

parseAuthorFeedType (FeedRemoteMediator.kt:102) remains untested — covering it means widening private to internal. I left it alone because the round-trip is not broken, so a test there would pass on first run: regression coverage, not TDD.

Closes #48. ## What was wrong `ProfileRoute`'s `PostCard` call never passed `repostedBy` or `replyParentAuthor`. Both default to `null` in `PostCard`, and the "Replying to X" / "Reposted by X" lines only render when non-null — so on the profile a reply was indistinguishable from a top-level post, and the author's own reposts rendered as posts by the original author. That is the whole of the "Replies tab looks identical to Posts" report. ## What was *not* wrong The plumbing, which was my first suspicion and is worth writing down so nobody re-investigates it: - `ProfileTab.REPLIES` sends `posts_with_replies` through `authorFeedType` into `getAuthorFeed` correctly. - Each filter gets its own Room key, and the composite `(feedType, uri)` primary key keeps the tabs from evicting one another. - The data already reached `TimelineItem` — `FeedMappingTest` has covered that round-trip since Phase 3.5. `FeedRoute` passed both fields all along. `ProfileRoute` was the sole consumer that dropped them. ## Expectation note `posts_with_replies` is a *superset* of `posts_no_replies`, so the Replies tab legitimately still contains every row the Posts tab shows, plus replies. Replies are now visually marked, which may make that read fine as-is. Whether to relabel the tab, or move it to `posts_and_author_threads`, is deliberately left open here. ## Tests Two contract tests, both failing before the fix: - `profile post rows forward reply parent attribution to PostCard` - `profile post rows forward repost attribution to PostCard` They scope their assertions to the `PostCard( ... )` argument block by paren balancing, so they cannot be satisfied by the identifier appearing anywhere else in the file. Suite: **712/712 green** (was 710; these two are the delta). ## Caveats for review - These are source-inspection tests, matching the repo's existing contract-test idiom. They pin the call site but do not prove the rendered output. A real render assertion would need a `PostCardGeometryDeviceTest`-style androidTest, which CI cannot run — buildserv has no KVM. - I ran `testDebugUnitTest` locally. `assembleDebug` and `lint` are unverified on my side; the main sources did compile as part of the test run, so a compile break is unlikely, but lint is genuinely untested until CI reports. - Not yet confirmed on a device. ## Still open, not addressed here `parseAuthorFeedType` (`FeedRemoteMediator.kt:102`) remains untested — covering it means widening `private` to `internal`. I left it alone because the round-trip is not broken, so a test there would pass on first run: regression coverage, not TDD.

starsetbyte added 1 commit 2026-07-26 12:48:19 +00:00

fix: forward feed context to PostCard on the profile (#48) ... c49821e62a

ProfileRoute's PostCard call never passed repostedBy or replyParentAuthor.
Both default to null in PostCard, and the "Replying to X" / "Reposted by X"
lines only render when non-null — so on the profile a reply was
indistinguishable from a top-level post, and the author's own reposts
rendered as posts by the original author.

This was the whole of the "Replies tab looks identical to Posts" report.
The plumbing was already correct: ProfileTab.REPLIES sends
posts_with_replies through authorFeedType into getAuthorFeed, each filter
gets its own Room key, and the composite (feedType, uri) primary key keeps
the tabs from evicting each other. The data also already reached
TimelineItem — FeedMappingTest has covered that round-trip since Phase 3.5.
Only the render call site dropped it. FeedRoute passed both fields all
along; ProfileRoute was the sole consumer that did not.

Note that posts_with_replies is a superset of posts_no_replies, so the
Replies tab legitimately still contains the Posts tab's rows. Whether to
relabel the tab is left open.

Tests: two contract tests scope their assertions to the PostCard( ... )
argument block by paren balancing, so they cannot be satisfied by the
identifier appearing elsewhere in the file. Both failed before the fix.
Suite is 712/712 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQ56Mh1F6otBp3a4pRWm5Y

starsetbyte added 1 commit 2026-07-26 13:41:08 +00:00

feat: narrow the profile Replies tab to authored replies (#48) ... 04fe05af55

The Replies tab asked for posts_with_replies and rendered the result
verbatim. That filter is a superset — the author's posts, their replies and
their reposts — so the tab read as a near-duplicate of Posts. The lexicon
has no replies-only filter (posts_with_replies, posts_no_replies,
posts_with_media, posts_and_author_threads are the options), so the
narrowing has to happen client-side.

isAuthoredReply keys off post.record.reply, not the feed-level
replyParentAuthor. The latter resolves to null when the parent was deleted
or blocked — FeedMappingTest has covered that case since Phase 3.5 — so a
predicate built on it would have silently hidden replies to deleted posts.
Reposts are excluded even when the reposted post is itself a reply, since
those are someone else's words.

Filtering at the persist step needs care: a page with no replies writes no
rows, nothing invalidates the PagingSource, and Paging never asks for
another append — the tab stalls empty. collectAuthoredReplies keeps pulling
pages until a reply lands or the cursor runs out, capped at 5 pages so an
account with thousands of non-reply posts cannot spin inside one load. It
returns the cursor even when nothing survived, so the next append resumes
rather than re-reading the same pages.

Known limitation: an author whose most recent ~5 pages contain no replies
at all will see an empty tab, and pull-to-refresh rescans the same range
rather than advancing. That is the cost of the cap; raising it trades
latency for reach.

The collector takes a page-fetcher lambda, so the loop is unit-testable
without a Room instance — no androidTest needed, which matters because CI
cannot run instrumented tests (buildserv has no KVM).

Suite 724/724 green, lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQ56Mh1F6otBp3a4pRWm5Y

starsetbyte commented 2026-07-26 13:41:25 +00:00

Author

Owner

Copy link

Follow-up commit 04fe05a resolves the open question in the PR body.

Confirmed on a device build of c49821e: reply rows do now show "Replying to X", so the attribution fix landed. But the tab still listed standalone posts and reposts, because posts_with_replies is a superset and the first commit only changed labelling, never row composition.

Decision was to make the tab mean "replies only, no reposts" rather than relabel it. isAuthoredReply keys off post.record.reply instead of the feed-level replyParentAuthor, because the latter is null for deleted/blocked parents and would have silently hidden those replies.

The non-obvious part is that filtering at the persist step can stall the tab: a page with no replies writes no rows, so the PagingSource never invalidates and Paging never requests another append. collectAuthoredReplies handles that with a bounded rescan.

Known limitation, worth a reviewer's eye: an author whose most recent ~5 pages hold no replies sees an empty tab, and pull-to-refresh rescans the same range rather than advancing. That is inherent to the cap. The alternative considered was persisting an isReply column (schema v13, additive) and filtering in the DAO, which avoids the stall entirely because every row still gets written — rejected as too much surface area for a display concern, but it is the better shape if this limitation bites.

Suite 724/724, lint clean locally.

Follow-up commit `04fe05a` resolves the open question in the PR body. Confirmed on a device build of `c49821e`: reply rows do now show "Replying to X", so the attribution fix landed. But the tab still listed standalone posts and reposts, because `posts_with_replies` is a superset and the first commit only changed labelling, never row composition. Decision was to make the tab mean "replies only, no reposts" rather than relabel it. `isAuthoredReply` keys off `post.record.reply` instead of the feed-level `replyParentAuthor`, because the latter is null for deleted/blocked parents and would have silently hidden those replies. The non-obvious part is that filtering at the persist step can stall the tab: a page with no replies writes no rows, so the PagingSource never invalidates and Paging never requests another append. `collectAuthoredReplies` handles that with a bounded rescan. **Known limitation, worth a reviewer's eye:** an author whose most recent ~5 pages hold no replies sees an empty tab, and pull-to-refresh rescans the same range rather than advancing. That is inherent to the cap. The alternative considered was persisting an `isReply` column (schema v13, additive) and filtering in the DAO, which avoids the stall entirely because every row still gets written — rejected as too much surface area for a display concern, but it is the better shape if this limitation bites. Suite 724/724, lint clean locally.

starsetbyte added 1 commit 2026-07-26 13:52:31 +00:00

refactor: select profile replies in the DAO instead of at the write ... 83be2433b1

Replaces the filter-on-write approach from 04fe05a. That version dropped
non-replies before persisting, which meant a page with no replies wrote no
rows, never invalidated the PagingSource, and left the tab stalled — so it
needed a bounded rescan loop and a page cap, and still showed an empty tab
for an author whose recent pages held no replies. The loop was scaffolding
holding up a constraint that only existed because of where the filter sat.

Now every fetched row is persisted as before and the Replies tab reads
through a narrowing query. Nothing invalidates less often than it used to,
so Paging behaves normally: no loop, no cap, no empty-tab case.

posts.isReply (schema v13, additive) records whether the post record carries
a reply ref. It is a fact about the post, not about a tab — the tab's rule
lives in PostDao.getRepliesForFeed as `isReply = 1 AND repostedBy IS NULL`,
reusing the already-persisted repost attribution rather than adding a second
column. A repost of someone else's reply is their words, so it is excluded.

isReply is derived from post.record.reply, not from the feed-level
replyParentAuthor, which is null when the parent was deleted or blocked;
deriving it from that would have silently hidden replies to deleted posts.

Two things found while landing this:

- An intermediate build regenerated the committed 12.json with an isReply
  column, because the entity gained the field before the version was bumped.
  A v12 schema claiming a column v12 never had would make
  runMigrationsAndValidate check against a fiction. Restored, and
  SchemaV13ContractTest now asserts v12 does not carry the column.
- The "database version matches the newest committed schema" assertion was
  duplicated into every schema contract test, so each bump meant editing all
  of them. Consolidated into the newest test.

The androidTest migration walk now runs v8 to v13 and asserts existing rows
backfill isReply as 0 rather than NULL. It cannot run in CI — buildserv has
no KVM — so the query itself is additionally pinned by a source-level
contract test.

Suite 723/723 green, lint clean, assembleDebug clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQ56Mh1F6otBp3a4pRWm5Y

starsetbyte added 1 commit 2026-07-26 13:52:54 +00:00

docs: record schema v13 and the read-side replies rule ... 1fa794b193

CLAUDE.md pinned the schema at v12 in two places. Also notes why the Replies
tab narrows on read rather than on write, since the write-side version is the
one that looks obvious and stalls the tab.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQ56Mh1F6otBp3a4pRWm5Y

starsetbyte commented 2026-07-26 13:53:10 +00:00

Author

Owner

Copy link

83be243 replaces the filter-on-write approach from 04fe05a. Please review the branch as a whole rather than commit by commit — the middle commit is an approach that was reconsidered.

Supersedes the limitation flagged in my previous comment. The empty-tab case, the rescan loop and the page cap are all gone. They were scaffolding holding up a constraint that only existed because the filter sat before the write: a page with no replies wrote no rows, so the PagingSource never invalidated and Paging never asked for more.

Every row is now persisted as before and the tab reads through a narrowing query. posts.isReply (schema v13, additive) is a fact about the post; the tab's rule lives in PostDao.getRepliesForFeed as isReply = 1 AND repostedBy IS NULL, reusing the already-persisted repost attribution rather than adding a second column.

Two things worth a reviewer's attention:

1. 12.json was briefly clobbered. An intermediate build regenerated the committed v12 schema with an isReply column, because the entity gained the field before the version was bumped. A v12 schema claiming a column v12 never had would make runMigrationsAndValidate check against a fiction. Restored, and SchemaV13ContractTest now asserts v12 does not carry the column, so the same slip fails loudly next time. Worth confirming git log -p app/schemas/ looks clean to you.

2. The tab's rule is SQL, which CI cannot execute. The androidTest migration walk now covers v8→v13 and asserts existing rows backfill isReply as 0 rather than NULL, but it needs an emulator and buildserv has no KVM. The clauses are additionally pinned by a source-level contract test — that catches a silent loosening of the rule, not a semantic error in the SQL. If you can run the androidTest locally against a device, that is the gap.

Also consolidated the "database version matches the newest committed schema" assertion, which had been duplicated into every schema contract test, so each bump meant editing all of them.

Suite 723/723, lint clean, assembleDebug clean. Still unverified on a device.

`83be243` replaces the filter-on-write approach from `04fe05a`. Please review the branch as a whole rather than commit by commit — the middle commit is an approach that was reconsidered. **Supersedes the limitation flagged in my previous comment.** The empty-tab case, the rescan loop and the page cap are all gone. They were scaffolding holding up a constraint that only existed because the filter sat before the write: a page with no replies wrote no rows, so the PagingSource never invalidated and Paging never asked for more. Every row is now persisted as before and the tab reads through a narrowing query. `posts.isReply` (schema v13, additive) is a fact about the post; the tab's rule lives in `PostDao.getRepliesForFeed` as `isReply = 1 AND repostedBy IS NULL`, reusing the already-persisted repost attribution rather than adding a second column. Two things worth a reviewer's attention: 1. **`12.json` was briefly clobbered.** An intermediate build regenerated the committed v12 schema with an `isReply` column, because the entity gained the field before the version was bumped. A v12 schema claiming a column v12 never had would make `runMigrationsAndValidate` check against a fiction. Restored, and `SchemaV13ContractTest` now asserts v12 does *not* carry the column, so the same slip fails loudly next time. Worth confirming `git log -p app/schemas/` looks clean to you. 2. **The tab's rule is SQL, which CI cannot execute.** The androidTest migration walk now covers v8→v13 and asserts existing rows backfill `isReply` as 0 rather than NULL, but it needs an emulator and buildserv has no KVM. The clauses are additionally pinned by a source-level contract test — that catches a silent loosening of the rule, not a semantic error in the SQL. If you can run the androidTest locally against a device, that is the gap. Also consolidated the "database version matches the newest committed schema" assertion, which had been duplicated into every schema contract test, so each bump meant editing all of them. Suite 723/723, lint clean, assembleDebug clean. Still unverified on a device.

All checks were successful

Hide checks

Peregrine CI / Build & Test (JDK 17) (pull_request) Successful in 5m14s

Details

Peregrine CI / Signed Minified Release Build (pull_request) Successful in 4m17s

Details

Peregrine CI / Instrumented Tests (API 29+) (pull_request) Has been skipped

Details

peregrine-ci / assembleDebug Build succeeded (3m46s)

Required

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/profile-feed-context-attribution:fix/profile-feed-context-attribution

git switch fix/profile-feed-context-attribution

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

auth

Authentication and OAuth

  

bug

Something is broken

  

composer

Post composition and drafts

  

correctness

Logic and state correctness

  

database

Room, migrations, persistence

  

durability

Data safety, persistence, recovery

  

enhancement

New feature or request

  

feeds

Feed management and preferences

  

interactions

Like, repost, reply interactions

  

notifications

Notification system

  

performance

Performance issues

  

reliability

Resilience against interruptions

  

safety

User safety and trust

  

security

Security concerns

No labels

auth

bug

composer

correctness

database

durability

enhancement

feeds

interactions

notifications

performance

reliability

safety

security

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

starsetbyte/peregrine!104

No description provided.