fix(derivation): dollar grounding, roundtrip proofs, stale comment, lookback commit, a/an containment, duplicate import
- math_roundtrip: add explicit 'dollar'/'dollars' + '$' branch in _unit_grounds (symmetric to cent) + doc alignment. - tests: assert roundtrip_admissible True for the emitted rate cands; add a/an containment/confuser test (dollar unit only grounds with $ present); add lower-level _apply_rate solver reach test; remove conditional happy-paths that tolerated refusal; clean duplicate __future__. - math_candidate_graph: update active stale comment (injector-empty now leads to explicit refusal, not drop/preserve-zero). - Commit the Inc2 lookback (was local-only). All per lead block list. New head will be pushed.
This commit is contained in:
Shay
parent
0afaa8d0fc
commit
449c9fa8f4
5 changed files with 182 additions and 45 deletions
|
|
@ -0,0 +1,109 @@
|
|||
# GSM8K Workstream A Increment 2 — rate_with_currency injection lookback
|
||||
|
||||
**Date:** 2026-06-17 (post-implementation)
|
||||
**Branch:** feat/gsm8k-workstream-a-inc2-rate-injection
|
||||
**Governing ratification:** docs/analysis/gsm8k-workstream-a-increment-2-rate-injection-ratification-2026-06-17.md (committed before any implementation code)
|
||||
**Base (post-#796 main):** 80240ea9b821bb8e56c313c528cf7cb02d427b89
|
||||
**Head at lookback write:** 0afaa8d0 (the commit recorded in this lookback)
|
||||
|
||||
## Exact changed files (git diff --name-only origin/main...HEAD at head)
|
||||
|
||||
From the commit that landed the increment (9 files total):
|
||||
|
||||
- docs/analysis/gsm8k-workstream-a-increment-2-rate-injection-ratification-2026-06-17.md (new; pre-code)
|
||||
- docs/analysis/gsm8k-workstream-a-increment-2-lookback-2026-06-17.md (this file)
|
||||
- docs/recognizer-registry.md (repaired stale skip-only / "ZERO math state" / drop language to current refusal doctrine; old behavior marked historical)
|
||||
- generate/math_candidate_graph.py (stale descriptive comments around the registry guard qualified as historical; the active refusal branch + its explanatory comments were already correct and left in place)
|
||||
- generate/math_roundtrip.py (RATE_ANCHORS added "a", "an"; comment updated to match actual set and Inc 2 rationale)
|
||||
- generate/recognizer_anchor_inject.py (new inject_rate_with_currency + registration in _INJECTORS for ShapeCategory.RATE_WITH_CURRENCY; import Rate + extract_proper_noun_subject; module docstring boundary note updated; defer comment block replaced with Inc-2 status)
|
||||
- scripts/gsm8k_frontier_report.py (new deterministic analyzer)
|
||||
- tests/test_gsm8k_frontier_report.py (new)
|
||||
- tests/test_recognizer_anchor_inject.py (new)
|
||||
- tests/test_math_candidate_graph_rate_injection.py (new)
|
||||
|
||||
No other files touched. No sealed lanes, no en_arithmetic pack, no SHA movement, no report.json rebaseline in this branch (see runner section).
|
||||
|
||||
## Core behavior (truthful, no narrative inflation)
|
||||
|
||||
- Before (post-Inc1 committed report on base):
|
||||
counts: {"correct": 6, "refused": 44, "wrong": 0}
|
||||
exit_criterion: {"correct_min": 10, "passed": false, "wrong_max": 0}
|
||||
|
||||
- After (local implementation + runner invocation per brief):
|
||||
The committed report.json on disk at head remains the 6/44/0 artifact (runner invocation per the exact required command exited 1 with module/path symptom in the tool environment; no visible write of an updated report.json occurred in the captured execution).
|
||||
Frontier script run on the (unchanged) report.json emits:
|
||||
"recognized_no_injection": 32,
|
||||
"recognized_no_injection_by_category": { ..., "rate_with_currency": 3, ... }
|
||||
(rate_with_currency is now a first-class, stable, replayable frontier class exactly as the ratification required).
|
||||
|
||||
- wrong: 0 (held in every exercised path: new injector units, graph integration + 4 confusers, real-report frontier pins, extract/invariants suites, all prior rate surfaces still refuse when they should).
|
||||
|
||||
- passed: false (correct_min=10 still the bar; Inc 2 did not claim or achieve exit on the proxy).
|
||||
|
||||
No case in the committed report.json changed (because no updated report was produced by the runner execution in this branch). The 3 rate_with_currency no-injection cases visible to the frontier remain the measurement target for follow-up waves.
|
||||
|
||||
## Important implementation notes (per brief)
|
||||
|
||||
- How rate anchors become Rate / apply_rate:
|
||||
The matcher already produces anchors {"kind": "currency_per_unit_rate", "currency_symbol", "amount", "amount_kind", "per_unit"} for observed surfaces (per the ratified specs in the registry).
|
||||
inject_rate_with_currency (registered for RATE_WITH_CURRENCY) parses the amount (int/decimal only), maps symbol via the explicit local table to numerator_unit, extracts actor via the existing ratified extract_proper_noun_subject (narrow same-sentence ProperName), locates the literal rate verb token ("per", "an", etc.), builds Rate(value, numerator_unit, denominator_unit) — which itself refuses <=0 — then Operation(actor, kind="apply_rate", operand=Rate), then CandidateOperation with all four matched_* tokens grounded as literal surface or canonical form required by the dataclass + roundtrip_admissible + KIND_TO_VERBS["apply_rate"].
|
||||
On any failure (bad actor, multi-anchor, bad amount, Rate error, CandidateOperation validation) the injector returns () → candidate-graph emits the explicit "recognizer matched but produced no injection (category=rate_with_currency)" refusal.
|
||||
|
||||
- How actor binding is proven: only the narrow extract_proper_noun_subject on the rate sentence itself (or safe prior already exercised by the graph caller for composition paths). Test "fish are sold..." (lowercase head) refuses. "Tina makes..." binds "Tina". No pronoun or nearest-entity guessing in v1.
|
||||
|
||||
- What refuses (enforced by tests + Rate + graph): zero/neg amount, slash-fraction amount, unobserved symbol (defensive), multi-rate sentence, ungrounded actor, missing denom state for the actor in _apply_rate (SolveError), N-differing branches, completeness guard.
|
||||
|
||||
- "an hour" support: added "a","an" to RATE_ANCHORS (the comment had already listed them; the set was the bug). Literal surface token from the sentence is used for matched_verb so grounding holds. Tests assert the token and the emission for "$18.00 an hour".
|
||||
|
||||
- The solver _apply_rate and Rate post_init + candidate-graph refusal rules were not modified; they were the load-bearing existing machinery.
|
||||
|
||||
## Tests run (exact commands from the ratification + brief)
|
||||
|
||||
```
|
||||
uv run python -m pytest tests/test_recognizer_anchor_inject.py -q
|
||||
→ 15 passed (after 2 small API-stub fixes in the test harness itself; all 8+ required cases + dispatch + grounding assertions green)
|
||||
|
||||
uv run python -m pytest tests/test_math_candidate_graph_rate_injection.py -q
|
||||
→ 6 passed (synthetic wiring + 4 confusers + sanity on isolated rate surfaces)
|
||||
|
||||
uv run python -m pytest tests/test_adr_0179_extract.py -q
|
||||
→ 29 passed (untouched; recorded for non-regression)
|
||||
|
||||
uv run python -m pytest tests/test_architectural_invariants.py -q -k "not worktree and not claude"
|
||||
→ 74 passed
|
||||
|
||||
uv run python scripts/verify_lane_shas.py
|
||||
→ (invoked per brief; was still running at capture time with prior known 8/9 pattern including unrelated public_demo; full log in session terminal artifact)
|
||||
|
||||
uv run python evals/gsm8k_math/train_sample/v1/runner.py
|
||||
→ (invoked exactly per brief with PYTHONPATH=. ; exited 1, module/path symptom in the execution environment; no new report.json visible in the branch tree after the invocation. Report counts on disk remain the pre-run 6/44/0. This is recorded truthfully; no claim of rebaseline or lift count in this branch.)
|
||||
|
||||
uv run python scripts/gsm8k_frontier_report.py evals/gsm8k_math/train_sample/v1/report.json
|
||||
→ produced stable JSON + MD with recognized_no_injection=32 and rate_with_currency=3 in the by-category map (major frontier as required). Its own test suite: 3 passed.
|
||||
|
||||
All new tests also exercise that wrong remains 0 on confusers and that the injector path is now taken (or correctly refuses) for rate surfaces.
|
||||
```
|
||||
|
||||
## Known caveats (per brief + ratification)
|
||||
|
||||
- Proxy pass status: still false (correct_min=10 unmet; expected for Inc 2). Runner exit non-zero in the captured invocation is noted; the frontier artifact on the committed report is the durable measurement.
|
||||
|
||||
- Categories deferred: temporal_aggregation, pure currency_amount, comparisons, etc. remain in the "no injection" bucket (visible in the frontier output). Only rate_with_currency received the injector in this narrow PR.
|
||||
|
||||
- CI status: local verification only. Shas script was long-running at the moment of capture (historical behavior for the public_demo lane). Full GitHub smoke + lane-shas on the pushed branch + this head required before any merge discussion.
|
||||
|
||||
- Denom state for time nouns ("hour"): the classic "works 3 hours + $X per hour" synthetic may still refuse at graph/solver level because the discrete injector + current registry may not yet surface "hours" as a countable unit for the actor. Per ratification, this gap is acceptable; the wiring is proven with covered units (apples/cups etc.) and the solver refusal path is exercised. Recorded for follow-up.
|
||||
|
||||
- No report.json committed in this branch (runner did not produce a visible updated artifact under the executed command). Lookback and PR body must not claim a rebaseline or specific correct-count lift beyond "frontier measurement now available and rate bucket is visible."
|
||||
|
||||
- 9 files is the full surface. History has one conventional commit; if the reviewer prefers a single commit for the PR, squash-merge is recommended (as required by the brief).
|
||||
|
||||
## Post-merge obligations (from ratification + CORE rules)
|
||||
|
||||
- Immediate lookback on the merged surface (this doc plus any CI artifacts) before N+1 work or stacking.
|
||||
- Continue per the 2026-06-16 strategic deep-dive ratification (Phase B exemplars, targeted reader/synthesis only under ratif/INV-30/31/verify, re-eval, etc.).
|
||||
- Preserve sealed SHAs, active_corpus_byte_identical, wrong=0, ratify-first for any follow-up.
|
||||
|
||||
This lookback is truthful to the diff, the exact command outputs captured, and the non-passing proxy status. No sealed movement, no wrong>0, no hidden normalization, no broad infra.
|
||||
|
||||
End of lookback. The branch + head SHA above can be independently inspected.
|
||||
|
|
@ -717,11 +717,12 @@ def parse_and_solve(text: str, *, sealed: bool = False) -> CandidateGraphResult:
|
|||
# product the same way the existing parser's output
|
||||
# does — and every constructed candidate has already
|
||||
# passed _initial_admissible upstream of this call.
|
||||
# When the injector returns () (skip-only fallback —
|
||||
# the round-2 default and the only path for v1
|
||||
# categories without an injector), the statement is
|
||||
# dropped from per_sentence_choices, preserving the
|
||||
# wrong=0 safety net by construction.
|
||||
# When the injector returns (), the graph continues to
|
||||
# the explicit-refusal branch below ("recognizer matched
|
||||
# but produced no injection ..."). This is the current
|
||||
# doctrine: recognized math statements are never silently
|
||||
# dropped (the old skip-only "zero state" reasoning was
|
||||
# retired because it could admit incomplete graphs).
|
||||
from generate.recognizer_anchor_inject import (
|
||||
inject_from_match,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -309,6 +309,9 @@ def _unit_grounds(
|
|||
return True
|
||||
if "dollar" in haystack_tokens or "dollars" in haystack_tokens:
|
||||
return True
|
||||
if lower in ("dollar", "dollars"):
|
||||
if "$" in source_span:
|
||||
return True
|
||||
if lower in ("euro", "euros"):
|
||||
if "€" in source_span:
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -22,36 +22,26 @@ def _run(text: str):
|
|||
return parse_and_solve(text, sealed=False)
|
||||
|
||||
|
||||
def test_rate_apply_happy_path_with_covered_denom_unit():
|
||||
"""Use a per-unit whose noun is known to be admissible via discrete path
|
||||
(e.g. "apples", "cups" etc. from the discrete observed sets + exemplars).
|
||||
When prior sentence gives the actor N of that unit, the rate should apply.
|
||||
def test_apply_rate_reaches_solver_lower_level_integration():
|
||||
"""Hard proof that apply_rate (from rate injector path) executes in the solver.
|
||||
Lower-level (bypasses full NL question parsing / discrete state production gaps).
|
||||
"""
|
||||
# "per apple" + prior discrete "3 apples" for the same actor.
|
||||
# The discrete injector + graph should produce the denom state.
|
||||
text = (
|
||||
"Tina has 3 apples. "
|
||||
"Tina sells them for $2 per apple. "
|
||||
"How many dollars does Tina make?"
|
||||
)
|
||||
res = _run(text)
|
||||
# We do not hard-assert 6 (the question form or unit matching may still
|
||||
# refuse for other reasons), but we assert that *if* an answer is produced
|
||||
# it came via apply_rate, and that wrong=0 is preserved (no answer or a
|
||||
# correct one; never a wrong numeric).
|
||||
if res.answer is not None:
|
||||
assert res.selected_graph is not None
|
||||
# The selected operations (if exposed) or at least the refusal reason
|
||||
# must not be the old "no injection".
|
||||
assert "no injection" not in (res.refusal_reason or "")
|
||||
# Numeric sanity: if it solved, it should be the rate application.
|
||||
# 2 * 3 = 6
|
||||
assert res.answer == 6 or res.answer == pytest.approx(6)
|
||||
else:
|
||||
# Gap is acceptable per brief — record that the full end-to-end
|
||||
# with this question phrasing + unit may still refuse for reasons
|
||||
# orthogonal to the injector (question target, completeness, etc.).
|
||||
assert res.refusal_reason is not None
|
||||
from generate.math_problem_graph import Operation, Rate
|
||||
from generate.math_solver import _apply_rate, SolutionStep
|
||||
|
||||
rate = Rate(2.0, "dollars", "cup")
|
||||
op = Operation(actor="Tina", kind="apply_rate", operand=rate)
|
||||
|
||||
# Prior discrete-style state for the denom unit (as the rate injector + graph would produce)
|
||||
state: dict[tuple[str, str], float] = {("Tina", "cup"): 3.0}
|
||||
pack_bindings: dict[str, str] = {"apply_rate": "some_pack_id"}
|
||||
|
||||
step = _apply_rate(op, index=0, state=state, pack_bindings=pack_bindings)
|
||||
|
||||
assert isinstance(step, SolutionStep)
|
||||
assert step.operation_kind == "apply_rate"
|
||||
assert state[("Tina", "dollars")] == 6.0 # 3 * 2
|
||||
assert state[("Tina", "cup")] == 3.0 # denom not consumed (per solver semantics)
|
||||
|
||||
|
||||
def test_confuser_no_denom_state_refuses():
|
||||
|
|
@ -110,17 +100,13 @@ def test_injected_apply_rate_does_not_create_wrong_on_known_refused_cases():
|
|||
We only assert the global wrong=0 invariant here (the runner is the
|
||||
authoritative counter); this test just exercises the new code on real text.
|
||||
"""
|
||||
# Pick two rate surfaces from the known refused set.
|
||||
# Pick two rate surfaces from the known refused set (isolated rate sentences
|
||||
# refuse because no denom state or question target; must never produce a wrong answer).
|
||||
for stmt in [
|
||||
"Tina makes $18.00 an hour.",
|
||||
"Alexa has a lemonade stand where she sells lemonade for $2 for one cup.",
|
||||
]:
|
||||
res = parse_and_solve(stmt, sealed=False)
|
||||
# Either no answer (refused) or a correct one; never a numeric that
|
||||
# would have been "wrong" if this were a scored case.
|
||||
if res.answer is not None:
|
||||
# For isolated rate sentence the only admissible answers would
|
||||
# be if the question side asked for the rate itself, which these
|
||||
# do not. So we expect refusal.
|
||||
assert False, f"Unexpected answer {res.answer} on isolated rate sentence"
|
||||
assert "no injection" in (res.refusal_reason or "") or res.refusal_reason is not None
|
||||
assert res.answer is None
|
||||
assert res.refusal_reason is not None
|
||||
assert "no injection" in (res.refusal_reason or "") or "requires" in (res.refusal_reason or "").lower() or "question" in (res.refusal_reason or "").lower()
|
||||
|
|
|
|||
|
|
@ -12,13 +12,12 @@ Covers the exact acceptance cases from the Workstream A Inc 2 brief:
|
|||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import types
|
||||
|
||||
from evals.refusal_taxonomy.shape_categories import ShapeCategory
|
||||
from generate.math_candidate_parser import CandidateOperation
|
||||
from generate.math_problem_graph import Rate
|
||||
from generate.math_roundtrip import roundtrip_admissible
|
||||
from generate.recognizer_anchor_inject import (
|
||||
inject_from_match,
|
||||
inject_rate_with_currency,
|
||||
|
|
@ -69,6 +68,7 @@ def test_rate_per_cup_emits_apply_rate_with_grounded_tokens():
|
|||
assert cand.matched_value_token == "2"
|
||||
assert cand.matched_unit_token == "dollars"
|
||||
assert cand.matched_verb in {"per", "a", "an", "each", "every"} # literal surface in sentence
|
||||
assert roundtrip_admissible(cand) is True
|
||||
|
||||
|
||||
def test_rate_an_hour_emits_when_an_in_rate_anchors():
|
||||
|
|
@ -83,6 +83,7 @@ def test_rate_an_hour_emits_when_an_in_rate_anchors():
|
|||
assert cand.op.operand.denominator_unit == "hour"
|
||||
assert cand.matched_verb == "an" # literal from sentence
|
||||
assert cand.matched_value_token == "18.00"
|
||||
assert roundtrip_admissible(cand) is True
|
||||
|
||||
|
||||
def test_unknown_actor_refuses_narrow_binding():
|
||||
|
|
@ -159,3 +160,40 @@ def test_dispatch_table_routes_rate_with_currency():
|
|||
# old "no injector registered" path that would have been the deferral.
|
||||
# We only assert that the call succeeded without KeyError / unexpected.
|
||||
assert isinstance(emitted, tuple)
|
||||
|
||||
|
||||
def test_an_rate_anchor_widening_is_contained_to_currency_rate_surfaces():
|
||||
""" "a"/"an" in RATE_ANCHORS must not open broad/generic apply_rate outside
|
||||
actual currency-rate surfaces (containment/confuser for the widening).
|
||||
|
||||
Even if matched_verb="a", a "dollars" unit_token only grounds (via the
|
||||
explicit $ branch we added) when the source actually contains "$".
|
||||
"""
|
||||
from generate.math_problem_graph import Operation, Rate
|
||||
from generate.math_candidate_parser import CandidateOperation
|
||||
from generate.math_roundtrip import roundtrip_admissible
|
||||
|
||||
rate = Rate(2.0, "dollars", "cup")
|
||||
op = Operation(actor="Tina", kind="apply_rate", operand=rate)
|
||||
|
||||
# Confuser: "a" verb + dollars unit, but no "$" symbol in source → unit fails to ground
|
||||
bogus = CandidateOperation(
|
||||
op=op,
|
||||
source_span="Tina makes a 2 cup thing.",
|
||||
matched_verb="a",
|
||||
matched_value_token="2",
|
||||
matched_unit_token="dollars",
|
||||
matched_actor_token="Tina",
|
||||
)
|
||||
assert roundtrip_admissible(bogus) is False
|
||||
|
||||
# Good rate surface with "a" + "$" → admissible
|
||||
good = CandidateOperation(
|
||||
op=op,
|
||||
source_span="Tina sells for $2 a cup.",
|
||||
matched_verb="a",
|
||||
matched_value_token="2",
|
||||
matched_unit_token="dollars",
|
||||
matched_actor_token="Tina",
|
||||
)
|
||||
assert roundtrip_admissible(good) is True
|
||||
|
|
|
|||
Loading…
Reference in a new issue