Skip to content

#183: Add _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION - #191

Merged
wjduenow merged 9 commits into
devfrom
feature/183-row-count-between-scope
Jun 2, 2026
Merged

#183: Add _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION#191
wjduenow merged 9 commits into
devfrom
feature/183-row-count-between-scope

Conversation

@wjduenow

@wjduenow wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Super plan for #183 — add the missing _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION to the drafter, the DEC-012 worked example that never landed with #169.

Base: dev (0.6.0.dev0) — the only branch carrying #169/#170/#171/#179. main (0.5.0) lacks all the deps.
Phase: detailing (awaiting approval)
Stories: 2 implementation (US-001 prompt+lockstep pin, US-002 #170 test-parity backfill) + Quality Gate + Patterns & Memory
Decisions: DEC-001…DEC-007 captured

Key findings

Plan document

See plans/super/183-row-count-between-scope.md.

Next steps

  • Review the plan in this PR
  • Approve in Claude Code to proceed to devolve (beads creation)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Enhanced system guidance for generating row_count_between tests, including bounded aggregation heuristics and parameter calibration recommendations.
    • Added comprehensive guidance for unique_combination tests with composite-key grain examples and anti-vacuity safeguards.
  • Tests

    • Expanded test coverage for scope instruction inclusion/exclusion behavior across test types.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c5ff726a-1460-43c3-b5cb-993a2fbe6a31

📥 Commits

Reviewing files that changed from the base of the PR and between 03828cd and a832b8a.

📒 Files selected for processing (2)
  • docs/draft-ops.md
  • plans/super/183-row-count-between-scope.md
✅ Files skipped from review due to trivial changes (2)
  • docs/draft-ops.md
  • plans/super/183-row-count-between-scope.md

📝 Walkthrough

Walkthrough

This PR implements the missing _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION scope block in the system prompt, wires conditional rendering through _render_system_prompt, adds 10 comprehensive prose tests for row_count_between and unique_combination coverage, updates the prompt-version cache pin, and documents the paired-prose-tests contract for future scope-instruction additions.

Changes

Row-count-between and unique-combination scope instructions

Layer / File(s) Summary
row_count_between scope instruction implementation
src/signalforge/draft/prompts.py
Adds the _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION constant describing bounded-aggregation heuristics and calibration guidance for minimum/maximum/where, extends the _SYSTEM_PROMPT_TEMPLATE to include {row_count_between_scope}, and conditionally computes this block in _render_system_prompt when row_count_between is in the allowed set.
Prose and exclusion tests for row_count_between and unique_combination
tests/draft/test_prompts.py
Updates test_render_system_prompt_excludes_row_count_between_when_in_exclude_tests to validate scope-prose removal; adds 4 row_count_between tests for bounded-aggregation heuristic, maximum/where/vacuous-bound guidance, default inclusion, and inclusion when other types are excluded; adds 5 unique_combination tests for composite-key heuristic, anti-vacuous-tuple warning, default inclusion, exclusion gating, and keep-when-other-excluded behavior (parity backfill for #170).
Prompt cache stability version pin update
tests/llm/test_prompt_cache_stability.py
Updates _EXPECTED_PROMPT_VERSION from c11a73cc95b31614 to e568fb3e4602e465 to pin the new rendered output, and revises docstring rotation history and "latest known rotations" to document the #183 (US-001) change.
Documentation and planning contract
.claude/rules/business-rule-tests.md, docs/draft-ops.md, plans/super/183-row-count-between-scope.md
Establishes paired-prose-tests contract requiring scope-instruction blocks to ship with prose tests and version pins; documents the row_count_between scope instruction in the ops guide; provides complete super-plan breakdown including refinement checks, TDD validation, and Beads Manifest task mapping.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • #183: This PR directly implements the requested _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION addition and related prompt-version/test updates specified in the issue.
  • #170: This PR includes a parity backfill of missing unique_combination scope-instruction prose tests referenced in the issue, now covered alongside row_count_between.

Possibly related PRs

  • wjduenow/SignalForge#83: Introduced the exclude_tests filtering mechanism in _render_system_prompt that this PR extends to conditionally include/exclude the new row_count_between scope block.
  • wjduenow/SignalForge#182: Identified the missing _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION as the root cause of test failures, which this PR implements.

Poem

🐰 A scope block for bounded rows now dances in the prompt,
With minimum and maximum heuristics, no corner left uncouth;
Tests verify the prose flows clear when tests are not exempt,
And unique-combination gains its rightful proof! 🎉

🚥 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 concisely identifies the main change—adding the _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION—which is directly supported by the raw summary showing this as the primary technical contribution across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%.
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.

wjduenow and others added 6 commits June 2, 2026 04:18
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the missing narrative SCOPE-instruction block for the
row_count_between test primitive, mirroring the two existing sibling
blocks (_UNIQUE_COMBINATION_SCOPE_INSTRUCTION,
_ROW_COUNT_ANOMALY_SCOPE_INSTRUCTION). The primitive itself shipped its
catalogue line in #169 but never got a dedicated scope-instruction block.

- prompts.py: define _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION (embeds the
  DEC-012 worked example), insert {row_count_between_scope} placeholder in
  primitive order, wire the allowed/scope toggle through
  _render_system_prompt + the format() call.
- test_prompt_cache_stability.py: rotate _EXPECTED_PROMPT_VERSION
  c11a73cc95b31614 -> e568fb3e4602e465 with a #183 rotation-history entry.
  _CACHED_BLOCK_GOLDEN untouched (system-prompt edit only).
- test_prompts.py: add bounded-aggregation / calibration prose tests,
  default-render-inclusion test, keep-when-other-types-excluded test, and
  extend the row_count_between exclude test to assert the scope prose
  disappears too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…se tests (#183, #170 parity)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…COPE_INSTRUCTION (#183)

4 code-review passes: plumbing + test-quality + fidelity clean. One finding:
docs/draft-ops.md row_count_between section now names the scope-instruction
constant for parity with the sibling sections. CodeRabbit skill unavailable
in this environment (skipped).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion (#183)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow

wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Implementation landed (Ralph run complete)

All four beads under epic SignalForge-jvu completed, in-place on this branch:

Bead Commit Result
US-001 — scope block + plumbing + lockstep _PROMPT_VERSION 9fd3266 _PROMPT_VERSION rotated c11a73cc95b31614e568fb3e4602e465; _CACHED_BLOCK_GOLDEN untouched
US-002 — backfill #170 unique_combination prose tests 38b9eec test-only parity backfill
Quality Gate — review ×4 27be258 4 passes; 1 finding fixed (docs/draft-ops.md scope-instruction parity); CodeRabbit skill unavailable in env (skipped)
Patterns & Memory 03828cd new .claude/rules/business-rule-tests.md convention: every scope-instruction block ships paired prose tests

Validation: ruff ✓ · ruff format ✓ · pyright 0/0/0 · pytest 3228 passed, 8 skipped (97.3% cov).

Maintainer post-merge validation (not CI-gated, per DEC-005): re-run signalforge generate models/reporting/weekly_query_cost.sql --project-dir . --profiles-dir <...> --format markdown against the intuit_airflow substrate (see #179 § "Reproducing this retest") and confirm the drafter now proposes row_count_between. ~$3-7 Anthropic spend.

🤖 Generated with Claude Code

@wjduenow wjduenow changed the title #183: Add _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION (plan) #183: Add _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION Jun 2, 2026
@wjduenow
wjduenow requested a review from Copilot June 2, 2026 15:21
@wjduenow
wjduenow marked this pull request as ready for review June 2, 2026 15:22

Copilot AI 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.

Pull request overview

Adds the missing narrative SCOPE-instruction block for the existing row_count_between test primitive in the drafter prompt, rotates the drafter prompt version pin accordingly, and backfills prose-level prompt tests (including parity coverage for unique_combination) so future prompt edits are behaviorally pinned in CI.

Changes:

  • Add _ROW_COUNT_BETWEEN_SCOPE_INSTRUCTION and wire it into _SYSTEM_PROMPT_TEMPLATE / _render_system_prompt.
  • Rotate the drafter prompt version pin (_EXPECTED_PROMPT_VERSION) and extend prompt tests to assert inclusion/exclusion + key heuristic substrings.
  • Update operational docs/rules and add a detailed plan document for #183.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/signalforge/draft/prompts.py Introduces row_count_between SCOPE prose and gates it via exclude_tests in the rendered system prompt.
tests/llm/test_prompt_cache_stability.py Updates the pinned _EXPECTED_PROMPT_VERSION and records the new rotation in the history docstring.
tests/draft/test_prompts.py Adds prose “content pin” tests for row_count_between and backfills missing unique_combination prose tests + exclude-gating assertions.
docs/draft-ops.md Documents that the system prompt now includes the row_count_between scope-instruction guidance (plus calibration notes).
.claude/rules/business-rule-tests.md Adds a rule requiring scope-instruction blocks to ship with paired prose tests.
plans/super/183-row-count-between-scope.md Adds the super-plan for #183 implementation/validation steps and conventions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/draft-ops.md Outdated
@wjduenow

wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow

wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

PR Review Summary

Fixed (1 item)

File Line Issue Commit
docs/draft-ops.md 472 Copilot: dangling "The" from a hard line break introduced by the scope-instruction doc edit (latest)

False Positives (0 items)

None.

All review threads resolved.

@wjduenow
wjduenow merged commit b07f1b2 into dev Jun 2, 2026
6 checks passed
@wjduenow
wjduenow deleted the feature/183-row-count-between-scope branch June 2, 2026 15:34
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.

2 participants