elementary/monitor/dbt_project/macros/artifacts/get_dbt_run_result.sql (1)

7-28: Duplicated projection definition with get_dbt_run_results.sql.

The heavy_cols / base_cols blocks are byte-identical to the ones in get_dbt_run_results.sql. If a schema column is added/removed, both macros must be edited in lockstep — easy to miss. Consider extracting into a tiny shared macro (e.g., _dbt_run_results_projection(include_heavy)) in _filter_helpers.sql or a new _projections.sql, and using it from both call sites.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/monitor/dbt_project/macros/artifacts/get_dbt_run_result.sql`
around lines 7 - 28, The projection logic in get_dbt_run_result.sql duplicates
the identical heavy_cols and base_cols definitions from get_dbt_run_results.sql,
which risks drift when schema changes; extract these into a single reusable
macro (e.g., _dbt_run_results_projection(include_heavy)) placed in a shared file
like _filter_helpers.sql or _projections.sql, then replace the
base_cols/heavy_cols and projection construction in both get_dbt_run_result.sql
and get_dbt_run_results.sql to call _dbt_run_results_projection(include_heavy)
so both macros use the same canonical column list.

elementary/monitor/dbt_project/macros/artifacts/get_dbt_models.sql (1)

40-45: Consider a two-column LIKE helper.

The name/alias block is duplicated verbatim in get_dbt_tests.sql (which adds short_name). If you want to trim duplication, a small _like_ci_any_filter(columns, value) helper in _filter_helpers.sql that ORs across a list of columns would collapse both sites. Optional.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/monitor/dbt_project/macros/artifacts/get_dbt_models.sql` around
lines 40 - 45, Create a reusable two-column case-insensitive LIKE helper to
remove the duplicated name/alias logic: add a macro named
_like_ci_any_filter(columns, value) in _filter_helpers.sql that accepts a list
of column names and the value, escapes the value via
elementary_cli._sql_escape(value), and returns an OR-clause like lower(col) LIKE
lower('%...%') for each column (joined with OR). Replace the duplicated block in
get_dbt_models.sql (the name/alias predicate) and the similar block in
get_dbt_tests.sql (which also checks short_name) by calling
_like_ci_any_filter(['name','alias'] , name) or
_like_ci_any_filter(['name','alias','short_name'], name) as appropriate. Ensure
the macro returns a properly parenthesized expression and is only rendered when
value is not none to preserve existing conditional behavior.

elementary/artifacts/entities/models.py (1)

108-117: Consolidate repeated error handling across entity commands.

This except ArtifactFetchError / except Exception block is duplicated verbatim in models.model (L154-163) and across test_results.py, run_results.py (and presumably the other three entities). Consider extracting a small helper or decorator (or reusing cli._handle_fetch_error plus a generic internal-error helper) so every entity command shares one implementation. This also addresses Ruff BLE001 (blind except Exception) in one place instead of six.

♻️ Example helper

# elementary/artifacts/common.py (or output.py)
from functools import wraps

def handle_cli_errors(func):
    `@wraps`(func)
    def wrapper(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except ArtifactFetchError as exc:
            sys.exit(emit_error(str(exc), exc.code, exc.details))
        except Exception as exc:  # noqa: BLE001
            sys.exit(emit_error(
                f"Unexpected error: {exc}",
                ErrorCode.INTERNAL_ERROR,
                {"type": type(exc).__name__},
            ))
    return wrapper

Then decorate each entity command with @handle_cli_errors and drop the inline try/except.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/artifacts/entities/models.py` around lines 108 - 117, Extract the
duplicated try/except into a single reusable helper (e.g., handle_cli_errors) in
a shared module (like elementary.artifacts.common or output) that catches
ArtifactFetchError and generic Exception and calls emit_error with the same
payloads; alternatively, compose/extend the existing cli._handle_fetch_error by
adding a small internal-error helper that handles the broad Exception case
(using ErrorCode.INTERNAL_ERROR and type(exc).__name__) so callers can reuse it.
Then remove the repeated except blocks in models.model and the commands in
test_results.py, run_results.py, etc., and either decorate those command
functions with `@handle_cli_errors` or call the shared handler wrapper so all
entity commands use one implementation and Ruff BLE001 is isolated in one place.

elementary/artifacts/entities/run_results.py (1)

133-133: Inconsistent macro parameter naming for the same user-facing flag.

The same --include-compiled-code CLI flag is forwarded to the list macro as lightweight=not include_compiled_code (L133) and to the single-get macro as include_compiled_code=include_compiled_code (L182). The inverted sense + different parameter names between get_dbt_run_results and get_dbt_run_result makes future maintenance error-prone (easy to flip the boolean). Consider aligning both SQL macros on one parameter name (e.g., include_compiled_code) and keeping the boolean polarity identical in both call sites.

Also applies to: 182-182

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/artifacts/entities/run_results.py` at line 133, The list macro
call in get_dbt_run_results uses the parameter name lightweight with inverted
polarity ("lightweight = not include_compiled_code") while get_dbt_run_result
passes include_compiled_code directly; standardize both to the same parameter
name and polarity to avoid confusion—update the call in get_dbt_run_results to
pass "include_compiled_code = include_compiled_code" (or, alternatively, change
get_dbt_run_result to use "lightweight = not include_compiled_code") so both
macros use the identical parameter name (include_compiled_code) and the same
boolean sense; adjust any downstream macro signatures if necessary to accept
include_compiled_code.

elementary/artifacts/cli.py (1)

27-34: Remove unused helpers and fix implicit Optional typing.

The functions _handle_fetch_error and _handle_bad_argument are never called within this module or elsewhere. Meanwhile, all entity modules (models, test_results, run_results, sources, tests, invocations) inline the same sys.exit(emit_error(...)) pattern directly. Either adopt these helpers consistently across entities to eliminate duplication, or remove them. Additionally, the details parameter in _handle_bad_argument should explicitly type None as Optional[Dict[str, Any]] to match the signature of emit_error and avoid Ruff RUF013 violations.

♻️ Proposed refactor

-def _handle_fetch_error(exc: ArtifactFetchError) -> None:
-    code = emit_error(str(exc), exc.code, exc.details)
-    sys.exit(code)
-
-
-def _handle_bad_argument(message: str, details: dict = None) -> None:
-    code = emit_error(message, ErrorCode.BAD_ARGUMENT, details or {})
-    sys.exit(code)
+def _handle_fetch_error(exc: ArtifactFetchError) -> None:
+    sys.exit(emit_error(str(exc), exc.code, exc.details))
+
+
+def _handle_bad_argument(message: str, details: Optional[Dict[str, Any]] = None) -> None:
+    sys.exit(emit_error(message, ErrorCode.BAD_ARGUMENT, details))

If adopting these helpers, add from typing import Any, Dict, Optional and refactor entity modules to use them instead of inlining the pattern.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/artifacts/cli.py` around lines 27 - 34, The two helpers
_handle_fetch_error and _handle_bad_argument are unused and _handle_bad_argument
has an implicit Optional typing for details; either remove both helpers or
consistently use them across entity modules instead of inlining
sys.exit(emit_error(...)), and if you keep _handle_bad_argument update its
signature to explicitly type details as Optional[Dict[str, Any]] and add the
required imports (Any, Dict, Optional) so the parameter matches emit_error and
avoids RUF013; ensure references to _handle_fetch_error (ArtifactFetchError) and
_handle_bad_argument are wired into callers or deleted to eliminate dead code.

elementary/artifacts/entities/invocations.py (2)

203-221: Redundant data.length field and _emit_single table uses list-only columns.

Two small output concerns:

1. Line 212: "data": {"length": len(rows)} is redundant with the top-level "count" already emitted on line 209. If this isn't load-bearing for a documented contract it's dead weight in the envelope and risks becoming a stale parallel counter.
2. _emit_single renders the detail record through LIST_COLUMNS, so edr artifacts invocation <id> -o table hides fields that the JSON path exposes (e.g. job_id, job_run_id, timing breakdowns). The JSON envelope already uses a distinct key ("invocation" vs "invocations"); a separate DETAIL_COLUMNS (or pivoted key/value rendering for a single row) would be more useful and mirror what tests.py does with DETAIL_JSON_FIELDS.

♻️ Suggested cleanup for the redundant field

     emit_json(
         {
             "count": len(rows),
             "has_more": has_more,
             "invocations": rows,
-            "data": {"length": len(rows)},
         }
     )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/artifacts/entities/invocations.py` around lines 203 - 221, The
_emit_list function emits a redundant nested data.length that duplicates the
top-level count—remove the "data": {"length": ...} entry so _emit_list only
returns count, has_more, and invocations via emit_json; and update _emit_single
to render table output using a richer column set instead of LIST_COLUMNS
(introduce or use DETAIL_COLUMNS or a pivoted key/value rendering) so
emit_table([row], DETAIL_COLUMNS) (or equivalent key/value display) shows all
detail fields like job_id, job_run_id, and timing breakdowns while JSON output
remains emit_json({"invocation": row}).

---

192-200: Consider validating user-supplied --started-after/--started-before locally.

When the user passes these flags, the strings are forwarded verbatim to the macro without parsing. A typo like 2024-13-01 will travel all the way to the warehouse and surface as a WAREHOUSE_ERROR, which is a noisy failure mode for a client-side mistake. Parsing with datetime.fromisoformat (or click.DateTime) here would produce a clean USER_ERROR with a clear message and also let you normalize to UTC consistently with the defaults.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/artifacts/entities/invocations.py` around lines 192 - 200, The
_resolve_time_window function currently forwards user-supplied
started_after/started_before strings verbatim; parse each non-None value with
datetime.fromisoformat (or equivalent) inside _resolve_time_window, catch
ValueError and propagate a clear USER_ERROR (or raise a ValueError with a
descriptive message) so typos like "2024-13-01" fail locally, normalize parsed
datetimes to UTC (use astimezone(timezone.utc)) and then return their
.isoformat() strings; keep the same return signature and treat None by
substituting the existing defaults before formatting.

elementary/artifacts/entities/tests.py (1)

209-227: Same redundant data.length field; detail table ignores DETAIL_JSON_FIELDS.

1. Line 218: "data": {"length": len(rows)} duplicates the top-level "count" on line 215 — same observation as in invocations.py. Recommend dropping unless it's part of a documented stable contract.
2. _emit_single renders the single test via LIST_COLUMNS, which intentionally omits the richer fields hydrated by DETAIL_JSON_FIELDS (test_params, meta, depends_on_*, etc.). For edr artifacts test <unique_id> -o table that means the table output is strictly less informative than the JSON output, which is surprising given the detail hydration above it. A dedicated DETAIL_COLUMNS (or a two-column key/value rendering for single-record output) would make the hydration actually observable.

♻️ Suggested cleanup for the redundant field

     emit_json(
         {
             "count": len(rows),
             "has_more": has_more,
             "tests": rows,
-            "data": {"length": len(rows)},
         }
     )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@elementary/artifacts/entities/tests.py` around lines 209 - 227, Remove the
redundant "data": {"length": ...} in _emit_list (it duplicates "count") and stop
emitting it unless it's a documented contract; in _emit_single, change the table
rendering to show the detailed fields instead of LIST_COLUMNS so the hydrated
DETAIL_JSON_FIELDS are visible — either use a new DETAIL_COLUMNS constant (or a
two-column key/value renderer) when calling emit_table([row], ...) in
_emit_single, or branch to a dedicated detail-table helper; keep emit_json
behavior unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes