Conversation
📝 WalkthroughWalkthroughAdds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/pages/seed.tsx (1)
121-121:⚠️ Potential issue | 🟡 MinorRemove
console.logdebug artifact.This log fires on every resource count during
seedAllresponses and will appear in production browser consoles whenever seeding is enabled.🔧 Proposed fix
- console.log(resource, count); - return `${count} ${lowerCaseResource(resource)}${count === 1 ? "" : "s"}`; + return `${count} ${lowerCaseResource(resource)}${count === 1 ? "" : "s"}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/seed.tsx` at line 121, Remove the debug console.log so it doesn't spam production: find the console.log(resource, count) call in the seedAll flow in ui/src/pages/seed.tsx (the line that logs resource and count during seeding) and delete it (or replace it with a debug-only or environment-gated logger if you need retained visibility), ensuring no other logic depends on that expression.
🧹 Nitpick comments (2)
server/lib/orcasite/radio/seed/utils.ex (1)
14-16: Optional: consolidate the twoAsh.Resource.Info.attributelookups into one.
writable_attr?andinclude_attribute_input?each independently callAsh.Resource.Info.attribute(resource, key). Combining them into a single lookup per key avoids the redundancy.♻️ Proposed refactor
Replace the two helpers at the call site with a single guard clause:
- writable_attr?(resource, attr) and include_attribute_input?(resource, attr, val) -> + match?(%{writable?: true}, Ash.Resource.Info.attribute(resource, attr)) and + (match?(%{allow_nil?: true}, Ash.Resource.Info.attribute(resource, attr)) or + not is_nil(val)) ->Or, more readably, add a combined helper:
def writable_and_includable?(resource, key, value) do case Ash.Resource.Info.attribute(resource, key) do %{writable?: true, allow_nil?: false} when is_nil(value) -> false %{writable?: true} -> true _ -> false end endand call it as:
- writable_attr?(resource, attr) and include_attribute_input?(resource, attr, val) -> + writable_and_includable?(resource, attr, val) ->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/orcasite/radio/seed/utils.ex` around lines 14 - 16, Consolidate the duplicate Ash.Resource.Info.attribute(resource, key) lookups by introducing a combined predicate (e.g., writable_and_includable?/3) that does a single attribute fetch and returns true only when the attribute exists, is writable, and passes the include/allow_nil checks currently performed by writable_attr? and include_attribute_input?; then replace the cond clause that calls both writable_attr?(resource, attr) and include_attribute_input?(resource, attr, val) with a single call to writable_and_includable?(resource, attr, val) and remove the redundant per-call attribute lookups in writable_attr?/include_attribute_input? where appropriate (or forward them to the new helper).server/lib/orcasite/radio/seed.ex (1)
64-81:resource!exceptions escape thewithblock — consider error-tolerant accumulation per feed.
withonly pattern-matches return values; it does not rescue exceptions. If__MODULE__.resource!/1raises for a single{feed, resource}pair, the exception propagates throughStream.map |> Enum.to_list(), discarding all previously accumulated results and returning an opaque runtime error to the GraphQL caller instead of a structured{:error, ...}tuple.For
seeds.exsthis is acceptable (fail loud). For theseedAllGraphQL mutation, a single failing feed's resource aborts the entire operation without indicating which feed/resource failed or returning partial results.♻️ Proposed refactor — error-tolerant accumulation
seed_params - |> Stream.map(fn {feed, resource} -> - __MODULE__.resource!(%{ - resource: resource, - start_time: start_time, - end_time: end_time, - feed_id: feed.id - }) - end) - |> Enum.to_list() - |> then(&{:ok, &1}) + |> Enum.flat_map(fn {feed, resource} -> + case __MODULE__.resource(%{ + resource: resource, + start_time: start_time, + end_time: end_time, + feed_id: feed.id + }) do + {:ok, results} -> + results + + {:error, error} -> + require Logger + Logger.warning("Failed to seed #{resource} for feed #{feed.id}: #{inspect(error)}") + [] + end + end) + |> then(&{:ok, &1})The same pattern applies to
latest(lines 91–107) withlatest_resource.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/orcasite/radio/seed.ex` around lines 64 - 81, The current implementation calls __MODULE__.resource!/1 inside Stream.map/Enum.to_list which lets exceptions escape the with block; wrap calls to resource!/1 (and similarly latest_resource) in a safe wrapper that rescues exceptions and returns a tagged result (e.g. {:ok, result} or {:error, {feed, resource, reason}}) so a single failure does not crash the whole operation, then partition or reduce the collected results into a final {:ok, results} with partial successes and a structured list of errors (or {:error, errors} if you prefer) before returning from the with; reference the seed_and_load_feeds call and the mapping that currently invokes __MODULE__.resource!/1 (and the latest -> latest_resource invocation) to locate where to add the try/rescue wrapper and aggregation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ui/src/pages/seed.tsx`:
- Line 121: Remove the debug console.log so it doesn't spam production: find the
console.log(resource, count) call in the seedAll flow in ui/src/pages/seed.tsx
(the line that logs resource and count during seeding) and delete it (or replace
it with a debug-only or environment-gated logger if you need retained
visibility), ensuring no other logic depends on that expression.
---
Nitpick comments:
In `@server/lib/orcasite/radio/seed.ex`:
- Around line 64-81: The current implementation calls __MODULE__.resource!/1
inside Stream.map/Enum.to_list which lets exceptions escape the with block; wrap
calls to resource!/1 (and similarly latest_resource) in a safe wrapper that
rescues exceptions and returns a tagged result (e.g. {:ok, result} or {:error,
{feed, resource, reason}}) so a single failure does not crash the whole
operation, then partition or reduce the collected results into a final {:ok,
results} with partial successes and a structured list of errors (or {:error,
errors} if you prefer) before returning from the with; reference the
seed_and_load_feeds call and the mapping that currently invokes
__MODULE__.resource!/1 (and the latest -> latest_resource invocation) to locate
where to add the try/rescue wrapper and aggregation logic.
In `@server/lib/orcasite/radio/seed/utils.ex`:
- Around line 14-16: Consolidate the duplicate
Ash.Resource.Info.attribute(resource, key) lookups by introducing a combined
predicate (e.g., writable_and_includable?/3) that does a single attribute fetch
and returns true only when the attribute exists, is writable, and passes the
include/allow_nil checks currently performed by writable_attr? and
include_attribute_input?; then replace the cond clause that calls both
writable_attr?(resource, attr) and include_attribute_input?(resource, attr, val)
with a single call to writable_and_includable?(resource, attr, val) and remove
the redundant per-call attribute lookups in
writable_attr?/include_attribute_input? where appropriate (or forward them to
the new helper).
2 participants
Summary
This fixes two seed data issues:
NoSuchInput / Required field :maintainer_emails during Orcasite.Radio.Seed.feedsRoot cause
Feed seeding failing due to input mismatches (Codex diagnosis and fix):
Changes
feed.ex
utils.ex
seed.ex
seeds.exs
seed.tsx
Validation
Notes