Abdul Samad

Posted on Aug 24

Giving back to my favorite Bookmarks manager - making it better for everyone

#devchallenge #bugsmash #opensource #gemini

Summer Bug Smash: Clear the Lineup 🐛🛹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

This is a submission for DEV's Summer Bug Smash: Smash Stories powered by Sentry.

About the Software

I had used Raindrop bookmark manager extensively ever since starting college, with the applicaiton eating links and images across my devices. I always loved the simplicity, speed and desktop apps.
Now, those who deal with bookmarks would know, as the volume piles up, the classification not so much.
After some time, its save and forget, never to come back. Never to be tagged. and soon the app was only installed on phone, where it was used to save links from android apps.
.

The Background

When I first got to know about this bugsmash challenge, I was like I only have a few decent projects, and how can there be bug that is sitting nicely in the code waiting to be solved. It was actually amusing trying to revisit the long forgotten repos and thinking about any fix or optimization I could do.
The focus shifted on the opensource software that I use (unhinged confidence). Well, what did I expect, that an issue with #low-hanging-fruit was waiting for me in chromium and vscode's repo, or that Canonical, Gnome and KDE have delayed fixing very-naive bugs just so peasants could win challenges. Fortunately, none was the case, and I don't use much out of mainstream (read apt) for this stability.

So, this was not happening, until something happened after a month. A week ago, I wanted to find a link I knew was saved. I opened Raindrop, and I was greeted by 'few' thousand entries, unordered, uncategorized, without any heirarchy of collections.
While I did found what I was looking for, I decided to look around and discovered the AI features introduced, particularly AI tagging.

Took the decision for my own well-being, to categorize all my bookmarks and use some self-resolutions to manage the data (will post them too).

So I installed the open-source desktop app, and found this.

The native app was refreshing at every action. I quickly turned off the net, and yes, it was unusable. What a disppointing end to the fiasco. I wanted to organise my images and needed them offline.
Or maybe I can myself fix the issue, and then converge the destiny to make a bugsmash submission on dev.to

Here I am, having opened the PR and trying to write an intrigue.
.

Gemini you gem ✦

How does someone who is now more into ideas and less into code from last few months, quickly understand a codebase written by someone else.
Had Gemini Pro and Google did release a linux binary of Antigravity 2 (thank you).
.

What's up

Initial thought was that I would store loaded bookmarks into some storage in the desktop, so loaded the source code of desktop with web submodule.

This was probably the first few cases of me not diving into codebase first (us purists) and instead opened in Antigravity. Good decision.

The report was anyways not for me but for context, or so I thought.

I asked where we could add the code to store bookmarks while the actual list is being fetched.

and surprise

Visual Deception

The cache is there, but is it not working? Reading more in the response, the subheading was Collections. I again checked the behavior, and saw I had no collections after I had deleted them for refactor and was navigating in sidebar filters only.

Collection pages were indeed cached. Moreover Gemini told me that the whole logic was in the webapp repo, and not the desktop wrapper.

This time I did carefully read the architecture report. Skimmed through the code as well and switched active directory to webapp repo.

Deeply tested out the behavior, and gave the revised query.

thankfully

Your inference is correct. Here’s the precise mechanism.....

Pointing the Pin

Raindrop's bookmark cache is a KV store keyed by the first route segment: state.bookmarks.spaces[spaceId].

Collections get their own id collectionId key, but every non-collection route -

all-bookmarks
every filter
every tag

collapses onto the single key '0'. So bookmarks list of /my/0/type:article and /my/0/#nature overwrite each other on every switch.

Raised the issue at #253
.

I can't run this app?

What do we do before making edits. We run the dev server.
But we can't. Why? Because the repo is not configured for local usage. It relies heavily on original environment, and hardcodes.
I went back and forth in another gemini session, and could get the signup screen after editing a lot of files by myself too, but the api is apparently only configured to take auth requests from the deployed url only. It also never signed in to the local session.

After learning so much about the code and getting so close to the solution, I was not ready to withdraw.

Decided to have a second agent replace the visual review.
One session does the edits (active main agent), a second session (review agent) as adversary (plus point if you tell it that claude did the work, Jealosy-Driven Development).

To keep it iterative and pass proper diffs, I decided to commit the work of active agent before giving to review.
The first commit in PR is actually the final work of over 20 commits locally.
.

Back and Forth

(These are not exact prompts and results, but semantic partitions. The sequence is however intact. I am showing the most valuable segments.)

💬 Proposal
Since filters are predefined as links, articles etc. then why not assign negative keys, like -11 to -20, just like trash is assigned -99

✦︎ Gemini

Pointed out that filters types come from api and are subject to change.

[spaceId refers to the key used to query cache]

👤 Me

These 2 points were the most important comments, as I would have wasted so much compute and time if these didn't resonate as they did.

The whole logic of keys in cache is mirroring the routing decisions and is hence are coupled in the code too. This includes rehydration, updates, invalidation etc. As pointed earlier, the '0' comes from the first route segment of all these pages.

Forces at work
spaceId is threaded through the entire bookmarks tree

✦︎ Gemini
suggests

👤 Me

After a lot of back and forth, we agreed that '0' has to be kept the key or there would be too many changes to keep track of, in absence of a live preview.

🌟 Instead, there would be multiple composite keys with the respective identifiers as suffix to '0' as shown in above response

The clever idea is that when the consumer of this key is cache-related, then use the full composite key thus giving it a separate space in that flow. (remember these are keys, not indexes)

When the consumer is not related to cache, mostly when directly fed in route as shown below, then use parseInt to trim it to '0'

This enables us to be compatible to opinions of existing logic, and also surgically apply our modification.

👤 Me again

Decided to go only with filters and not tags. There were just too many bugs that could be introduced since tags are user defined and have their own flow of create and delete.

Filters on the other hand are server defined and not editable. So, told Gemini my decision to do it only for filters.

✦︎ Gemini

Component where the key is passed down to all other consumers.

That single prop becomes the cache key everywhere

💬 Proposal

Pass a second prop cacheId alongside an untouched spaceId, then swap it in only where cache reads happen. Nothing else sees a changed value. Surely safe.

✦︎ Gemini

Audited every reachable consumer and argued against me.

Additionally, it sorted consumer into three ways.
Bucket 1 already calls parseInt
Bucket 2 treats the value as an opaque string.
Bucket 3, consumers interpolating spaceId raw into a URL: popover, info chips, "open in new tab" etc. Solve with with parseInt(spaceId)||0

Right now all these use the spaceId props, the consumers who are supposed to use the composite cacheId prop may overlap in these, and may also pass them down in future where two different props for same route is being maintained.
A lot of silent bugs can be introduced esp with improper documentation.

👤 Me

Agreed. Brainstormed through multiple suggestions and gave it my own, to modify the main key being passed down. Modifying it only when current route is a filter page.
Told Gemini and it updated its plan with a hook and some refactors

✦︎ Gemini

Implements the plan

Modified the passed prop with composite key

Catching filter routes in a new file

Added parseInts to all consumers.

👤 Me

Changed regex to /^(?:[A-Za-z]+:[A-Za-z]+|❤️)$/ as i didn't want to cache if filter was combined with a search query, for ex. type:link & searchstring. Only pure routes should be cached.

✦︎ GEMINI REVIEW AGENT

It is not fully correct. Composite cache keys are not integrated into mutation and refresh paths, producing stale or blank filter views.

On deletion, status correction only processes numeric collection keys and bare 0, not 0:type:*

Therefore 0:type:article can have zero bookmarks after deletion while retaining status.main as loaded.
Instead of UI for empty bookmark list, the UI is of non-empty bookmark list but without bookmarks.

👤 Me and ✦︎ Gemini

Confirmed that this is an introduced bug, and not already present. asked for list of status and their effects for reference.

Tried to trace from where the above code gets data.

One line in bookmark helper

and Gemini's comments on it

It parseInts. 0:type:article collapses back to '0' before it is ever looked up. So even passing the composite key wouldn't have helped.

iterateSpaceId was never taught to parse the key we created. Hence, the whole actualize/insert/remove helper family is blind to it.

[Note the parseInt here was not added by our previous work]

🌟 Gemini Bonus Bug

The issue highlighted is related to the above discussed problem and will be fixed together.
But, that last line was a genuine bug in the deployed app where bookmark was being added to current filter page irrespective of its filter type.

Raised another issue for it at #255

💬 Proposal

In bookmark helpers iterateSpaceId function, the end goal is to pass the spaceId to func callback. I asked Gemini if it could make this callback available to our composite key too.

✦︎ Gemini

Now iterates over all filter keys, and calls func, hence registers them for status correction listeners.

It may seem redundant to use the loop, but we are to avoid all edge cases and also call with one key only once.

Additionally it pointed out that the blank case was not exactly our doing.
The broken view code is not our edit. The switch in empty/view.js traces to upstream Raindrop, untouched by edits. The missing case 'loaded': has always been there.

But whether that gap was reachable is the part we changed. Earlier, every space a bookmark could live in was either in action.spaceId or explicitly actualized. loaded status with zero ids wasn't realistically reachable via removal.
The unhandled case was a harmless impossible gap.

👤 Me

I accepted the edits, and also asked to apply defensive guards that mitigate against the existing gaps we discussed.

✦︎ Gemini

loaded status comes into effect only if the bookmark array is empty (ideally should never reach here).

In the bookmark reducer's REMOVE case, the same case which calls the iterateSpaceId with callback, added this line that sets status to empty if the cache has been emptied.

Thus we fixed a subtle bug that was present in upstream repo too.

✦︎ Gemini Review Agent

It flagged a msjor issue in the AI flow.

AI refresh remains pointed at raw cId (0) rather than the visible cache ID (0:type:article), so it does not refresh a filter view after a tool changes bookmarks.

The AI code calls refresh(cId), while the visible page reads spaces['0:type:article']. Refreshing cId neither fetches nor invalidates 0:type:article, therefore it does not fulfill the “next refresh” contract.

👤 Me

This was a real risk, we were finding downstream consumers of cId/spaceId, but then we were in the root of normal data flow of bookmarks.
The Ai flows in header and page are detached and separate from above. (recent features).

✦︎ Gemini

It suggested strongly that the same useMemo we used initially hook can be reused by moving to a new file. No need to undo any work.