fix(claw-server): keep the feedback invite until it is dismissed - #2767
Merged
Merged
Conversation
The card appeared once per installation and never again. Recording the impression spent the invitation, so the offer was consumed by the first paint of a new tab whether or not anyone read it, and most of a new tab's openings are incidental. Almost nobody ever saw it, and opening the booking page ended it too, which is not the same as having booked. Eligibility now ends only when the reader dismisses it. Dismissal gets its own column rather than being read off the outcome, because the two answer different questions: the outcome is the funnel's strongest claim about what the reader did, where a click outranks a later dismissal, while this is an instruction to stop showing the card that nothing outranks. A click arriving after a dismissal still raises the outcome and still leaves the card hidden. The migration carries existing dismissals into the new column, so anyone who already said no is not asked again. The impression event is now counted once per browser profile. The cockpit is the new tab page, so tracking every appearance would report thousands of impressions for one reader and leave the funnel without a usable denominator. The server is still told each time, since it holds the first-seen timestamp, and visibility remains its answer rather than the browser's.
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
✅ Tests passed: 2714/2717Ran 12 of 16 suites (4 not affected by this change).
|
…cross tabs The funnel outcome and the dismissal gate were two separate statements, so a crash between them could leave the outcome saying dismissed while the gate stayed null. Eligibility reads only the gate, so the card came back after a restart with no migration left to repair it. Both columns now move in one statement, each with its own guard: the outcome only rises by rank, the gate is only ever set. Dismissing in one tab left every other open cockpit tab still offering the card until each reloaded, which the card returning on every load turned from a corner case into the common one. A dismissal is now remembered locally as well, so the tabs already open stop showing it. That memory only ever suppresses: the server stays the authority on who is invited, so losing the key costs one more appearance and can never reveal an invitation the server refused. The impression marker was written before knowing the event had anywhere to go. Analytics readiness is settled by its own request, so the invitation can arrive first, and marking then dropped the event permanently and left that profile out of the count. The marker now waits until capture is live. The migration's choice about older rows is now stated where it is made. A reader who booked and then closed the card is stored as clicked, because a click outranks a later dismissal, and the old schema cannot tell them from someone who booked and never dismissed. Carrying every clicked row across as a dismissal would permanently silence the most engaged group, so the gap is accepted: it costs at most one more appearance.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The card appeared once per installation, ever — and then never again, whether or not anyone read it.
The cause was not the click handler. Recording the impression spent the invitation:
decide()refused as soon as a row existed, and the row was written the moment the card first painted. Since the cockpit is the new tab page and most new tabs are opened incidentally, the offer was consumed by a paint nobody looked at. That is why reach was so low.Opening the booking page ended it too, which is a separate mistake: clicking through to a booking page is not the same as having booked, and someone who means to finish later came back to nothing.
The fix
Eligibility now ends only when the reader dismisses it.
Why dismissal gets its own column
outcomealready carries a precedence rule where a click outranks a later dismissal, so that the funnel keeps the strongest claim about what the reader did. Reading "should I stop showing this" off the same field would let the card come back after someone asked it not to.So the two are separated.
outcomestays the funnel's answer;dismissed_at_msis the reader's instruction to stop, set once and never cleared. A click arriving after a dismissal still raises the outcome toclickedand still leaves the card hidden. A test covers exactly that combination.Migration
m0019adds the column and backfills it from rows already recorded asdismissed, so nobody who already said no gets asked again. That backfill has its own test, run against a database migrated only as far asm0018.The impression metric had to change with it
The card now returns on every cockpit load until dismissed. Tracking every appearance would report thousands of impressions for a single reader and leave the funnel without a usable denominator, so
feedback_invite_shownis counted once per browser profile.The split is deliberate: the browser deduplicates the analytics event, while the server is still told about every appearance because it holds the first-seen timestamp, and visibility stays the server's answer rather than the browser's. Losing that local key costs at most a repeated impression, never a missed invitation.
Tests
Server, 14 endpoint tests over the real router and 12 at the database layer:
Client, 10 tests driving a real render, including the new one confirming the card still shows on a later load while the impression is counted only once. I checked that test fails when the guard is removed.
cargo fmt,cargo clippy --workspace --all-targets -D warnings, the fullclaw-server-rustsuite and biome all pass.