Skip to content

#179: empirical retest writeup against intuit_airflow - #182

Merged
wjduenow merged 1 commit into
devfrom
docs/179-retest-writeup
Jun 2, 2026
Merged

#179: empirical retest writeup against intuit_airflow#182
wjduenow merged 1 commit into
devfrom
docs/179-retest-writeup

Conversation

@wjduenow

@wjduenow wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Close-out artefact for epic #179 (Test Generation Expansion).

What this PR adds

docs/research/179-test-primitive-expansion-retest.md (404 lines) — the
empirical retest of #169 / #170 / #171 against the intuit_airflow
substrate originally stood up for the 2026-05-30 baseline measurement.

Results

Phase A (binary, weekly_query_cost.sql):

Phase B (aggregate, 15 candidates):

Primitive Measured Projected Delta
row_count_between (#169) 57.1% (8/14) 76% -18.9 pp ❌
unique_combination (#170) 100.0% (5/5) 86% +14.0 pp ✅

Findings to be filed as separate follow-on issues

  1. _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION + DEC-012 worked example (Phase A FAIL root cause).
  2. Drafter mis-scopes row_count_anomaly_by_period to date columns on every model with audit timestamps (Add row_count_anomaly_by_period: drafter-proposed historical-trend row-count guard #171 calibration bug; reproduced 3/3 in Phase B v1/v2 before workaround).
  3. AuditRecordTooLargeError blocks generate on wide-table models (datashare_googlead, 170 cols → 4,519-byte record vs. 4,000-byte cap from safety-layer.md DEC-011).

Phase C (#171 calibration retest)

Deferred — requires a maintainer-controlled Snowflake fixture warehouse per the epic's option 3.

Cost + wall

~17 drafter + 4,044 grade calls; ~$3–7 LLM spend; ~95 min wall-clock.

Summary by CodeRabbit

  • Documentation
    • Added research documentation detailing testing workflows, validation results, and process improvements.

Phase A binary (weekly_query_cost.sql) + Phase B aggregate (15 candidates)
against the substrate built for the 2026-05-30 baseline. #170 ships well-
calibrated (100% match-rate, beats projection); #169 misses the projection
by 18.9 pp on a 14-model sample with a named root cause (missing
_ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION in prompts.py per DEC-012); #171 has a
drafter-calibration bug visible on every model with audit-timestamp columns.
One additional ops-class finding: 170-col model blows the safety-layer
audit-record size cap.

Three follow-ons named for separate filing; Phase C (#171 calibration retest)
deferred per epic option 3 — needs maintainer-controlled Snowflake fixture
warehouse.
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

A comprehensive research write-up documenting Issue #179's empirical retest campaign, reporting Phase A binary outcomes for unique_combination and row_count_between, Phase B aggregate coverage metrics across fifteen models, identified root causes (prompt-scope instruction gap, calibration mis-scoping with audit timestamps), an operational safety-layer finding, estimated costs, and follow-on work.

Changes

Issue #179 Primitive Expansion Retest

Layer / File(s) Summary
Retest overview and Phase A results
docs/research/179-test-primitive-expansion-retest.md
Executive summary records Issue #179 status, anchor commits, and SignalForge environment; experimental substrate section defines repository/dbt setup and manifest synthesis for Phase B coverage; Phase A section documents binary pass/fail outcomes for unique_combination and row_count_between with prompt-cache evidence and named prompt-scope/shape mismatch root cause.
Phase B execution and candidate outcomes
docs/research/179-test-primitive-expansion-retest.md
Phase B section details candidate selection criteria, documents row_count_anomaly_by_period mis-scoping failure mode revealed when audit timestamp columns are present, records Phase B v3 llm.exclude_tests workaround, and reports per-model and aggregate matched/missed/failed coverage metrics for both primitives.
Phase B analysis and operational findings
docs/research/179-test-primitive-expansion-retest.md
Compares Phase B measured coverage against epic projections and enumerates named miss cluster hypotheses for row_count_between (composite-key uniqueness interaction, missing scope instruction); records separate ops-class finding of AuditRecordTooLargeError on a 170-column model due to audit record size cap.
Cost metrics, follow-on tickets, and reproduction
docs/research/179-test-primitive-expansion-retest.md
Reports cost and wall-clock timing with call/token counts; lists actionable follow-on tickets (prompt-scope instruction, mis-scoping fix options, safety-layer cap mitigation, deferred Snowflake fixture, optional manifest-synthesis helper); provides reproduction steps and example commands; includes references to issue, child stories, worked examples, and preserved baseline sidecars.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A research tale, carefully penned,
Where primitives dance and test cycles bend,
Phase A found quirks, Phase B revealed more,
Each finding a brick for the knowledge floor!
SignalForge gleams with the truth now laid bare, 🔍✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: a research writeup documenting an empirical retest of test primitives against the intuit_airflow codebase for issue #179, matching the added documentation file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/research/179-test-primitive-expansion-retest.md`:
- Around line 123-142: The fenced code blocks that begin with "5
unique_combination (+ row_count_between):", "ERROR: LLM response violated the
anchor contract...", and "PER-MODEL BREAKDOWN" are missing language identifiers
(MD040); update each opening triple-backtick to include a language tag like text
(e.g. change ``` to ```text) so markdownlint stops flagging them, and apply the
same fix to the other occurrences referenced (the similar blocks around the
other ranges) by locating the blocks by their starting lines shown above and
adding the language identifier to the opening fence for each.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 949d57dc-7081-4413-ab4b-c03e37ea5550

📥 Commits

Reviewing files that changed from the base of the PR and between 7cc4e80 and 4c19fc7.

📒 Files selected for processing (1)
  • docs/research/179-test-primitive-expansion-retest.md

Comment on lines +123 to +142
```
5 unique_combination (+ row_count_between):
raw/taxday_auction_insights (16 cols, 9 spaced/quoted)
analytical/tvp_yelp (58 cols)
analytical/core_hourly_performance (16 cols, 8 spaced/quoted)
raw/taxday_mappings ( 7 cols)
raw/googleads_auction_insights (15 cols, 9 spaced/quoted)

10 row_count_between only:
reporting/data_store_test_control (61 cols, 11 spaced/quoted)
operational/map_cid_sa360 (16 cols)
analytical/fiscal_season (12 cols)
raw/query_history (21 cols)
raw/concord_sku (12 cols)
raw/aio (14 cols)
analytical/calendar_hour ( 8 cols)
ingress/yelp_business_metrics_stg (35 cols)
reporting/cid_all_concord_sku (43 cols, 22 spaced/quoted)
raw/datashare_googlead (170 cols)
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add explicit language identifiers to fenced code blocks.

These three fenced blocks are missing language tags (MD040), which will keep markdown lint noisy. Please annotate them (for example, text for lists/logs).

Suggested patch
-```
+```text
 5 unique_combination (+ row_count_between):
   raw/taxday_auction_insights        (16 cols, 9 spaced/quoted)
   analytical/tvp_yelp                (58 cols)
   analytical/core_hourly_performance (16 cols, 8 spaced/quoted)
   raw/taxday_mappings                ( 7 cols)
   raw/googleads_auction_insights     (15 cols, 9 spaced/quoted)
@@
   reporting/cid_all_concord_sku      (43 cols, 22 spaced/quoted)
   raw/datashare_googlead             (170 cols)

- +text
ERROR: LLM response violated the anchor contract (4 violation(s)).

  • column test on column='creation_ts' references None
  • test references nonexistent column None (available: [...])
  • column test on column='update_ts' references None
  • test references nonexistent column None (available: [...])

-```
+```text
PER-MODEL BREAKDOWN
taxday_auction_insights     row_count_between    shape-missed
taxday_auction_insights     unique_combination   shape-matched   column-set exact match
tvp_yelp                    row_count_between    shape-matched
@@
AGGREGATE
row_count_between           matched= 8  missed= 6  failed= 1  evaluable= 14/15  coverage=  57.1%
unique_combination          matched= 5  missed= 0  failed= 0  evaluable=  5/5   coverage= 100.0%
</details>

 


Also applies to: 150-156, 193-219

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 123-123: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/research/179-test-primitive-expansion-retest.md around lines 123 - 142,
The fenced code blocks that begin with "5 unique_combination (+
row_count_between):", "ERROR: LLM response violated the anchor contract...", and
"PER-MODEL BREAKDOWN" are missing language identifiers (MD040); update each
opening triple-backtick to include a language tag like text (e.g. change ``` to

other occurrences referenced (the similar blocks around the other ranges) by
locating the blocks by their starting lines shown above and adding the language
identifier to the opening fence for each.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant