malloydata/publisher/malloy-review

Malloy Code Review

Single-pass code reviewer for .malloy files. The deliverable is one Markdown file in the canonical shape (see reference/output-template.md). Findings cite the project's standards file (e.g., CLAUDE.md) where it applies; otherwise they cite rubric IDs (reference/rubric-*.md).

Tool names are written bare here - get_context, execute_query, search_malloy_docs. The exact prefixed name depends on the host surface; match each against the tools you actually have.

Before you start

Read these in order:

1. Project standards: whatever conventions exist in your host environment (CLAUDE.md, AGENTS.md, etc.). Treat as the higher-priority source of truth; rubric rules defer to it where they overlap.
2. `reference/severity-taxonomy.md`: the vocabulary every finding uses.
3. `reference/rubric-*.md`: the seven dimension rubrics. Don't re-read for every review; load them on demand based on what's in scope.
4. `reference/output-template.md`: the shape of the review file you produce.
5. `.malloy-review.local.md` in the scope folder, if present, project-specific severity overrides or extra rules.

Make sure the Malloy MCP tools are configured before running: this skill uses execute_query for data checks and search_malloy_docs for verifying Malloy capabilities. Both are optional; the review degrades gracefully if either is unavailable.

Inputs

/malloy-review [<path>] [--pr <n>] [--out <file>] [--comment]

If scope is ambiguous (multiple packages, wrong file type, empty result), stop and ask: don't guess. See reference/scope-resolution.md for the rules.

Workflow

resolve scope → read files → verify PKs → apply rubrics → write output

1. Resolve scope

Per reference/scope-resolution.md. Echo the resolved scope to the user before doing anything expensive. Multi-package repos → present packages as A/B/C and let the user pick. Never auto-fan-out across packages: different packages may target different database connections.

2. Read files

For each in-scope .malloy file, read the full content. As you read, track each source's primary_key: and its join_one/join_many/join_cross targets, you'll use this for the PK uniqueness check (C-12) and join-style consistency (Y-03).

If mcp__ide__getDiagnostics is available, call it on the in-scope files and promote each diagnostic to a finding: errors → blocker (confidence 95), warnings → major (confidence 85), info/hint → minor (confidence 75), all with source: "diagnostic". Skip rubric rules these already cover. If unavailable, skip, the LLM rubric pass still runs.

3. PK data verification (if execute_query is available)

For every source with a declared primary_key:, run a uniqueness check and store the result on source_index.<src>.pk_verified:

run: <source> -> {
  aggregate:
    rows is count()
    distinct_pk is count(<pk_col>)
}

This step only collects evidence, the emit decision and fix template live in reference/rubric-correctness.md § C-12. When any source is "skipped" or "error", note the coverage gap in the output's Scope section.

4. Apply rubrics

Score the in-scope files against each applicable rubric. You don't need to read all seven: pick by content:

Findings use the canonical shape from reference/severity-taxonomy.md:

{
  "id": "C1",
  "severity": "critical",
  "category": "correctness-join",
  "file": "packages/x/customers.malloy",
  "line_range": [12, 12],
  "rule": "C-12 declared primary_key is not unique in the data",
  "current": "primary_key: customer_id (customer_id has duplicates per execute_query check)",
  "expected": "customer_id is unique per row, or the source carries a where: that scopes to a uniquely-keyed subset",
  "suggested_fix": "...",
  "confidence": 95,
  "source": "rule"
}

Drop findings with confidence < 80. The output should feel curated, not exhaustive.

5. Look for cross-cutting themes

After per-file findings, scan for systemic patterns. This is the highest-value output. Examples that emerged from real reviews:

• N base sources all missing include {} curation
• M files use raw conn.sql() where Malloy-native patterns exist
• Canonical naming violations across N files (total_revenue vs net_revenue, etc.)
• "Junk-drawer" files with multiple unrelated sources

Promote these to a Cross-Cutting Themes section in the output. They are usually more actionable than per-line findings, collapse 3+ findings of the same rule into one theme with the file list, and emit individual findings only for the top 2–3 worst offenders.

6. Assemble output

Apply reference/output-template.md. Section order is fixed (skip if empty):

1. Header (scope, file count, LOC, timestamp)
2. Scope (paths reviewed)
3. Executive Summary (recommendation + top 1–3 risks)
4. Coverage & Risk Map (file table; skip if scope is one file)
5. Cross-Cutting Themes
6. Top Issues (blockers + criticals)
7. Detailed Findings (collapsed <details> per file)
8. Questions for the Author (≤5 bullets)
9. Positive Notes (only if real)
10. Suggested Follow-ups (aggregated minor/nit findings)
11. Suggested Split (only when scope is >2000 LOC or >30 files)

7. Write the file

Default ./malloy-review-<YYYYMMDD-HHMMSS>.md. Tell the user the path and the top 1–3 issues inline. End with an offer to start fixing, most reviews are the start of iterative work, not a static report.

Scaling notes

Mode notes

Single-file mode (user passes one .malloy file): trim the template hard. Drop the Coverage Map, Cross-Cutting Themes, and Suggested Split sections. Lean conversational; write the file AND print the Summary + Findings inline so the user doesn't need to open the file for a small review. End with a fix offer ("Want me to fix B1?").

Audit mode (user passes a directory or invokes inside a publisher.json package): every file is in play. Coverage Map matters more, the user needs to know what was deep-reviewed vs. skimmed. For unfamiliar packages, consider running just the source-level summary first, presenting it, and asking whether to proceed with full review.

PR mode (--pr <n>):

1. gh pr view <n> --json state,isDraft,title,baseRefName,headRefName,url,author and gh pr diff <n> --name-only. Stop if the PR is closed or has no .malloy changes.
2. Filter changed files to .malloy and intersect with any path argument, that's the scope.
3. Fetch full file contents on the PR's head (via gh pr checkout <n> if the working tree is clean, otherwise via gh api repos/<owner>/<repo>/contents/<path>?ref=<headRef>). Don't clobber working state without asking.
4. Run the workflow above. PR header: # Malloy Model Review, PR #<n>: <title> with branch/package/LOC/url metadata.
5. With --comment, post via gh pr comment <n> -F <file> (cap at GitHub's 65,536-char limit; if larger, post the exec summary + top issues and reference the local file). Lead the comment body with <!-- malloy-review: <timestamp> --> so re-runs can detect prior reviews. Never post without `--comment`.

What this skill does NOT do

• Does not modify any `.malloy` files. Output is the Markdown review only.
• Does not post PR comments without `--comment`.
• Does not walk above the resolved scope. Even if a finding would benefit from cross-scope context, scope is fixed once resolved.
• Does not guess at multi-package disambiguation. Always asks.
• Does not flag modeling features as hazards. Source-level where: clauses and deliberate private: choices are part of how Malloy models work. If something looks unusual, surface it as a "Question for the Author," not a finding.