review gate: don't block a PR for hits already on main

[asubbarao]

The rules gate runs every check on the whole changed file, so a check that already fires on main blocks any PR that touches the file. #65 is held by sql_new_table_in_file for fs.dirs and fs.filelist, tables that exist on main and were approved.

Now the queue also reads each modified file as it is on main (pr_base_files), the gate runs the checks on both, and a hit main's copy already has is reported as existing instead of its action. Only block fails the status, so existing hits no longer block; new ones still do.

Tested by running gate.sql locally:

• CI: setup.sql boots on an empty home #65's fs.sql, host.sql, lake.sql (PR head vs main): all 7 hits come back existing, none block.
• A new file with LIMIT 1 beside an old file that already had it: the new file is block, the old one existing.

Limit, noted in the code: matched per file and check, so a second hit of a check the file already has passes the gate; the Opus review still sees the added lines.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NCnrCZ6kWmagfJjoK8jVGQ

[asubbarao]

duckstack/rules: success

• warn sql_file_too_long in skills/duckstack-review/queue.sql: Keep a job or loader file to 30-80 lines and one concern; split it, or state the reason and line count in the PR.
• warn sql_dispatch_shape in skills/duckstack-review/queue.sql: Self-dispatch has one shape: source rows, one generated statement per row, array_agg(selfdispatch(sql)), then CROSS JOIN UNNEST of the receipts.

duckstack review queue, rule gate (hooks/hook_checks.sql)

[asubbarao]

duckstack/review: failure

• skills/duckstack-review/queue.sql:68 1 / RULES 3 (lateral table function with column parameters): The new view passes row columns into git_read_each through CROSS JOIN LATERAL. That is the lateral table-function shape check 1 flags, and here it is repeated a second time next to pr_files.
  Rewrite: Read head and base blobs in one relation: pr_blobs AS (SELECT pr_changes.*, side, blob.text AS content FROM pr_changes CROSS JOIN UNNEST([{side:'head', sha: head_sha}, {side:'base', sha: base_sha}]) AS v(s) CROSS JOIN LATERAL git_read_each(git_uri(repo, file_path, s.sha)) AS blob WHERE blob.is_text AND (s.side, status) NOT IN ...). Then define pr_files and pr_base_files as WHERE side = ... over it. If correlated git_read_each is not accepted, self-dispatch one literal git_read_each per (file, sha) row.
• skills/duckstack-review/queue.sql:66 5 / 9 (shared relation, bloat): pr_base_files adds a second read of every modified file at base_sha, but pr_added_lines.old_lines still runs its own identical git_read_each(git_uri(..., base_sha)) with the same status = 'modified' AND blob.is_text filter. The same blobs are now read twice per tick and the new relation is not shared.
  Rewrite: Make old_lines read the new view: old_lines AS (SELECT pr_number, file_path, old.line, count(old.line) AS copies FROM pr_base_files CROSS JOIN UNNEST(string_split(content, E'\n')) AS old(line) GROUP BY ALL). That removes the second git_read_each and goes from 6 lines to 4.

Opus 5.5, duckstack review queue (duckstack-review skill)

[asubbarao]

duckstack/rules: success

• warn sql_dispatch_shape in skills/duckstack-review/queue.sql: Self-dispatch has one shape: source rows, one generated statement per row, array_agg(selfdispatch(sql)), then CROSS JOIN UNNEST of the receipts.
• warn sql_file_too_long in skills/duckstack-review/queue.sql: Keep a job or loader file to 30-80 lines and one concern; split it, or state the reason and line count in the PR.

duckstack review queue, rule gate (hooks/hook_checks.sql)

[asubbarao]

duckstack/review: failure

• skills/duckstack-review/gate.sql:43 Check 9 (bloat) / shorter version; RULES.md 9: The base-side match is written as a four-condition ON clause that repeats each qualified key. A relation filtered to side 'base' and joined USING the shared keys says the same thing in fewer lines.
  Rewrite: WITH file_hits AS (
  SELECT DISTINCT tc_src.side, tc_src.pr_number, tc_src.file_path, tc_src.bad_example, check_hits.check_id
  FROM check_hits JOIN tc_src USING (call_id)
  ), base_hits AS (
  SELECT pr_number, file_path, check_id, true AS on_main FROM file_hits WHERE side = 'base'
  )
  SELECT pr_number, file_path, check_id,
  CASE WHEN bad_example THEN 'example' WHEN on_main THEN 'existing' ELSE hook_checks.action END AS action,
  hook_checks.message
  FROM file_hits JOIN hook_checks USING (check_id)
  LEFT JOIN base_hits USING (pr_number, file_path, check_id)
  WHERE side = 'head' AND hook_checks.action IN ('block', 'warn');

Opus 5.5, duckstack review queue (duckstack-review skill)

[asubbarao]

duckstack/rules: success

• warn sql_dispatch_shape in skills/duckstack-review/queue.sql: Self-dispatch has one shape: source rows, one generated statement per row, array_agg(selfdispatch(sql)), then CROSS JOIN UNNEST of the receipts.
• warn sql_file_too_long in skills/duckstack-review/queue.sql: Keep a job or loader file to 30-80 lines and one concern; split it, or state the reason and line count in the PR.

duckstack review queue, rule gate (hooks/hook_checks.sql)

[asubbarao]

duckstack/review: failure

• skills/duckstack-review/gate.sql:39 check 4 / RULES.md 9 (short): The base_hits CTE exists only to add a constant 'true AS on_main' flag for a LEFT JOIN, which is a roundabout way to test whether main already has the hit.
  Rewrite: WITH file_hits AS (...) SELECT head.pr_number, head.file_path, head.check_id, CASE WHEN head.bad_example THEN 'example' WHEN base.check_id IS NOT NULL THEN 'existing' ELSE hook_checks.action END AS action, hook_checks.message FROM file_hits AS head JOIN hook_checks USING (check_id) LEFT JOIN file_hits AS base ON base.side = 'base' AND (base.pr_number, base.file_path, base.check_id) = (head.pr_number, head.file_path, head.check_id) WHERE head.side = 'head' AND hook_checks.action IN ('block', 'warn'); -- 12 lines to 9; the base_hits CTE is gone
• skills/duckstack-review/queue.sql:67 RULES.md 8 (code as data) / check 4: The WHERE clause uses a CASE to decide which versions apply, but that rule belongs with each version struct being unnested.
  Rewrite: CROSS JOIN UNNEST([{'side': 'head', 'sha': pr_changes.head_sha, 'wanted': pr_changes.status <> 'deleted'}, {'side': 'base', 'sha': pr_changes.base_sha, 'wanted': pr_changes.status = 'modified'}]) AS v(version) CROSS JOIN LATERAL git_read_each(git_uri(pr_changes.repo, pr_changes.file_path, version.sha)) AS blob WHERE version.wanted AND blob.is_text; -- same 3 lines; each version now states its own condition, and versions that don't apply are dropped before any read

Opus 5.5, duckstack review queue (duckstack-review skill)

[asubbarao]

duckstack/rules: success

• warn sql_file_too_long in skills/duckstack-review/queue.sql: Keep a job or loader file to 30-80 lines and one concern; split it, or state the reason and line count in the PR.
• warn sql_dispatch_shape in skills/duckstack-review/queue.sql: Self-dispatch has one shape: source rows, one generated statement per row, array_agg(selfdispatch(sql)), then CROSS JOIN UNNEST of the receipts.

duckstack review queue, rule gate (hooks/hook_checks.sql)

[asubbarao]

duckstack/review: success

No findings.

Opus 5.5, duckstack review queue (duckstack-review skill)