Conversation
Replace the entire Elixir/Phoenix/Ash implementation with a Gleam application targeting Erlang/BEAM. Architecture: - Wisp + Mist for HTTP (server-rendered HTML + HTMX, no SPA) - Lustre SSR + lustre_pipes for view layer - SQLite + Parrot + sqlight for storage - gleam_otp actors for background workers (scheduler + fetcher) - Tailwind v4 + DaisyUI dark-only via glailglind (no Node.js) - xmerl via Erlang FFI for RSS/Atom parsing Feature parity with the old app: - Unread/Starred/History/Feeds pages - Add/delete feeds, OPML import - Toggle read/star (context-aware removal via HTMX + Referer) - Load more pagination (HTMX fragments + OOB swap) - Feed name in metadata, Heroicons SVG on buttons - Dark theme, favicon, structural/color parity Resilience: - HTTP fetcher isolated in unlinked monitored process (gleam_httpc FFI crashes on socket_closed_remotely) - Supervision tree with OneForOne restart strategy - Crash resilience tests (2 new tests, 82 total) Docker: - Multi-stage build (Gleam builder + Erlang runtime) - Fixed OTP version match (28), C compiler for esqlite NIF, bind 0.0.0.0 for container networking DB compatibility: schema is identical to old Elixir app. Existing feedreader_dev.db works directly with no migration.
|
Warning Review limit reached
More reviews will be available in 4 minutes and 32 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR completely replaces a Phoenix/Elixir/Ash/Oban RSS feed reader with a Gleam/Erlang implementation. All prior Elixir source, config, migrations, and tests are deleted. New code adds a SQLite schema, Parrot-generated SQL/Gleam query layer, XML FFI via ChangesFeedreader Gleam Rewrite
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Router as router.handle_request
participant DB as db (sqlight)
participant Fetcher as fetcher actor
participant Scheduler as scheduler actor
participant HTTP as http.fetch
rect rgba(70, 130, 180, 0.5)
note over Browser,DB: Full page load
Browser->>Router: GET / (unread)
Router->>DB: list_unread(conn, 50, 0)
DB-->>Router: List(Entry)
Router-->>Browser: 200 full HTML (pages.unread_page)
end
rect rgba(100, 160, 80, 0.5)
note over Browser,DB: HTMX toggle
Browser->>Router: POST /entries/:id/toggle-read
Router->>DB: toggle_read(conn, id)
Router->>DB: get_entry(conn, id)
Router-->>Browser: 200 entry card fragment (or empty fragment)
end
rect rgba(180, 100, 60, 0.5)
note over Scheduler,DB: Background fetch tick
Scheduler->>DB: list_feeds(conn)
Scheduler->>Scheduler: feeds_due(feeds, utc_now)
Scheduler->>Fetcher: Fetch(feed_id)
Fetcher->>DB: get_feed(conn, feed_id)
Fetcher->>HTTP: fetch(feed_url)
HTTP-->>Fetcher: Ok(body)
Fetcher->>Fetcher: rss.parse_feed(body)
Fetcher->>DB: upsert_entry(...) × N
Fetcher->>DB: log_fetch_success(conn, id, ts)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (18)
src/feedreader/web/router.gleam-63-63 (1)
63-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid
asserton DB reads in request handlers.These
assert Ok(...)paths can terminate the request process on transient DB errors instead of returning a controlled error response.Also applies to: 81-82, 100-101, 117-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/web/router.gleam` at line 63, Replace the `assert Ok(entries)` pattern on the `db.list_unread` function call with proper error handling using a case statement or let binding. Instead of asserting on the Result, pattern match on the Result to handle both the Ok and Error cases, returning a controlled error response (such as a 500 status or error page) when the database operation fails, rather than allowing the assertion to crash the request handler. Apply this same approach to all other database read calls mentioned at lines 81-82, 100-101, and 117 where similar assertions are used.src/feedreader/web/router.gleam-152-153 (1)
152-153:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t ignore toggle mutation results before reading back.
Both toggle handlers ignore
db.toggle_*result, so a failed write can still return an entry fragment as if the toggle succeeded.Also applies to: 173-174
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/web/router.gleam` around lines 152 - 153, The toggle_read operation result is being ignored with let _, which allows db.get_entry to return a successful entry fragment even if the toggle failed. Check the result returned by db.toggle_read(conn, id) to ensure it succeeded before calling db.get_entry(conn, id). If the toggle operation fails, return an appropriate error response instead of proceeding with the read. Apply the same fix to the second toggle handler around lines 173-174.src/feedreader/web/router.gleam-228-245 (1)
228-245:⚠️ Potential issue | 🟠 Major | ⚡ Quick winImported count should reflect successful inserts, not parsed rows.
The OPML import branch ignores each
db.insert_feedresult and reportslist.length(feed_attrs), which can over-report success when inserts fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/web/router.gleam` around lines 228 - 245, The current implementation calls db.insert_feed for each item in feed_attrs but discards the results using underscore assignment, then reports list.length(feed_attrs) as the successful import count, which can over-report successes when inserts fail. Modify the logic to capture the results from the db.insert_feed calls, filter or count only the successful results (typically Ok values in a Result type), and use that successful count instead of list.length(feed_attrs) when generating the success message in the feeds_page call.src/feedreader/web/router.gleam-203-215 (1)
203-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn error flash when feed insert fails.
db.insert_feedis ignored and success flash is always shown. Duplicate URL/DB failures are currently reported as successful adds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/web/router.gleam` around lines 203 - 215, The return value of db.insert_feed is being ignored with let _ =, causing the code to always show a success flash message regardless of whether the insert actually succeeded or failed. Instead of ignoring the result, capture the return value from db.insert_feed and pattern match on it to determine success or failure. When the insert fails (e.g., duplicate URL or database error), pass an error flash message to feeds_page indicating the failure, and only pass the success flash message when the insert actually completes successfully.src/feedreader/web/server.gleam-64-85 (1)
64-85:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFetcher is not under supervision despite restart guarantees in comments.
Only the scheduler is added to the supervisor. If
fetcherexits, scheduler keeps sending messages to a dead subject and background fetching silently degrades.Suggested direction
- case fetcher.start_with_http(conn) { - Ok(started) -> { - let fetcher_subject = started.data - let scheduler_spec = ... - sup.new(...) - |> sup.add(scheduler_spec) + // Start both actors as supervised children and wire scheduler from fetcher child + // through a supervised initialiser flow (or a parent actor that owns both subjects).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/web/server.gleam` around lines 64 - 85, The fetcher actor is not under supervision, only the scheduler is being added to the supervisor. Create a supervision spec for the fetcher similar to the scheduler_spec using supervision.worker with the fetcher.start_with_http call, configure it with supervision.restart(supervision.Permanent) to restart on crash, and then add both the fetcher_spec and scheduler_spec to the supervisor using sup.add() calls before calling sup.start to ensure the fetcher is also monitored and automatically restarted if it crashes.src/feedreader/fetcher.gleam-49-62 (1)
49-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not ignore upsert/log write failures in the success path.
The current code discards every
db.upsert_entryresult anddb.log_fetch_successresult, soFetchedcan be returned even when persistence fails. Propagate the first write failure and returnFetchFailedto keep fetch state consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/fetcher.gleam` around lines 49 - 62, The code currently ignores all results from db.upsert_entry calls within the list.each iteration and from db.log_fetch_success, causing Fetched to be returned even when persistence fails. Instead of discarding these results with let _ =, capture and check the Result types returned by these database operations. If any db.upsert_entry call or the db.log_fetch_success call returns an error, propagate that failure immediately by returning FetchFailed rather than continuing. Only return Fetched when all upsert operations and the log_fetch_success operation complete successfully.src/feedreader/fetcher.gleam-32-35 (1)
32-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDifferentiate DB errors from missing feeds.
db.get_feedfailures are currently folded into"Feed not found", which masks real storage faults and breaks operational diagnosis. HandleOk(None)andError(_)as separateFetchFailedcases.Suggested fix
case db.get_feed(conn, feed_id) { Ok(Some(feed)) -> do_process(conn, feed_id, feed.feed_url, fetch_fn) - _ -> FetchFailed(error: "Feed not found") + Ok(None) -> FetchFailed(error: "Feed not found") + Error(db_error) -> FetchFailed(error: "DB error: " <> db_error) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/fetcher.gleam` around lines 32 - 35, The pattern matching on db.get_feed in the process_feed function uses a catch-all _ that groups both the Ok(None) case (feed doesn't exist) and Error case (database failure) into the same "Feed not found" error message, which masks actual storage faults. Replace the catch-all _ with explicit pattern matching for Ok(None) to return a FetchFailed with "Feed not found" message, and add a separate Error(db_error) case that returns a FetchFailed with a distinct error message that differentiates it as a database/storage error rather than a missing feed, allowing proper operational diagnosis.src/feedreader_xml_ffi.erl-14-16 (1)
14-16:⚠️ Potential issue | 🟠 MajorReplace
xmerl_scanwith a secure alternative or apply critical hardening options.This parser processes untrusted remote feed XML using
xmerl_scan:string/2without security options. This creates two vulnerabilities:
- XXE/Entity Expansion: Add
{allow_entities, false}to prevent external entity attacks (SSRF, file disclosure, DoS).- Atom Exhaustion DoS:
xmerl_scandynamically generates atoms from XML tag and attribute names, allowing untrusted input to exhaust the atom table and crash the VM. This risk cannot be mitigated by parser options alone.Official Erlang security guidance recommends avoiding
xmerl_scanfor untrusted data. Consider migrating toxmerl_sax_parserwithdisallow_entitiesoption, or a modern XML parser that does not exhibit these flaws. If immediate migration is not feasible, at minimum add{allow_entities, false}and implement strict limits on XML document size and element nesting depth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader_xml_ffi.erl` around lines 14 - 16, The xmerl_scan:string/2 call in the XML parsing logic processes untrusted remote feed data without critical security options, exposing the system to XXE attacks and potential atom exhaustion DoS. Immediately add {allow_entities, false} to the Opts list to prevent external entity injection attacks. Additionally, consider migrating from xmerl_scan to xmerl_sax_parser with disallow_entities option, or implementing a modern XML parser that does not dynamically generate atoms from untrusted input. If migration is not immediately feasible, also implement strict size limits on the XmlString parameter and depth limits on XML element nesting to mitigate the atom exhaustion vulnerability in xmerl_scan.src/feedreader/rss.gleam-49-57 (1)
49-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent empty
external_idfallback to avoid entry collisions.On Line 55,
external_idfalls back tolink, which can be"". On Line 70 this value is persisted downstream, so multiple no-id/no-link items can collapse into one upsert key.Suggested fix
- // external_id: guid > id > link (fallback) - let external_id = case guid { - Some(g) -> g - None -> - case id { - Some(i) -> i - None -> link - } - } + // external_id: guid > id > non-empty link > non-empty title + let external_id = + case guid { + Some(g) -> g + None -> + case id { + Some(i) -> i + None -> + case string_trim(link) { + Some(l) -> l + None -> + case string_trim(title) { + Some(t) -> t + None -> "missing-id" + } + } + } + }Also applies to: 69-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/rss.gleam` around lines 49 - 57, The external_id assignment in the nested case expression currently falls back to link without validating that it is not empty, which allows multiple items without guid or id to share the same empty string external_id and collapse into a single upsert entry. Modify the innermost None case in the id case expression to check whether link is empty before using it as the fallback value, and provide an alternative fallback (such as a unique identifier or a meaningful default) when link is an empty string to ensure each entry has a distinct external_id.src/feedreader/rss.gleam-86-93 (1)
86-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
rel=""as alternate in Atom link selection.Line 92 treats
Some("")as non-match, even though your own comment says rel="" should be accepted. This can produce emptycontent_linkfor valid Atom entries.Suggested fix
case xml.attr(link, "rel") { Some("alternate") -> True + Some("") -> True None -> True _ -> False }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/rss.gleam` around lines 86 - 93, The pattern matching in the alternate link selection is missing a case for when the rel attribute has an empty string value. Currently, Some("") falls through to the wildcard case and returns False, but according to your comment, an empty rel attribute value should be treated as alternate. Add an additional pattern match case for Some("") that returns True, placing it alongside the existing Some("alternate") case in the pattern match expression within the list.find call.src/feedreader/opml.gleam-36-65 (1)
36-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSupport top-level feed outlines (
xmlUrl) in OPML body.Line 36+ treats all top-level outlines as categories and only parses their children. Many OPML exports place feeds directly under
<body>, so those subscriptions are silently dropped.Suggested fix
fn extract_feeds(category_outlines: List(XmlNode)) -> List(FeedAttrs) { category_outlines |> list.flat_map(fn(category_outline) { - let category = category_name(category_outline) - - // Direct children that have xmlUrl are feeds in this category - let children = xml.children_by_tag(category_outline, "outline") - let feeds = - children - |> list.filter(fn(child) { - case xml.attr(child, "xmlUrl") { - Some(url) -> url != "" - None -> False - } - }) - |> list.map(fn(feed_outline) { outline_to_attrs(feed_outline, category) }) - - feeds + case xml.attr(category_outline, "xmlUrl") { + Some(url) if url != "" -> [outline_to_attrs(category_outline, "Uncategorized")] + _ -> { + let category = category_name(category_outline) + let children = xml.children_by_tag(category_outline, "outline") + children + |> list.filter(fn(child) { + case xml.attr(child, "xmlUrl") { + Some(url) -> url != "" + None -> False + } + }) + |> list.map(fn(feed_outline) { outline_to_attrs(feed_outline, category) }) + } + } }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/opml.gleam` around lines 36 - 65, In the parse_opml function, after retrieving category_outlines from the body element, modify the logic to handle both top-level feeds and category outlines. First, filter the category_outlines to separate any outlines that have an xmlUrl attribute (these are direct feeds, not categories) from those that don't (these are categories). Extract the top-level feeds directly using outline_to_attrs with no category, then process the remaining outlines through the extract_feeds function to get feeds from within categories. Combine both lists of feeds and return them together so that top-level feeds are no longer silently dropped.Dockerfile-24-44 (1)
24-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun the runtime image as a non-root user.
The runtime stage defaults to root. If the process is compromised, filesystem and container permissions are broader than necessary.
Suggested runtime hardening
FROM erlang:28-alpine WORKDIR /app # Install SQLite runtime library (esqlite NIF depends on libsqlite3) RUN apk add --no-cache sqlite-libs COPY --from=builder /app/build/erlang-shipment ./ COPY --from=builder /app/priv ./priv + +RUN addgroup -S app && adduser -S -G app app \ + && mkdir -p /data \ + && chown -R app:app /app /data ENV DATABASE_PATH=/data/feedreader.db VOLUME ["/data"] EXPOSE 3000 +USER app CMD ["./entrypoint.sh", "run"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 24 - 44, The runtime stage in the Dockerfile (starting with FROM erlang:28-alpine) currently runs as the root user by default, which poses a security risk if the process is compromised. Add a non-root user using a RUN command to create the user and set appropriate permissions on the /app and /data directories, then add a USER instruction before the CMD instruction to switch to that non-root user. This limits the filesystem and container permissions in case the running process is compromised.Source: Linters/SAST tools
mise.toml-2-4 (1)
2-4:⚠️ Potential issue | 🟠 MajorPin
misetool versions to match CI instead oflatest.Using
latestmakes local behavior non-deterministic and diverges from CI (gleam: 1.16.0,otp: 28), causing avoidable check/test/codegen failures.Suggested version pinning
-gleam = "latest" -erlang = "latest" -rebar = "latest" +gleam = "1.16.0" +erlang = "28" +rebar = "<pin_tested_rebar_version>"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mise.toml` around lines 2 - 4, The mise.toml file uses "latest" version specifiers for gleam, erlang, and rebar tools, causing non-deterministic behavior and divergence from CI versions. Replace the "latest" values with specific pinned versions that match your CI configuration: gleam should be pinned to 1.16.0, erlang (otp) should be pinned to 28, and rebar should be pinned to its corresponding stable version used in CI. This ensures consistent local development behavior that matches your CI pipeline..github/workflows/ci.yml-13-18 (1)
13-18:⚠️ Potential issue | 🟠 MajorPin action references to commit SHAs and disable checkout credential persistence.
The workflow uses tag-based
actions/references (actions/checkout@v4,erlef/setup-beam@v1) which are mutable and can be force-pushed, creating a supply-chain risk. Additionally,actions/checkoutpersists credentials by default, potentially leaking token scope to later workflow steps.Suggested fix
- - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 + with: + persist-credentials: false - - uses: erlef/setup-beam@v1 + - uses: erlef/setup-beam@fc68ffb90438ef2936bbb3251622353b3dcb2f93 with: gleam-version: "1.16.0" otp-version: "28"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 13 - 18, The workflow currently uses mutable tag-based action references (actions/checkout@v4 and erlef/setup-beam@v1) which create supply chain risks. Replace these tag references with specific commit SHAs for both actions/checkout and erlef/setup-beam to pin to exact versions. Additionally, add persist-credentials: false to the actions/checkout step to prevent credential persistence and reduce token scope exposure to subsequent workflow steps.Source: Linters/SAST tools
src/feedreader/db.gleam-386-425 (1)
386-425:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftToggle operations are non-atomic and can lose updates under concurrency.
Both toggle paths read current state first, then write the opposite. Concurrent toggles can read the same value and collapse two toggles into one final state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/db.gleam` around lines 386 - 425, The toggle_read and toggle_starred functions perform non-atomic read-then-write operations that can lose updates under concurrent access. Instead of reading the current entry value and then updating the toggled value separately, refactor both functions to use a single atomic SQL UPDATE statement that toggles the boolean directly in the database. This means removing the get_entry call and the in-memory boolean negation, and instead have the SQL update operation toggle the is_read or is_starred column atomically using SQL logic (typically a NOT or similar operation) so that concurrent requests cannot read stale values and lose intermediate updates.src/feedreader/db.gleam-193-201 (1)
193-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFeed mappers leak sentinel empty strings as real values.
insert_feedwritesNoneas"", butrow_to_feed_*returns rawrow.name/site_url/..., so callers receiveSome("")instead ofNone.🐛 Mapper normalization fix
fn row_to_feed_list(row: sql.ListFeeds) -> Feed { Feed( id: row.id, - name: row.name, - site_url: row.site_url, + name: str_to_opt(row.name), + site_url: str_to_opt(row.site_url), feed_url: row.feed_url, category: row.category, - last_fetched_at: row.last_fetched_at, - fetch_error: row.fetch_error, + last_fetched_at: str_to_opt(row.last_fetched_at), + fetch_error: str_to_opt(row.fetch_error), ) }Also applies to: 450-483
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/db.gleam` around lines 193 - 201, The insert_feed function converts None values to empty strings using opt_to_str, but the row_to_feed_* mapper functions are returning the raw row values without converting empty strings back to None. This causes a mismatch where None becomes Some("") instead of staying as None. Modify the row_to_feed_* mapper functions to normalize empty strings back to None before returning them to callers, ensuring the round-trip conversion works correctly. The same fix needs to be applied to all mapper functions affected by this pattern, including those referenced at lines 450-483.src/feedreader/sql/queries.sql-77-83 (1)
77-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHistory ordering currently prioritizes undated entries over newest dated entries.
e.published_at IS NULL DESCplaces NULLs first, so history is not truly “newest first” for users.🐛 Query fix
-ORDER BY e.published_at IS NULL DESC, e.published_at DESC +ORDER BY e.published_at IS NULL ASC, e.published_at DESC🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/sql/queries.sql` around lines 77 - 83, The ListHistory query's ORDER BY clause incorrectly prioritizes entries with NULL published_at values by using IS NULL DESC, which sorts NULL values first instead of last. Fix this by changing the sort order of the IS NULL condition to ASC instead of DESC in the ORDER BY clause within the ListHistory query, so that entries with actual published_at dates appear first in reverse chronological order, followed by any entries without a published date.priv/static/css/app.css-6-16 (1)
6-16:⚠️ Potential issue | 🟠 MajorStylelint will block CI until Tailwind v4 at-rules are allowed in config, and the stylesheet lacks interaction transitions per coding guidelines.
The
.stylelintrc.jsonextendsstylelint-config-standard-scss, which activatesat-rule-no-unknownby default. Since@sourceand@pluginare Tailwind v4-specific at-rules (not part of standard CSS/SCSS), they will be flagged as invalid. Add these directives to an ignore list in theat-rule-no-unknownrule configuration.Additionally, the stylesheet contains only theme tokens and is missing micro-interactions. Per coding guidelines, CSS files should implement subtle hover effects, loading states, and smooth transitions. Consider adding base interaction styles (e.g., button hover transitions, focus states) to this stylesheet.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@priv/static/css/app.css` around lines 6 - 16, The Stylelint configuration flagging Tailwind v4 at-rules as unknown needs to be fixed, and the stylesheet lacks interaction micro-interactions. Update the at-rule-no-unknown rule in .stylelintrc.json to add `@source` and `@plugin` to an ignore list so these Tailwind v4-specific directives are allowed. Additionally, add base interaction styles to app.css such as button hover transitions, focus states, and smooth transitions to comply with the coding guidelines for micro-interactions, ensuring the stylesheet goes beyond theme tokens to include subtle interactive effects.Source: Linters/SAST tools
🟡 Minor comments (5)
src/feedreader/web/router.gleam-136-143 (1)
136-143:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReferer page detection breaks when query params are present.
string.ends_with(ref, "/starred")/"/history"misses referers like/starred?after=50, so filtered-page removal logic can silently fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/web/router.gleam` around lines 136 - 143, The referer page detection logic using string.ends_with for "/starred" and "/history" patterns does not account for query parameters in the URL, causing matches to fail when referers like "/starred?after=50" are present. Extract the path portion of the ref variable by splitting on the question mark character to isolate just the path component before the query string, then use this path variable in the string.ends_with calls for is_unread, is_starred, and is_history comparisons to ensure the pattern matching works correctly regardless of query parameters.src/feedreader_tcp_ffi.erl-15-22 (1)
15-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the accept loop running on timeout to avoid flaky crash tests.
The current
{error, _} -> okbranch exits the loop on transient accept timeouts, so later requests may not hit the intended “accept then close” path.Proposed fix
case gen_tcp:accept(ListenSock, 1000) of {ok, Sock} -> %% Read a bit then abruptly close — no HTTP response sent gen_tcp:close(Sock), Loop(); - {error, _} -> + {error, timeout} -> + Loop(); + {error, closed} -> ok end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader_tcp_ffi.erl` around lines 15 - 22, The error handling in the gen_tcp:accept case statement treats all errors the same way by exiting the loop with ok. When a timeout occurs (which is transient and expected with the 1000ms timeout), the loop should continue to keep accepting connections rather than exit. Modify the error clause to distinguish between timeout errors and other errors, calling Loop() recursively when the error is a timeout to keep the accept loop running, while allowing the function to exit for non-timeout errors.src/feedreader/time.gleam-27-33 (1)
27-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFuture timestamps are misclassified as “just now”.
diff_secondscan be negative (clock skew or future pub dates), and the current first branch (d < 60) labels those as “just now”. Add an explicit future guard before relative-time branches.Proposed fix
fn cond_format(diff_seconds: Int, dt: birl.Time) -> String { case diff_seconds { + d if d < 0 -> format_full_date(dt) d if d < 60 -> "just now" d if d < 3600 -> int.to_string(d / 60) <> "m ago"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/time.gleam` around lines 27 - 33, The case statement matching on diff_seconds incorrectly classifies negative values (which represent future timestamps) as "just now" because the first guard condition `d if d < 60` evaluates to true for all negative numbers. Add an explicit guard at the beginning of the case expression to check if diff_seconds is negative and handle future timestamps appropriately before the relative-time branches, ensuring negative values are handled with a dedicated pattern that returns a suitable message for future-dated content.src/feedreader/http.gleam-46-49 (1)
46-49:⚠️ Potential issue | 🟡 MinorHandle invalid URL parsing without crashing the worker.
let assert Ok(req) = request.to(url)crashes the worker when given a malformed URL, surfacing as a generic monitor-down error. Therequest.to/1function returnsError(Nil)for invalid URLs, which the assert cannot handle. Use a case statement to return an error result instead, keeping the real failure cause in fetcher logs.Proposed fix
fn do_http_request(url: String) -> Result(String, String) { - let assert Ok(req) = request.to(url) - let req = request.set_method(req, http.Get) + let req = + case request.to(url) { + Ok(r) -> request.set_method(r, http.Get) + Error(_) -> return Error("Invalid URL") + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feedreader/http.gleam` around lines 46 - 49, In the do_http_request function, the `let assert Ok(req) = request.to(url)` statement crashes the worker when the URL is malformed because request.to/1 returns Error(Nil) for invalid URLs. Replace this assert statement with a case expression that matches on the result of request.to(url), returning an error result in the Error case instead of crashing, so that invalid URLs are handled gracefully and the real failure cause is logged.README.md-1-17 (1)
1-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winREADME package/module naming is inconsistent with the project config.
The docs reference
feedreader_gleam(gleam add, import, Hex links), but project metadata declaresfeedreader. This will mislead users and break copy-pasted setup commands.Suggested README correction
-# feedreader_gleam +# feedreader -[](https://hex.pm/packages/feedreader_gleam) -[](https://hexdocs.pm/feedreader_gleam/) +<!-- Update/remove package badges if this repo is an app and not a published Hex package --> ```sh -gleam add feedreader_gleam@1 +gleam run-import feedreader_gleam +import feedreader-Further documentation can be found at https://hexdocs.pm/feedreader_gleam.
+Further documentation can be found in this repository's README and source modules.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 1 - 17, The README contains inconsistent package
naming that conflicts with the actual project configuration. Replace all
references tofeedreader_gleamwithfeedreaderthroughout the README: change
the gleam add command fromgleam add feedreader_gleam@1togleam run, update
the import statement fromimport feedreader_gleamtoimport feedreader, and
replace the Hex documentation link pointing to
https://hexdocs.pm/feedreader_gleamwith a reference to the repository's
README and source modules instead. This ensures the setup instructions and code
examples match the actual project configuration and won't cause errors when
users follow the copy-pasted commands.</details> <!-- cr-comment:v1:395c8796f7353a87ef674e96 --> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (7)</summary><blockquote> <details> <summary>test/feedreader/fetcher_test.gleam (1)</summary><blockquote> `103-112`: _⚡ Quick win_ **Align parse-error test assertions with the test’s stated purpose.** `process_feed_parse_error_logs_error_test` currently validates only `FetchFailed`, not that fetch error metadata was actually written. Add `db.get_feed` assertions for `last_fetched_at` and `fetch_error` to cover the logging contract. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@test/feedreader/fetcher_test.gleamaround lines 103 - 112, The test
process_feed_parse_error_logs_error_testonly validates thatFetchFailedis
returned but does not verify that the error was actually logged to the database.
After asserting theFetchFailedcase, add assertions usingdb.get_feedto
retrieve the feed record and verify that both thelast_fetched_atand
fetch_errorfields were properly updated with the error metadata. This ensures
the test covers the full contract of error logging behavior.</details> <!-- cr-comment:v1:3ae0b4321f2504fa6b5eb6af --> </blockquote></details> <details> <summary>test/feedreader/rss_test.gleam (1)</summary><blockquote> `74-90`: _⚡ Quick win_ **Add Atom regression coverage for `<link rel=\"\" href=\"...\">`.** There is no test for the default-empty `rel` case, so the current selector bug is not caught. <details> <summary>Suggested test addition</summary> ```diff +pub fn atom_link_empty_rel_is_accepted_test() { + let body = + " + <feed xmlns=\"http://www.w3.org/2005/Atom\"> + <entry> + <title>Default Rel</title> + <link rel=\"\" href=\"http://example.com/default\"/> + <id>default-rel-id</id> + </entry> + </feed>" + + let assert Ok(entries) = rss.parse_feed(body) + let assert Ok(first) = list.first(entries) + assert first.content_link == "http://example.com/default" +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/feedreader/rss_test.gleam` around lines 74 - 90, Add a new test function for Atom feed parsing that covers the regression case of link elements with an empty or missing rel attribute. Create a test similar to atom_link_multiple_selects_alternate_test that parses an Atom feed containing a link element without a rel attribute (or with rel=""), then verify that the parser correctly handles this default case and sets the content_link appropriately. This will ensure the link selector logic properly handles links without an explicit rel attribute.test/feedreader/opml_test.gleam (1)
34-55: ⚡ Quick winAdd coverage for flat OPML exports (feeds directly under
<body>).Current tests only validate category-wrapped outlines, so dropped flat outlines won’t be detected.
Suggested test addition
+pub fn parse_flat_body_outlines_test() { + let body = + " + <opml version=\"2.0\"> + <body> + <outline title=\"Flat A\" text=\"Flat A\" xmlUrl=\"https://flat-a.com/rss\" htmlUrl=\"https://flat-a.com\"/> + </body> + </opml>" + + let assert Ok(feeds) = opml.parse_opml(body) + assert list.length(feeds) == 1 + let assert Ok(first) = list.first(feeds) + assert first.feed_url == "https://flat-a.com/rss" +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/feedreader/opml_test.gleam` around lines 34 - 55, Add a new test function to cover flat OPML exports where feeds are directly under the `<body>` tag without category wrappers. Create an OPML structure with outline elements at the top level of `<body>` (not nested inside category outlines), parse it using opml.parse_opml, and assert that the feeds are correctly parsed with the appropriate default category value (likely an empty string or similar) to ensure that flat feed structures are handled correctly and won't regress in future changes.test/feedreader/date_test.gleam (1)
9-37: ⚡ Quick winStrengthen positive parse tests to verify normalization output.
These tests only check
!= None, so they won’t catch incorrect timezone normalization or formatting regressions inparse_date. Assert expected equivalence/value for representative inputs (e.g., EST vs-0500) instead of presence-only checks.Suggested test tightening
pub fn parse_rfc822_with_named_tz_test() { - let result = date.parse_date(Some("Thu, 12 Jun 2025 14:30:00 EST")) - assert result != None + let est = date.parse_date(Some("Thu, 12 Jun 2025 14:30:00 EST")) + let offset = date.parse_date(Some("Thu, 12 Jun 2025 14:30:00 -0500")) + assert est == offset }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/feedreader/date_test.gleam` around lines 9 - 37, The parse date tests (parse_iso8601_test, parse_iso8601_with_offset_test, parse_rfc822_test, parse_rfc822_with_named_tz_test, parse_rfc822_with_pst_test, and parse_rfc822_with_numeric_offset_test) only verify that parse_date returns Some value without checking the actual parsed content. Replace the weak assertions with proper value assertions that verify the parsed result contains the correct date, time, and normalized timezone values for each input format. For example, test that EST is normalized to the correct offset equivalent, or that the parsed ISO8601 date has the expected year, month, day, and time components.priv/static/css/app.css (1)
14-48: ⚡ Quick winAdd baseline micro-interactions for interactive elements.
This theme file defines colors/radii well, but it still lacks explicit transition/hover behavior for links/buttons/cards.
♻️ Suggested baseline interaction layer
+@layer base { + a, + button, + .btn, + .card { + transition: color 160ms ease, background-color 160ms ease, border-color 160ms ease, transform 160ms ease, box-shadow 160ms ease; + } + + .btn:hover, + .card:hover { + transform: translateY(-1px); + } +}As per coding guidelines:
**/*.{js,ts,jsx,tsx,css}should implement subtle micro-interactions such as hover effects and smooth transitions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@priv/static/css/app.css` around lines 14 - 48, The dark theme configuration in the `@plugin` "daisyui-theme" block defines colors and radius values but lacks baseline micro-interactions for interactive elements. Add transition properties (such as transition-duration and transition-timing-function) to the theme configuration, and then define hover state rules for common interactive elements like links, buttons, and cards that apply subtle color or opacity changes. These micro-interactions should be smooth and understated to align with the coding guidelines for interactive element behavior.Source: Coding guidelines
test/feedreader/db_test.gleam (1)
81-95: ⚡ Quick winAdd round-trip assertions for nullable feed fields in this test.
This test validates only the value returned by
insert_feed, not what comes back fromget_feed, so it won’t catchSome("")vsNoneregressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/feedreader/db_test.gleam` around lines 81 - 95, The test insert_feed_with_defaults_test only validates the feed object returned directly from db.insert_feed, but does not verify that the nullable fields (name and site_url) are correctly persisted in the database and retrieved back. After the current assertions on the returned feed object, add a round-trip verification by calling db.get_feed with the inserted feed's ID and add matching assertions on the retrieved feed object to ensure the None values for name and site_url are correctly stored and retrieved from the database, preventing regressions where Some("") could be stored instead of None.priv/schema.sql (1)
4-8: ⚡ Quick winPrevent schema drift between runtime migrations and codegen input.
Both
priv/schema.sqlandsrc/feedreader/sql/schema.sqlare maintained copies and both claim source-of-truth. If they diverge, runtime DB shape and generated query bindings can silently drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@priv/schema.sql` around lines 4 - 8, The repository currently maintains two separate copies of the schema file - priv/schema.sql and src/feedreader/sql/schema.sql - which both claim to be the source of truth. This creates the risk of silent schema drift between runtime migrations and generated code. Consolidate the schema to have a single authoritative source of truth by either removing the duplicate schema file from src/feedreader/sql/schema.sql and ensuring it is generated as an artifact from priv/schema.sql during the mise run gen process, or by establishing an automated validation step that fails the build if these files diverge to prevent accidental inconsistencies between the runtime database shape and generated query bindings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f3db699-b4a8-489d-8483-93dc55d6cd5a
⛔ Files ignored due to path filters (4)
mix.lockis excluded by!**/*.lockpriv/static/favicon.icois excluded by!**/*.icopriv/static/images/logo.svgis excluded by!**/*.svgpriv/static/js/htmx.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (130)
.credo.exs.dockerignore.formatter.exs.github/workflows/ci.yml.github/workflows/docker.yml.gitignore.igniter.exs.pi/skills/gleam-testing/SKILL.mdAGENTS.mdDockerfileE2E.mdPLAN.mdREADME.mdSCOUTING_REPORT.mdSTATUS.mdassets/css/app.cssassets/js/app.jsassets/tsconfig.jsonassets/vendor/topbar.jsconfig/config.exsconfig/dev.exsconfig/prod.exsconfig/runtime.exsconfig/test.exsdocker-compose.yamlgleam.tomllib/feedreader.exlib/feedreader/accounts.exlib/feedreader/accounts/emails.exlib/feedreader/accounts/token.exlib/feedreader/accounts/user.exlib/feedreader/application.exlib/feedreader/core.exlib/feedreader/core/entry.exlib/feedreader/core/feed.exlib/feedreader/mailer.exlib/feedreader/repo.exlib/feedreader/secrets.exlib/feedreader/workers/fetch_feed.exlib/feedreader/workers/scheduler.exlib/feedreader_web.exlib/feedreader_web/components/core_components.exlib/feedreader_web/components/layouts.exlib/feedreader_web/components/layouts/root.html.heexlib/feedreader_web/components/time_helpers.exlib/feedreader_web/controllers/auth_controller.exlib/feedreader_web/controllers/error_html.exlib/feedreader_web/controllers/error_json.exlib/feedreader_web/controllers/page_controller.exlib/feedreader_web/controllers/page_html.exlib/feedreader_web/controllers/page_html/home.html.heexlib/feedreader_web/endpoint.exlib/feedreader_web/gettext.exlib/feedreader_web/live/entry_live/index.exlib/feedreader_web/live/entry_live/index.html.heexlib/feedreader_web/live/feed_live/index.exlib/feedreader_web/live/feed_live/index.html.heexlib/feedreader_web/live_user_auth.exlib/feedreader_web/router.exlib/feedreader_web/telemetry.exmanifest.tomlmark-read.shmise.tomlmix.exspriv/gettext/en/LC_MESSAGES/errors.popriv/gettext/errors.potpriv/repo/migrations/.formatter.exspriv/repo/migrations/20260323185857_add_oban.exspriv/repo/migrations/20260323185915_initialize_and_add_authentication_resources.exspriv/repo/migrations/20260324122843_add_core_resources.exspriv/repo/migrations/20260324172014_add_created_at.exspriv/repo/migrations/20260324233336_migrate_resources1_dev.exspriv/repo/seeds.exspriv/resource_snapshots/repo/entries/20260324122844.jsonpriv/resource_snapshots/repo/entries/20260324171514_dev.jsonpriv/resource_snapshots/repo/entries/20260324172015.jsonpriv/resource_snapshots/repo/feeds/20260324122845.jsonpriv/resource_snapshots/repo/tokens/20260323185916.jsonpriv/resource_snapshots/repo/users/20260323185917.jsonpriv/resource_snapshots/repo/users/20260324233337_dev.jsonpriv/schema.sqlpriv/static/css/app.csspriv/static/robots.txtpriv/static/vendor/daisyui-theme.jspriv/static/vendor/daisyui.jspriv/static/vendor/heroicons.jsreceipt.htmlsrc/feedreader.gleamsrc/feedreader/date.gleamsrc/feedreader/db.gleamsrc/feedreader/fetcher.gleamsrc/feedreader/http.gleamsrc/feedreader/opml.gleamsrc/feedreader/rss.gleamsrc/feedreader/scheduler.gleamsrc/feedreader/sql.gleamsrc/feedreader/sql/queries.sqlsrc/feedreader/sql/schema.sqlsrc/feedreader/time.gleamsrc/feedreader/web/fragments.gleamsrc/feedreader/web/html.gleamsrc/feedreader/web/pages.gleamsrc/feedreader/web/router.gleamsrc/feedreader/web/server.gleamsrc/feedreader/xml.gleamsrc/feedreader_tcp_ffi.erlsrc/feedreader_xml_ffi.erltest/feedreader/core/entry_test.exstest/feedreader/core/feed_delete_test.exstest/feedreader/core/feed_test.exstest/feedreader/date_test.gleamtest/feedreader/db_test.gleamtest/feedreader/fetcher_test.gleamtest/feedreader/http_test.gleamtest/feedreader/opml_test.gleamtest/feedreader/rss_test.gleamtest/feedreader/scheduler_test.gleamtest/feedreader/web/pages_test.gleamtest/feedreader/workers/fetch_feed_test.exstest/feedreader/xml_test.gleamtest/feedreader_test.gleamtest/feedreader_web/components/time_helpers_test.exstest/feedreader_web/controllers/error_html_test.exstest/feedreader_web/controllers/error_json_test.exstest/feedreader_web/controllers/page_controller_test.exstest/feedreader_web/live/entry_live_test.exstest/feedreader_web/live/feed_live_test.exstest/support/conn_case.extest/support/data_case.extest/test_helper.exs
💤 Files with no reviewable changes (79)
- lib/feedreader_web/controllers/page_controller.ex
- .formatter.exs
- lib/feedreader_web/live/feed_live/index.html.heex
- test/feedreader/core/entry_test.exs
- lib/feedreader_web/controllers/page_html.ex
- priv/resource_snapshots/repo/users/20260324233337_dev.json
- .credo.exs
- test/feedreader_web/controllers/error_json_test.exs
- .github/workflows/docker.yml
- priv/static/robots.txt
- config/test.exs
- lib/feedreader/accounts.ex
- lib/feedreader/accounts/user.ex
- lib/feedreader/mailer.ex
- priv/resource_snapshots/repo/users/20260323185917.json
- lib/feedreader_web/live_user_auth.ex
- lib/feedreader_web/controllers/error_html.ex
- .dockerignore
- lib/feedreader_web/controllers/error_json.ex
- priv/repo/migrations/20260323185915_initialize_and_add_authentication_resources.exs
- test/test_helper.exs
- test/feedreader/core/feed_delete_test.exs
- config/config.exs
- lib/feedreader_web/controllers/page_html/home.html.heex
- lib/feedreader_web/components/time_helpers.ex
- test/feedreader/core/feed_test.exs
- lib/feedreader_web/live/feed_live/index.ex
- lib/feedreader_web/components/layouts.ex
- lib/feedreader_web/controllers/auth_controller.ex
- priv/repo/seeds.exs
- assets/js/app.js
- lib/feedreader/core/entry.ex
- test/feedreader_web/controllers/error_html_test.exs
- assets/tsconfig.json
- lib/feedreader.ex
- priv/repo/migrations/20260323185857_add_oban.exs
- assets/vendor/topbar.js
- priv/repo/migrations/20260324122843_add_core_resources.exs
- lib/feedreader_web/gettext.ex
- mix.exs
- test/support/conn_case.ex
- lib/feedreader_web/components/layouts/root.html.heex
- test/feedreader_web/live/feed_live_test.exs
- priv/resource_snapshots/repo/tokens/20260323185916.json
- config/runtime.exs
- priv/resource_snapshots/repo/feeds/20260324122845.json
- priv/resource_snapshots/repo/entries/20260324122844.json
- priv/resource_snapshots/repo/entries/20260324171514_dev.json
- lib/feedreader_web/live/entry_live/index.html.heex
- test/feedreader_web/components/time_helpers_test.exs
- test/support/data_case.ex
- lib/feedreader/accounts/token.ex
- lib/feedreader/core.ex
- lib/feedreader_web.ex
- priv/gettext/en/LC_MESSAGES/errors.po
- lib/feedreader/accounts/emails.ex
- lib/feedreader/repo.ex
- test/feedreader_web/controllers/page_controller_test.exs
- test/feedreader/workers/fetch_feed_test.exs
- priv/gettext/errors.pot
- lib/feedreader/secrets.ex
- assets/css/app.css
- priv/resource_snapshots/repo/entries/20260324172015.json
- lib/feedreader_web/telemetry.ex
- test/feedreader_web/live/entry_live_test.exs
- lib/feedreader/core/feed.ex
- priv/repo/migrations/20260324172014_add_created_at.exs
- lib/feedreader/workers/fetch_feed.ex
- priv/repo/migrations/20260324233336_migrate_resources1_dev.exs
- lib/feedreader_web/router.ex
- lib/feedreader/application.ex
- config/dev.exs
- lib/feedreader_web/endpoint.ex
- .igniter.exs
- lib/feedreader/workers/scheduler.ex
- lib/feedreader_web/components/core_components.ex
- priv/repo/migrations/.formatter.exs
- config/prod.exs
- lib/feedreader_web/live/entry_live/index.ex
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,css}: Implement subtle micro-interactions such as button hover effects and smooth transitions
Focus on delightful details like hover effects, loading states, and smooth page transitions
Files:
priv/static/css/app.css
**/*.css
📄 CodeRabbit inference engine (AGENTS.md)
Ensure clean typography, spacing, and layout balance for a refined, premium look
Files:
priv/static/css/app.css
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: kasuboski/feedreader
Timestamp: 2026-06-21T18:03:17.401Z
Learning: Use `mise run pre-commit` alias when you are done with all changes and fix any pending issues
Learnt from: CR
Repo: kasuboski/feedreader
Timestamp: 2026-06-21T18:03:17.401Z
Learning: Use sqlite and parrot for all storage
Learnt from: CR
Repo: kasuboski/feedreader
Timestamp: 2026-06-21T18:03:17.401Z
Learning: Produce world-class UI designs with a focus on usability, aesthetics, and modern design principles
🪛 HTMLHint (1.9.2)
receipt.html
[error] 338-338: Special characters must be escaped : [ < ].
(spec-char-escape)
🪛 LanguageTool
.pi/skills/gleam-testing/SKILL.md
[style] ~281-~281: Consider using “who” when you are referring to a person instead of an object.
Context: ...test.gleam. The dispatcher is an actor that receives SessionEvents, emits :telem...
(THAT_WHO)
AGENTS.md
[style] ~5-~5: Consider using a different verb for a more formal wording.
Context: ... when you are done with all changes and fix any pending issues - Use sqlite and [pa...
(FIX_RESOLVE)
SCOUTING_REPORT.md
[uncategorized] ~65-~65: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...nread/Starred/History/Feeds) is fine as full page loads. This makes HTMX the natural fit ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~129-~129: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ad + star toggles must update without a full page refresh. That is HTMX's exact sweet spo...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~135-~135: ‘at a fraction of’ might be wordy. Consider a shorter alternative.
Context: ...sor to the current Phoenix LiveView app at a fraction of the complexity. **Styling is preserved...
(EN_WORDINESS_PREMIUM_AT_A_FRACTION_OF)
PLAN.md
[uncategorized] ~143-~143: The official name of this software platform is spelled with a capital “H”.
Context: ...yaml(volume for*.db). **6.3 CI** — .github/workflows/ci.yml: gleam format --chec...
(GITHUB)
E2E.md
[style] ~612-~612: Consider an alternative for the overused word “exactly”.
Context: ... to catch encoding regressions (this is exactly the class of bug that killed `parsed_it...
(EXACTLY_PRECISELY)
🪛 markdownlint-cli2 (0.22.1)
STATUS.md
[warning] 55-55: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 112-112: Spaces inside code span elements
(MD038, no-space-in-code)
SCOUTING_REPORT.md
[warning] 15-15: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 28-28: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 188-188: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 237-237: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 257-257: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 270-270: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 276-276: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 284-284: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🪛 Stylelint (17.13.0)
priv/static/css/app.css
[error] 6-6: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 7-7: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 10-10: Unexpected unknown at-rule "@plugin" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 15-15: Unexpected unknown at-rule "@plugin" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🪛 Trivy (0.69.3)
Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🪛 zizmor (1.25.2)
.github/workflows/ci.yml
[warning] 13-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
| import feedreader/web/html as view | ||
| import gleam/int | ||
| import gleam/list | ||
| import gleam/option.{None, Some} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify `option.Option` usage and imports
rg -n 'import gleam/option|option\.Option' src/feedreader/web/pages.gleamRepository: kasuboski/feedreader
Length of output: 218
🏁 Script executed:
# Search for all uses of option.Option to see import patterns in context
rg -l 'option\.Option' src/Repository: kasuboski/feedreader
Length of output: 95
🏁 Script executed:
# Check if there are other files that import gleam/option and how they do it
rg -B2 'option\.Option' src/ | head -50Repository: kasuboski/feedreader
Length of output: 445
🏁 Script executed:
# Look for the gleam.toml or any build configuration to understand Gleam version
fd -e 'toml|lock' | head -20Repository: kasuboski/feedreader
Length of output: 46
🏁 Script executed:
# Read the imports section of pages.gleam to see all imports
head -20 src/feedreader/web/pages.gleamRepository: kasuboski/feedreader
Length of output: 666
🏁 Script executed:
# Check if there's a gleam.toml file in the root
cat gleam.toml 2>/dev/null || echo "Not found at root"Repository: kasuboski/feedreader
Length of output: 1207
🏁 Script executed:
# Search for any existing errors or build outputs
find . -type f -name "*.txt" -o -name "*error*" -o -name "*build*" | grep -E '(error|build|log)' | head -10Repository: kasuboski/feedreader
Length of output: 46
🏁 Script executed:
# Search for other files that use gleam/option to see import patterns
rg -l 'gleam/option' src/ --type gleamRepository: kasuboski/feedreader
Length of output: 94
🏁 Script executed:
# Check if option.Option is used anywhere else in the codebase
rg 'option\.' src/ --type gleam | head -20Repository: kasuboski/feedreader
Length of output: 94
🏁 Script executed:
# Look for full module imports (import gleam/X pattern without curly braces)
rg 'import gleam/\w+$' src/ --type gleam | head -20Repository: kasuboski/feedreader
Length of output: 94
🏁 Script executed:
# Search for gleam/option imports across all files
rg 'import gleam/option' src/Repository: kasuboski/feedreader
Length of output: 876
🏁 Script executed:
# Look at the full pages.gleam file to understand the context better
wc -l src/feedreader/web/pages.gleamRepository: kasuboski/feedreader
Length of output: 99
🏁 Script executed:
# Check lines around 163 and 170 to see the actual usage
sed -n '155,175p' src/feedreader/web/pages.gleamRepository: kasuboski/feedreader
Length of output: 586
🌐 Web query:
Gleam import system selective import option.Option syntax
💡 Result:
In Gleam, you can perform selective imports—also known as unqualified imports—by specifying the names of the functions, types, or constructors you wish to bring into the local scope within curly braces following the module path [1][2]. The syntax for a selective import is: import module_name.{name1, name2,...} Key aspects of this system include: Unqualified Function and Constant Imports: To import functions or constants so they can be used without the module prefix, list them inside the braces [1][2]. For example: import gleam/list.{map, reverse} Type Imports: Types are imported using the type keyword before the type name to distinguish them from values [3][4]. For example: import gleam/option.{type Option} Constructor Imports: Constructors for custom types are imported as if they were functions [5]. If you need to import both a type and its constructors, you list them together [5]: import my_module.{type MyType, ConstructorA, ConstructorB} Renaming Imports: You can rename modules or imported items using the as keyword to avoid name clashes or to provide clarity [2][6]. Module renaming: import gleam/int as i Item renaming: import gleam/list.{map as m} While Gleam supports this selective import syntax, the official documentation and community conventions generally recommend using qualified imports (e.g., list.map) for functions and constants to maintain code readability and avoid potential naming conflicts [1][2][7]. Unqualified imports are considered more idiomatic for types and their constructors [3][7].
Citations:
- 1: https://tour.gleam.run/basics/unqualified-imports/
- 2: https://exercism.org/tracks/gleam/concepts/modules
- 3: https://tour.gleam.run/basics/type-imports/
- 4: https://csfoundations.cs.aalto.fi/en/courses/modern-and-emerging-programming-languages/part-3/7-modules-and-dependencies
- 5: Importing type variants gleam-lang/gleam#3232
- 6: Do not permit multiple imports to use the same name gleam-lang/gleam#1696
- 7: https://github.com/gleam-lang/website/blob/patterns/documentation/conventions-patterns-anti-patterns.djot
🏁 Script executed:
# Check if there are any other imports that might bring in option.Option
grep -n 'import' src/feedreader/web/pages.gleamRepository: kasuboski/feedreader
Length of output: 599
🏁 Script executed:
# See what other files that use option.Option actually import
rg 'option\.' src/feedreader/web/pages.gleam -B5 | head -30Repository: kasuboski/feedreader
Length of output: 550
🌐 Web query:
Gleam full module import vs selective import qualified names
💡 Result:
In Gleam, module imports can be handled via qualified or unqualified (selective) imports. Qualified imports (the default) require using the module name as a prefix to access its members, which is the idiomatic approach for functions and constants [1][2][3]. Unqualified imports allow specific members to be accessed directly without the prefix [1][3]. Qualified Imports: When you import a module without curly braces, you must use a qualified name to access its exported functions, types, or constants [1][3]. By default, the name used to refer to the module is the last part of its path, although this can be customized using the as keyword [1][3]. Example: import gleam/list // Access via list.map list.map([1][4], fn(x) { x + 1 }) Unqualified (Selective) Imports: This method allows you to import specific functions or types directly into the current module's namespace [1][3]. Example: import gleam/list.{map, reverse} // Access directly map([1][4], fn(x) { x + 1 }) Key Distinctions and Best Practices: 1. Functions and Constants: The Gleam community strongly prefers qualified imports for functions and constants [2]. Using qualified names improves code readability by making it clear where a function originates and prevents naming conflicts [1][2]. 2. Types: Unlike functions, it is common and idiomatic to import types using the unqualified syntax [5][3]. To do this, you must prefix the type name with the type keyword inside the curly braces [5][6][3]. Example: import gleam/string_tree.{type StringTree} 3. No Wildcard Imports: Gleam does not support wildcard imports (e.g., import module.{*}); you must explicitly list the members you wish to import unqualified [7]. 4. Aliasing: You can rename modules during import using the as keyword, which is useful for avoiding naming collisions or simplifying long module paths [1][3]. Overall, while both styles are technically supported, the standard convention is to keep function calls qualified to maintain clarity, while selectively importing types to reduce verbosity [2][3].
Citations:
- 1: https://exercism.org/tracks/gleam/concepts/modules
- 2: https://github.com/gleam-lang/website/blob/patterns/documentation/conventions-patterns-anti-patterns.djot
- 3: https://tour.gleam.run/everything/
- 4: ✨ Allow imports to be aliased with
as _. gleam-lang/gleam#2236 - 5: https://tour.gleam.run/basics/type-imports/
- 6: https://csfoundations.cs.aalto.fi/en/courses/modern-and-emerging-programming-languages/part-3/7-modules-and-dependencies
- 7: https://stackoverflow.com/questions/78983589/how-do-i-import-a-whole-library
Import type Option from gleam/option to use option.Option in type annotations.
The code references option.Option in type signatures (lines 163, 170) but the import statement import gleam/option.{None, Some} is missing type Option. In Gleam, types must be explicitly imported using the type keyword. This will cause a compilation error.
Suggested fix
-import gleam/option.{None, Some}
+import gleam/option.{type Option, None, Some}Also applies to: 163-163, 170-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/feedreader/web/pages.gleam` at line 10, The import statement for
gleam/option is missing the `type Option` declaration which is required to use
`option.Option` in type annotations elsewhere in the file. Modify the import
statement at line 10 to include `type Option` along with the existing `None` and
`Some` imports from gleam/option, using Gleam's type import syntax which
requires the `type` keyword prefix.
CI used erlef/setup-beam without rebar3-version, so esqlite (a native rebar3/C NIF dep via sqlight) failed to compile at gleam check. Switched to jdx/mise-action which reads mise.toml and provisions gleam + erlang + rebar3 together. Pinned all tool versions for reproducibility. - actions/checkout bumped v4 -> v7.0.0 (SHA-pinned) - jdx/[email protected] (SHA-pinned) - persist-credentials: false on checkout - concurrency group with cancel-in-progress - mise.toml: latest -> exact versions (gleam 1.16.0, erlang 28.3.1, rebar 3.27.0)
Previously `let _ = db.migrate(conn)` swallowed migration errors, allowing the app to boot with an uninitialized schema and crash on every subsequent request. Now handle the Result and log before exiting, matching the existing db.open error pattern. Addresses CodeRabbit review comment on PR #23.
The Gleam parser used guid > id > link priority for external_id, but the original Elixir parser uses guid > link (checking only RSS <guid>, falling back to the link URL). For Atom feeds this produced different external_ids (tag:chown.me,... vs https://chown.me/...), causing the upsert ON CONFLICT to treat existing entries as new — inserting them as unread and flooding the unread page with old entries from 2016-2017. Match the Elixir parser exactly: guid > link, no <id> fallback.
STATUS.md is a local status tracker (already in .gitignore) but was committed before the ignore rule was added. Remove from git tracking; the local file is unaffected.
1 participant
Summary by CodeRabbit
Rewrite
Documentation