-
Notifications
You must be signed in to change notification settings - Fork 1
.pr_agent_accepted_suggestions
| PR 115 (2026-04-26) |
[security] Missing cache-control headers
Missing cache-control headers
`send_json_download/3` returns potentially sensitive result payloads without any cache-control directives, so intermediaries/browsers may cache responses unexpectedly. This increases the risk of data leakage when Aludel is deployed behind shared proxies or on shared machines.Export downloads can be cached by browsers/proxies because the response lacks explicit cache-control headers.
send_json_download/3 currently sets only content-type and content-disposition before sending the JSON body.
- lib/aludel/web/export_controller.ex[30-35]
Add defensive cache headers for downloads, e.g.:
cache-control: no-store, max-age=0- optionally
pragma: no-cacheandexpires: 0for legacy behavior. Ensure the headers are set beforesend_resp/3.
[maintainability] Manual download response building
Manual download response building
`send_json_download/3` manually constructs the download response (headers + `Jason.encode!`) instead of using Phoenix’s download helpers, increasing the chance of subtle header/encoding edge cases and making the code harder to evolve. Centralizing this behavior via `Phoenix.Controller.send_download/2` (or equivalent) would reduce maintenance risk.The controller hand-rolls download response construction (headers + encoding), which is more error-prone and less maintainable than using Phoenix’s built-in download helper.
send_json_download/3 sets headers and sends a binary response body directly.
- lib/aludel/web/export_controller.ex[30-35]
Refactor send_json_download/3 to use Phoenix.Controller.send_download/2 (or the project-standard download helper) with:
{:binary, Jason.encode!(payload, pretty: true)}filename: filename-
content_type: "application/json"Then layer any additional headers (like cache-control) in the same helper.
| PR 113 (2026-04-26) |
[maintainability] Raw `html =~` assertions added
Raw `html =~` assertions added
The new LiveView test asserts against raw rendered HTML strings (`html =~ ...`) instead of using selector-based LiveViewTest helpers. This makes tests brittle and violates the requirement to use selectors tied to stable DOM IDs.The test uses raw HTML string assertions (assert html =~ ...) which is disallowed; tests must use Phoenix.LiveViewTest selector helpers and target stable DOM IDs.
The new test renders missing metrics as N/A and shows callback metadata currently asserts "N/A / N/A" and "N/A" via html =~ ..., which is brittle and violates the compliance rule.
- test/aludel_web/live/run_live/show_test.exs[163-201]
- lib/aludel/web/live/run_live/show.html.heex[104-125]
[reliability] Metadata encoding can crash
Metadata encoding can crash
`Aludel.Web.RunLive.Show.format_result_metadata/1` uses `Jason.encode!/2`, so any non-JSON-encodable value in `result.metadata` will raise and crash the run results LiveView render. Since `RunResult.metadata` is a plain `:map` with no encodability validation, this failure can be triggered by any code path that populates metadata with unsupported terms.format_result_metadata/1 uses Jason.encode!/2, which will raise if metadata contains a term Jason can’t encode. Because the HEEx template calls this during render, a single bad metadata payload can crash the entire run results page.
- Metadata is stored/cast as an Ecto
:mapand currently has no validation ensuring it’s JSON-encodable. - Even if most metadata originates from JSON, defensive rendering avoids page crashes and makes the UI resilient to unexpected inputs.
- lib/aludel/web/live/run_live/show.ex[60-79]
- lib/aludel/web/live/run_live/show.html.heex[186-195]
- lib/aludel/runs/run_result.ex[18-54]
- Replace
Jason.encode!/2withJason.encode/2and render a safe fallback string (e.g.,"(unable to encode metadata)"orinspect(metadata, pretty: true, limit: ...)) when encoding fails. - Optionally add a changeset validation to ensure
metadatais JSON-encodable (so bad payloads fail earlier at persistence time).
| PR 90 (2026-04-09) |
[maintainability] `to_form` uses raw params
`to_form` uses raw params
The refactored test case form is driven by `to_form/2` over plain maps/params instead of a changeset-backed form, so changeset validation and error integration are not standardized as required. This undermines consistent LiveView validation/error handling expected by the compliance checklist.The test-case edit/validate/save form is currently built from plain maps/params (to_form(form_params, ...) and to_form(test_case_params, ...)) instead of a changeset-backed form, violating the required to_form(changeset) standard.
Compliance requires LiveView forms to be standardized around changesets for consistent validation and error rendering.
- lib/aludel/web/live/suite_live/show.ex[185-266]
- lib/aludel/evals/test_case_editor.ex[28-45]
[reliability] Validate event can crash
Validate event can crash
handle_event("validate_test_case") pattern-matches on {:ok, assertions} from AssertionParser.parse/2; parse/2 can now return {:error, msg} (e.g., invalid/missing assertion type), causing a MatchError and crashing the LiveView process.Aludel.Web.SuiteLive.Show.handle_event("validate_test_case", ...) currently does {:ok, assertions} = AssertionParser.parse(...). Since AssertionParser.parse/2 can return {:error, message} after this refactor (it validates types/fields), the LiveView can crash with MatchError when form params are invalid or incomplete.
This handler runs on form validation events; it should be resilient to invalid client params and keep the socket alive.
- lib/aludel/web/live/suite_live/show.ex[216-233]
- lib/aludel/evals/assertion_parser.ex[24-32]
- lib/aludel/evals/assertion_parser.ex[34-55]
Replace the pattern match with a case (or with ... else) that:
- on
{:ok, assertions}updates:editing_assertions - on
{:error, message}avoids crashing (e.g., keep prior assertions and optionally set a validation flash or form error)
[observability] Changeset error discarded
Changeset error discarded
`DocumentIngestion.persist_document/3` drops the returned changeset and replaces it with a generic `"Database error"` string, losing actionable error detail. This conflicts with the requirement to return structured/actionable errors rather than obscuring failures.DocumentIngestion.persist_document/3 discards the changeset from Evals.create_test_case_document/1 and returns a generic "Database error", reducing debuggability and obscuring actionable error information.
The project error-handling guideline requires propagating structured errors when they are actionable, and avoiding generic error replacement that hides the underlying failure.
- lib/aludel/evals/document_ingestion.ex[34-47]
[reliability] Visual index parsing raises
Visual index parsing raises
AssertionParser.parse_visual_assertions/1 uses String.to_integer/1 on client-controlled keys ("assertion_type_" suffix). A non-numeric suffix will raise ArgumentError and can crash callers (LiveView validation/save flows).AssertionParser.parse_visual_assertions/1 does String.to_integer(idx) on the suffix of any key starting with assertion_type_. If a client submits assertion_type_abc, it will raise and crash the request/LiveView.
The assertions param keys are user-controlled; the parser should treat malformed indices as invalid input, not as an exception.
- lib/aludel/evals/assertion_parser.ex[70-76]
Use safe parsing (e.g., Integer.parse/1) and either:
- ignore keys with non-integer suffixes, or
- return
{:error, "Invalid assertion index ..."}fromparse/2rather than raising
[maintainability] File read error type mismatch
File read error type mismatch
DocumentIngestion.ingest/3 returns `:file.format_error(reason)` for File.read failures, which is a charlist/iodata rather than the declared `String.t()` in ingest_result; this violates the spec and makes the return shape inconsistent.DocumentIngestion.ingest/3 returns :file.format_error(reason) for File.read errors, but the declared type says the reason is a String.t(). This is inconsistent and can confuse callers/dialyzer.
put_upload_flash/3 interpolates the reason, so runtime behavior is OK; this is mainly about making the return type consistent.
- lib/aludel/evals/document_ingestion.ex[9-21]
Wrap the formatted error with to_string/1 (or List.to_string/1) and/or adjust the type to accept iodata consistently.
| PR 85 (2026-04-08) |
[maintainability] `index_test` asserts raw HTML
`index_test` asserts raw HTML
The new LiveView test asserts directly against the rendered HTML string (`html =~ ...`) instead of using selector/element-based assertions with stable IDs/classes. This makes tests brittle and violates the required LiveView testing pattern.A new LiveView test uses raw HTML substring assertions (assert html =~ ...) instead of selector-based assertions.
Compliance requires LiveView tests to target elements via selectors (ideally IDs or stable classes) using Phoenix.LiveViewTest helpers.
- test/aludel_web/live/provider_live/index_test.exs[52-56]
[reliability] Nil LLM config crash
Nil LLM config crash
Aludel.LLM.get_google_api_key/0 indexes into Application.get_env(:aludel, :llm) without a default, so if :aludel/:llm isn’t configured it will raise and crash LLM.call/3 instead of returning {:error, :missing_api_key}. This risk now affects the new :google provider path directly (and the same pattern exists for other providers too).get_google_api_key/0 calls Application.get_env(:aludel, :llm)[:google_api_key]. If :aludel, :llm is not configured, Application.get_env/2 returns nil and the bracket access raises, crashing LLM.call/3.
This PR introduces a new get_google_api_key/0 with this pattern, and the same pattern exists in get_openai_api_key/0 and get_anthropic_api_key/0.
- lib/aludel/interfaces/llm.ex[123-145]
Use Application.get_env(:aludel, :llm, []) (or equivalent) so missing config results in :error and downstream {:error, :missing_api_key} instead of an exception. Apply the same safe access pattern to the OpenAI/Anthropic helpers for consistency.
[reliability] Async env mutation
Async env mutation
The new Google provider tests mutate global application env (`Application.put_env/3`) inside an `async: true` test module, which can race with other async tests and can leak state if a test errors before restoring. This makes the test suite more prone to non-deterministic failures.Google provider tests change global application env in an async test module, which can create test flakiness and state leakage.
Application.put_env/3 affects the entire VM, not just the current test process.
- test/aludel/llm_test.exs[1-3]
- test/aludel/llm_test.exs[374-406]
Pick one (or combine):
- Make this test module non-async (remove
async: true) since it mutates global env. - Wrap env changes with
on_exit(fn -> Application.put_env(:aludel, :llm, original_config) end)(ortry...after) so restoration happens even if the test raises/fails early. - Prefer refactoring to avoid global env mutation entirely if feasible (e.g., inject config or pass api_key via provider config in a way that isn’t overwritten).
| PR 84 (2026-04-08) |
[correctness] `` uses interpolated class
`` uses interpolated class
The new select option check icon builds a conditional class via string interpolation instead of the required HEEx class list syntax. This violates the project’s HEEx class-attribute conventions and can lead to inconsistent/fragile class composition.The <.icon> in the custom select options uses string interpolation in class= to conditionally add is-visible. Our HEEx convention requires conditional/multi classes to be expressed via class={[ ... ]}.
This was introduced in the new custom select dropdown rendering.
- lib/aludel/web/components/core_components.ex[332-335]
[correctness] Hidden select steals focus
Hidden select steals focus
The shared select component renders the real as "sr-only" but keeps it tabbable (and still subject to native required validation), so focus/validation can land on an invisible control and appear broken. Because the CustomSelect hook listens for keydown on the wrapper, it can also prevent default keyboard behavior while focus is on that hidden .The custom select renders a real <select> with sr-only, which remains tabbable and participates in native required validation. This can cause focus/validation to target an invisible element and the wrapper-level key handler can also interfere with keyboard interaction.
The hidden <select> exists to submit form values / work with LiveView, but the visible control is the button trigger. We should avoid letting the hidden element become the primary focus/validation target.
- lib/aludel/web/components/core_components.ex[241-341]
- assets/js/hooks/custom_select.js[38-114]
- In the custom-select (non-multiple) branch, make the hidden
<select>non-tabbable (e.g., addtabindex="-1"). - Avoid native HTML constraint validation focusing the hidden select (either strip
requiredfrom@restfor this hidden select, or ensure forms using this control havenovalidateso server-side errors render instead). - Consider binding
keydownto the visible trigger button (and/or guarding inhandleKeydownso it only runs when focus is on the trigger) to avoid intercepting key events coming from the hidden<select>.
[correctness] isOpen null dropdown bug
isOpen null dropdown bug
CustomSelect.isOpen() returns true when the dropdown element is missing because it negates an optional-chained expression (!undefined === true). In that state, clicking the trigger will always call close() and open() will never run, masking markup regressions and breaking interaction.isOpen() returns true when this.dropdown is null/undefined due to !this.dropdown?.classList.contains(...).
This only triggers when markup is missing or temporarily inconsistent (e.g., partial patch), but when it happens it prevents the dropdown from ever opening.
- assets/js/hooks/custom_select.js[121-137]
Update isOpen() to explicitly require a dropdown element, e.g.:
| PR 83 (2026-04-07) |
[reliability] Mix.env runtime call
Mix.env runtime call
`Aludel.Runs.execute_run/2` calls `Mix.env()` at runtime to choose sequential vs concurrent execution, which can crash in release/production where `:mix` is not started/available. This would break run execution in non-test environments.Aludel.Runs.execute_run/2 uses Mix.env() at runtime to decide between sequential and concurrent execution. This is unsafe in releases/production and can crash run execution.
The sequential behavior is only needed to make tests deterministic/Mox-friendly, but the environment check must not rely on Mix at runtime.
- /lib/aludel/runs.ex[160-187]
- /config/test.exs[1-50]
- Introduce an application config flag (e.g.,
:aludel, :run_execution_mode) with values like:sequential/:concurrent. - In
config/test.exs, set it to:sequential; default to:concurrentin other envs. - Replace the
case Mix.env()withcase Application.get_env(:aludel, :run_execution_mode, :concurrent)(or similar).
| PR 82 (2026-04-07) |
[correctness] Project delete modal missing
Project delete modal missing
PromptLive.Index now assigns `@prompts` to only unassigned prompts, but the delete confirmation modals are rendered only for `@prompts`. Project prompts still render a delete button that opens `confirm-delete-prompt-`, but that modal is never present, breaking deletion for prompts inside projects.Project prompts still render a delete button that calls show_modal("confirm-delete-prompt-#{prompt.id}"), but the modal list is only rendered for @prompts. Since @prompts is now assigned to only unassigned prompts, prompts inside projects have no modal in the DOM, so deletion cannot be confirmed.
- Project prompts are rendered from
project.prompts. - Delete modals are generated from a
:forloop over@prompts. - This PR changed the LiveView to assign
@promptstounassigned_prompts.
- lib/aludel/web/live/prompt_live/index.ex[18-47]
Pick one:
- Keep assigning
:promptsas the full filtered prompt list (not just unassigned) so the modal loop covers all prompts, while still rendering unassigned prompts in the table viaEnum.filter(@prompts, &is_nil(&1.project_id)). - Introduce a new assign like
:all_visible_promptsthat combinesunassigned_promptsand allproject.prompts(dedupe by id), and update the modal:forloop to iterate over that list.
[correctness] Empty selected project hidden
Empty selected project hidden
`maybe_reject_empty_projects/4` rejects empty projects whenever `selected_project_id` is set, even when there are no search/tag filters. Selecting a project with zero prompts results in that project being removed from `@projects`, causing the UI to show the empty state instead of the selected project.When a project is selected and it has zero prompts (or filtering yields zero prompts), maybe_reject_empty_projects/4 removes it. This makes selected empty projects disappear from the listing.
The current match clauses only preserve empty projects when search_query == "" AND selected_tags == [] AND selected_project_id is nil/"".
- lib/aludel/web/live/prompt_live/index.ex[251-272]
Change the base case to preserve projects whenever there are no search/tag filters, regardless of selected_project_id, e.g.:
[correctness] Search trim mismatch
Search trim mismatch
Unassigned prompts are filtered via `Prompts.list_prompts/1`, which trims search input, while project prompts are filtered in-memory using the raw search string. Searches with leading/trailing whitespace can match unassigned prompts but fail to match project prompts, reintroducing inconsistent filtering.DB filtering trims the search query, but in-memory project filtering does not. This can cause different results for the same UI search depending on whether the prompt is unassigned or in a project.
-
Prompts.list_prompts/1usesnormalize_search/1(trim). -
filter_by_search/2lowercases but doesn't trim.
- lib/aludel/web/live/prompt_live/index.ex[18-31]
- lib/aludel/web/live/prompt_live/index.ex[274-283]
Trim search_query once in handle_params/3 (and any other places using assigns for search, like after delete) before passing it to both list_filtered_prompts/3 and filter_projects/4, e.g.:
| PR 70 (2026-04-05) |
[reliability] Unique index raises exception
Unique index raises exception
The DB enforces unique (prompt_id, version), but PromptVersion.changeset/2 does not declare a unique_constraint, so concurrent version creation can raise Ecto.ConstraintError and crash the new transaction instead of returning {:error, changeset}.Concurrent inserts can produce the same (prompt_id, version) due to max(version)+1. The DB unique index will reject one insert, and because the changeset lacks unique_constraint/3, Ecto will raise Ecto.ConstraintError, crashing the request.
This impacts both the existing create_prompt_version/2 and the new insert_prompt_version/3 used inside the transaction.
- lib/aludel/prompts/prompt_version.ex[39-43]
- priv/repo/migrations/20260319000002_create_prompt_versions.exs[15-17]
- lib/aludel/prompts.ex[265-277]
- lib/aludel/prompts.ex[300-309]
- Add
unique_constraint/3(orunique_constraint/2) for the(prompt_id, version)index inPromptVersion.changeset/2(use the correct index name, typically:prompt_versions_prompt_id_version_index). - Optionally: in
insert_prompt_version/3, detect the unique-constraint error and retry version-number assignment a small number of times to make concurrent updates resilient.
[reliability] Nil template can crash
Nil template can crash
maybe_insert_version/3 only skips when template == "", so a nil "template" value will attempt version creation and can crash in extract_variables/1 when Regex.scan is called with a non-binary. normalize_attrs/1 can also propagate nil by copying :template to "template" when :template is present but nil.maybe_insert_version/3 only treats "" as blank. If attrs contain "template" => nil (or :template => nil which normalize_attrs/1 copies), the workflow will try to create a version and call extract_variables/1 with a non-binary template.
This is in the new transactional APIs (create_prompt_with_initial_version/1, update_prompt_with_optional_version/2) and the private helpers maybe_insert_version/3 and insert_prompt_version/3.
- lib/aludel/prompts.ex[137-145]
- lib/aludel/prompts.ex[175-187]
- lib/aludel/prompts.ex[257-277]
- lib/aludel/prompts.ex[289-296]
- Coerce templates to a string (or treat non-binary/nil as blank) before deciding whether to insert a version.
- Update the guard to skip when
templateisnilor whenString.trim(template) == ""(only after confirming it’s binary).
[correctness] Wrong changeset on failure
Wrong changeset on failure
On :prompt_version failure, the context returns a PromptVersion changeset, but PromptLive.New renders it as the prompt form changeset, so version-insert errors won’t be displayed on the relevant fields (and may be silently dropped). This makes transaction failures hard to understand/resolve from the UI.When the :prompt_version Multi step fails, the context returns the PromptVersion changeset directly. The LiveView then renders this changeset as the prompt form, so users won’t see meaningful errors tied to the prompt fields.
The new transactional APIs wrap prompt + version insert/update. The UI expects a Prompt changeset for rendering.
- lib/aludel/prompts.ex[137-155]
- lib/aludel/prompts.ex[175-198]
- lib/aludel/web/live/prompt_live/new.ex[103-126]
- In the transaction
caseclauses for{:error, :prompt_version, changeset, changes_so_far}, convert the failure into a Prompt changeset (e.g., add an error on:templateor:base), using the prompt struct inchanges_so_far[:prompt]when available. - Alternatively, change the return type to include a tagged error (e.g.
{:error, :prompt_version, changeset}) and handle it explicitly in the LiveView.
[correctness] Template compare order fragile
Template compare order fragile
update_prompt_with_optional_version/2 assumes prompt.versions is already ordered newest-first when versions are preloaded; if callers preload versions without an explicit order, latest_template/1 may compare against the wrong version and create/skip versions incorrectly.The version-creation decision relies on the head of prompt.versions being the latest version. If versions are preloaded in an arbitrary order, the comparison can be wrong.
This is a public context function; callers may pass a prompt struct with versions already loaded but not ordered.
- lib/aludel/prompts.ex[175-187]
- lib/aludel/prompts.ex[279-287]
- lib/aludel/prompts/prompt.ex[23-31]
- Make
latest_template/1compute the max version in-memory (e.g.,Enum.max_by(versions, & &1.version, fn -> nil end)) instead of assuming order, OR - Always preload versions with ordering even if already loaded (e.g., force preload), OR
- Query just the latest template from the DB when deciding whether to create a new version.
| PR 68 (2026-04-05) |
[correctness] Empty project_id filters prompts
Empty project_id filters prompts
`filter_projects/4` treats `selected_project_id == ""` as “no project selected”, but `handle_params/3` still forwards `""` into `Prompts.list_prompts/1`, causing the prompt query to filter by an empty project_id and return no results. This can break pagination/filtered views when a `project_id` param exists but is empty (e.g., from URL construction that retains nil values).selected_project_id can be an empty string (""). The new filter_projects/4 treats "" as “no selection”, but handle_params/3 still treats "" as present and forwards it into Prompts.list_prompts/1, which applies a DB filter with p.project_id == "".
- In Elixir,
""is truthy. - This creates a mismatch between the project tree and the paginated prompt query.
- The pagination links always include a
project_idkey and the routing helper does not drop nil values, so empty-value params are plausible.
- lib/aludel/web/live/prompt_live/index.ex[18-35]
- lib/aludel/web/helpers.ex[19-27]
- lib/aludel/web/helpers.ex[68-77]
- lib/aludel/web/live/prompt_live/index.html.heex[233-266]
- lib/aludel/prompts.ex[36-43]
- Normalize
selected_project_idinhandle_params/3(and anywhere else you read it) tonilwhen it is"". - When building
prompts_params, only add:project_idwhen the normalized value is not nil. - Prevent generating query params with nil values (either by dropping nils in
aludel_path/2before encoding, or by conditionally includingproject_idin pagination link param maps). - Optionally harden
Prompts.list_prompts/1to treat""as nil forproject_id.
| PR 67 (2026-04-05) |
[correctness] Activity range off-by-one
Activity range off-by-one
Aludel.Stats.Activity.daily_activity/1 subtracts `days` and then builds an inclusive Date.range to today, so `daily_activity(30)` produces 31 buckets and inflates the dashboard chart totals/averages while the UI labels it “Last 30 Days”.Aludel.Stats.Activity.daily_activity/1 currently returns days + 1 buckets because it subtracts days and then uses an inclusive Date.range/2. This makes the dashboard’s “Last 30 Days” activity chart actually cover 31 days and skews derived UI values (like the displayed average).
The dashboard calls Activity.daily_activity(30) and labels the chart as “Activity — Last 30 Days”, and the UI uses length(@daily_activity) when computing averages.
- lib/aludel/stats/activity.ex[12-53]
- lib/aludel/web/live/dashboard_live.ex[45-50]
- lib/aludel/web/live/dashboard_live.html.heex[164-173]
- Compute the start date as
Date.utc_today() |> Date.add(-(days - 1))(whendays > 0) so the inclusive date range has exactlydayselements. - Keep output ordering oldest→newest as it is today.
- Consider guarding
days <= 0to return[](or a single-day bucket), depending on expected behavior.
[performance] DATE() filter forces scans
DATE() filter forces scans
daily_activity/1 uses `DATE(inserted_at)` in the WHERE clause, which forces calendar-day filtering and prevents using a plain timestamp range filter (and any future `inserted_at` index), making the dashboard query slower as tables grow.daily_activity/1 filters with DATE(inserted_at) >= start_date, which applies a function to the DB column in the WHERE clause. This prevents a plain timestamp range filter and will force less efficient plans as runs/suite_runs grow.
The rows are timestamped with :utc_datetime, so the query can filter by inserted_at >= start_datetime (and optionally < end_datetime) while still grouping by day for bucketing.
- lib/aludel/stats/activity.ex[13-37]
- Compute
start_datetime = DateTime.new!(start_date, ~T[00:00:00], "Etc/UTC")(or equivalent) and filter withwhere: r.inserted_at >= ^start_datetime(and similarly forSuiteRun). - Keep the grouping/selection as daily buckets via
DATE(inserted_at)(ordate_trunc('day', inserted_at)), but avoid wrappinginserted_atin the WHERE clause. - If you need an exact
dayswindow, optionally add an upper bound:where: inserted_at >= ^start_datetime and inserted_at < ^end_datetime.
| PR 64 (2026-04-04) |
[reliability] Tests ignore `#provider-form` id
Tests ignore `#provider-form` id
New provider LiveView tests use `form("form", ...)` rather than selecting via the template’s explicit `id="provider-form"`. This violates the requirement that tests use key element IDs for stable selectors.Provider LiveView tests submit the form using the generic selector form("form", ...) even though the template defines id="provider-form".
Compliance requires tests to use explicit DOM IDs for stable selectors.
- test/aludel_web/live/provider_live/new_test.exs[30-36]
[reliability] Tests assert `html =~ ...`
Tests assert `html =~ ...`
New LiveView tests assert against raw HTML strings (e.g., `assert html =~ ...`) instead of using element-based assertions (`element/2`, `has_element?/2`). This violates the requirement for resilient, selector-based LiveView tests.LiveView tests are asserting with raw HTML substring matches (e.g., assert html =~ ...) instead of using selector-based assertions.
Compliance requires element/2, has_element?/2, etc., to reduce brittleness when markup changes.
- test/aludel_web/live/provider_live/new_test.exs[46-61]
| PR 55 (2026-04-03) |
[correctness] `save_suite_metadata` uses raw form
`save_suite_metadata` uses raw form
The suite metadata edit UI adds a new `project_id` field inside a plain HTML `` and reads raw params in the LiveView, rather than using a `to_form` assign with `` and `` fields.The suite metadata edit form (including the newly added project_id field) is implemented as a raw HTML <form> and handled via raw param extraction, rather than using to_form assigns and <.form>/<.input>.
This pattern is explicitly disallowed by the LiveView form conventions in the compliance checklist and can cause inconsistencies with validation/rendering.
- lib/aludel/web/live/suite_live/show.html.heex[35-50]
- lib/aludel/web/live/suite_live/show.ex[68-80]
[correctness] Blank project names possible
Blank project names possible
`Aludel.Projects.Project.changeset/2` trims `:name` after `validate_required/1` and `validate_length/3`, so a whitespace-only name can pass validations and then be persisted as an empty string. The same ordering can also incorrectly fail otherwise-valid names due to leading/trailing whitespace before trimming.Project.changeset/2 trims the :name after validations. This allows whitespace-only names to slip through (validated pre-trim, stored post-trim) and can also reject valid names because length is checked before trimming.
Projects are user-facing grouping entities; storing blank names breaks navigation and can lead to confusing UI.
- lib/aludel/projects/project.ex[34-40]
Reorder the pipeline so trimming happens before validations, e.g.:
castupdate_change(:name, &String.trim/1)validate_required(:name)-
validate_length(:name, min: 1, max: 255)(Optionally, consider usingvalidate_changeto explicitly reject names that become empty after trimming.)
[correctness] Project preselect ignored
Project preselect ignored
`SuiteLive.New` ignores request params in `apply_action/3` for `:new`, so navigating to `/suites/new?project_id=...` does not preselect the project in the form. The Suite index explicitly links to that route with `project_id`, so the intended UX is currently broken.SuiteLive.New.apply_action/3 for :new discards params and therefore doesn’t use the project_id passed from the index page.
The suites index links to suites/new with %{"project_id" => project.id} to preselect the project.
- lib/aludel/web/live/suite_live/new.ex[144-158]
Change apply_action(socket, :new, _params) to accept params and:
- read
project_id = params["project_id"] - build initial attrs for the changeset (e.g.,
%{"project_id" => project_id}when present) - assign
:suitewith thatproject_idso the<option selected=...>logic works.
[performance] Overeager project preloads
Overeager project preloads
`Projects.list_projects/0` always preloads both `:prompts` and `suites: :prompt`, but several call sites only need `{id, name}` for a select (suite new/show), and the prompts index needs prompts but not suites. This adds unnecessary DB work and memory usage on common page loads.Projects.list_projects/0 always preloads prompts and suites, which is unnecessary for dropdown-only use cases and for the prompts index (which doesn’t use suites).
This function is now a shared dependency across multiple LiveViews; keeping it “always preload everything” can become a scaling bottleneck.
- lib/aludel/projects.ex[17-23]
- lib/aludel/web/live/suite_live/new.ex[144-155]
- lib/aludel/web/live/suite_live/show.ex[24-30]
- lib/aludel/web/live/prompt_live/index.ex[17-26]
Introduce separate APIs, for example:
-
list_projects_for_select/0(no preloads, possiblyselect: [:id, :name]) -
list_projects_with_prompts/0(preload:prompts) -
list_projects_with_suites/0(preloadsuites: :prompt) Then update each LiveView to call the narrowest function it needs.
| PR 53 (2026-04-03) |
[correctness] Log context not printed
Log context not printed
`Aludel.Runs` logs `provider_id` and `reason` as Logger metadata when `create_run_result/1` fails, but the configured Logger formatter only outputs `:request_id` metadata (and dev outputs no metadata). As a result, the warning logs won’t include provider/error details in actual log output, undermining the PR’s debugging goal.Logger.warning/2 is currently called with reason: and provider_id: metadata, but the project’s Logger formatter does not print these metadata fields (and dev prints none). This means the logs won’t show the error details/provider context that the PR intends to add.
- The warning message only contains the run ID.
-
reasonandprovider_idare passed as metadata, but the formatter configuration only outputs:request_idmetadata.
Choose one:
- Embed the context in the log message string (recommended to avoid global config changes), and update tests to assert the provider_id/reason are present in the captured log output.
-
Update Logger formatter metadata config to include
:provider_idand:reason(and optionally:run_id) and ensure dev/prod formatting prints them.
- lib/aludel/runs.ex[220-246]
- config/config.exs[47-51]
- config/dev.exs[52-54]
- test/aludel/runs_test.exs[393-435]
| PR 52 (2026-04-03) |
[reliability] Fork PRs fail CI
Fork PRs fail CI
The `CI` workflow runs on `pull_request` but unconditionally performs a Codecov upload using `secrets.CODECOV_TOKEN` and now fails the job on any upload error. For fork-based PRs (the documented contribution workflow), repository secrets are not available, so the Codecov step can fail and block external contributions.The CI workflow unconditionally uploads coverage to Codecov on pull_request and now hard-fails the job on upload errors. On forked PRs, repository secrets (including CODECOV_TOKEN) are not available, so the upload step can fail and block contributions.
- Workflow triggers on
pull_request. - Codecov upload step uses
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}and setsfail_ci_if_error: true. - CONTRIBUTING.md instructs contributors to fork the repository.
- Add an
if:guard so the Codecov upload runs only when the PR is from the same repository (or whensecrets.CODECOV_TOKENis available), e.g. skip on forks. - Alternatively, conditionally set
fail_ci_if_error/step behavior for fork PRs.
Use a condition such as:
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
- .github/workflows/ci.yml[68-76]
[reliability] CI coupled to Codecov uptime
CI coupled to Codecov uptime
By removing the prior error-tolerance and setting `fail_ci_if_error: true`, any transient Codecov upload issue will now fail the entire unit test job even when tests/coverage generation succeeded. This can introduce CI flakiness unrelated to code correctness and block merges during external service incidents.The unit test job will now fail whenever Codecov upload errors, which can cause flaky CI when Codecov/network is transiently unavailable.
The Codecov upload is a reporting step after tests; coupling overall CI success to its availability can block merges for reasons unrelated to test correctness.
- Consider scoping hard-fail behavior to
pushonmainwhile keeping PR uploads non-blocking, OR add retry/backoff if supported.
- .github/workflows/ci.yml[68-76]
- Prompts
- Providers
- Runs and Execution
- Evaluation Suites
- Regex Assertions
- Metric Context
- Evaluator Execution Details
- Rubric Judges
- Judge Catalog
- Repeated Sampling
- Quality Policies
- ExUnit Evaluations
- File-Based Suites
- Evaluation Reporters
- Datasets
- Red-Team Datasets
- Generated Red-Team Cases
- Analytics and Prompt Evolution
- Exports and CI
- Documents and Storage
- Embedding and Access