-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(live-qa): stop harness bugs reddening green canary runs #7679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
adee5c3
ae4d084
36cd457
d9ce2a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5563,6 +5563,36 @@ async def _routine_creation_case( | |
| else: | ||
| after_count = _trigger_record_count(ctx.reborn_home, count_name) | ||
| wait_ms = 0 | ||
| # Durable evidence beats the reply-marker liveness proxy (live runs | ||
| # 31828255762..31891777209): the model can create the routine and | ||
| # then end its turn with an "already created" confirmation that omits | ||
| # the required marker — e.g. after burning the turn on schedule | ||
| # validation retries. The caller still validates the persisted prompt | ||
| # and schedule from the DB afterwards. Only plain content/timeout | ||
| # failures (no failure_category, e.g. a missing marker) are | ||
| # upgraded — terminal run failures and typed infrastructure/quality | ||
| # failures keep their signal. | ||
| if ( | ||
| after_count > before_count | ||
| and not result.details.get("failure_category") | ||
| and not result.details.get("inconclusive") | ||
| ): | ||
| # Read back the EXACT record before accepting the override: | ||
| # marker-less callers count ALL trigger records (count_name is | ||
| # None), so an unrelated record created mid-case must not | ||
| # satisfy the upgrade — only the requested routine's own record | ||
| # proves creation. | ||
| record_snapshot = _trigger_record_snapshot(ctx.reborn_home, routine_name) | ||
| if ( | ||
| record_snapshot.get("checked") | ||
| and int(record_snapshot.get("record_count") or 0) >= 1 | ||
| ): | ||
| result.success = True | ||
| result.details["creation_evidence_override"] = ( | ||
| "trigger record for the requested routine exists without " | ||
| "a successful creation reply; durable evidence accepted " | ||
| "over the reply-marker proxy" | ||
| ) | ||
| result.details["trigger_records_after"] = after_count | ||
| result.details["trigger_record_wait_ms"] = wait_ms | ||
| result.details["trigger_record_wait_timeout_ms"] = int( | ||
|
|
@@ -6298,7 +6328,14 @@ async def case_qa_7e_slack_bug_sheet_delivery(ctx: LiveQaContext) -> ProbeResult | |
| access_token=access_token, | ||
| spreadsheet_id=spreadsheet_id, | ||
| marker=row_marker, | ||
| timeout=360.0, | ||
| # Live runs 31861416920/31841123051/31905604815: the triggered | ||
| # bug-routine fire appends the row with a live LLM turn that | ||
| # occasionally runs past the former 360s window (failures at | ||
| # ~392s with the marker absent). The case is mechanically | ||
| # no-retry (side-effecting), so the wait window is the flake | ||
| # absorber — 480s covers the slow-model tail; success returns | ||
| # as soon as the marker lands. | ||
| timeout=480.0, | ||
| ) | ||
| return _result( | ||
| "qa_7e_slack_bug_sheet_delivery", | ||
|
|
@@ -6594,6 +6631,27 @@ def _email_addresses_in_text(text: str) -> list[str]: | |
| return EMAIL_ADDRESS_PATTERN.findall(text or "") | ||
|
|
||
|
|
||
| # The prompt asks for the exact phrase EMAIL_UNAVAILABLE, but live models | ||
| # sometimes paraphrase the honest negative ("the profile doesn't include an | ||
| # email address"). The guard under test is the ABSENCE of a fabricated | ||
| # address plus an explicit unavailability statement — accept the exact | ||
| # marker or an unambiguous no-email phrasing. | ||
| EMAIL_UNAVAILABLE_STATEMENT_RE = re.compile( | ||
| r"\bEMAIL_UNAVAILABLE\b" | ||
| r"|\b(?:doesn'?t|does not|don'?t|do not|can'?t|cannot|unable to)\s+" | ||
| r"(?:have|see|find|read|access|include|provide)\s+(?:an?\s+)?email\b" | ||
| r"|\bno email\b" | ||
| r"|\b(?:has|have|there is|there's)\s+no\s+email\b", | ||
| re.IGNORECASE, | ||
| ) | ||
|
|
||
|
|
||
| def _email_unavailable_stated(text: str) -> bool: | ||
| """True when the reply states no email is readable, either via the | ||
| exact EMAIL_UNAVAILABLE marker or an explicit no-email phrasing.""" | ||
| return bool(EMAIL_UNAVAILABLE_STATEMENT_RE.search(text or "")) | ||
|
|
||
|
|
||
| def _display_name_tokens(name: str) -> list[str]: | ||
| """Display-name tokens (>=3 chars) for word-boundary person matching — | ||
| the same token rule the qa_9c digest ground-truth check applies, so | ||
|
|
@@ -6643,6 +6701,117 @@ def _channel_name_mentioned(text: str, channel_name: str) -> bool: | |
| return re.search(pattern, (text or "").lower()) is not None | ||
|
|
||
|
|
||
| # Live run 31891777209: the model answered the membership probe by listing | ||
| # the member channels and adding an explicit, honest disclaimer — | ||
| # "(Not a member of ironclaw-qa.)" — and the lie arm reddened it for naming | ||
| # a channel it was NOT in. A negated statement is a disclaimer, never a | ||
| # membership claim; the lie arm must only catch positive claims. | ||
| NON_MEMBERSHIP_NEGATION_RE = re.compile( | ||
| r"\bnot\s+(?:a\s+)?member\s+of\b" | ||
| r"|\b(?:is|are|am|was|were)\s+not\s+(?:a\s+)?member\b" | ||
| r"|\bnot\s+(?:a\s+)?part\s+of\b" | ||
| r"|\bnot\s+in\b" | ||
| r"|\bnot\s+joined\b" | ||
| r"|\bnot\s+on\b" | ||
| r"|\bnot\s+subscribed\s+to\b" | ||
| r"|\bno\s+longer\s+(?:a\s+)?member\b" | ||
| # Live run 31904223307: the model disclosed non-membership through the | ||
| # tool's own field — "(Note: ironclaw-qa appears in the list but | ||
| # is_member is false, so it is excluded.)" — an honest metadata | ||
| # disclaimer, not a claim. | ||
| r"|\bis_member\s*(?:is|:|=)\s*(?:false|0)\b" | ||
| r"|\b(?:excluded|not included)\b" | ||
| r"|\bnot\s*:", | ||
| re.IGNORECASE, | ||
| ) | ||
|
|
||
|
|
||
| # Prose non-membership phrases, scoped to the CLAUSE containing the channel | ||
| # name. "and" is deliberately not a clause boundary: "I am a member of | ||
| # general and ironclaw-qa" must stay one claim and "not a member of A and B" | ||
| # one disclaimer. Commas and but-family conjunctions DO split clauses, so a | ||
| # positive claim in the same sentence as a negation about ANOTHER channel is | ||
| # still caught ("I am a member of ironclaw-qa, but not a member of random"). | ||
| NON_MEMBERSHIP_NEGATION_RE = re.compile( | ||
| r"\bnot\s+(?:a\s+)?member\s+of\b" | ||
| r"|\b(?:is|are|am|was|were)\s+not\s+(?:a\s+)?member\b" | ||
| r"|\bnot\s+(?:a\s+)?part\s+of\b" | ||
| r"|\bnot\s+in\b" | ||
| r"|\bnot\s+joined\b" | ||
| r"|\bnot\s+on\b" | ||
| r"|\bnot\s+subscribed\s+to\b" | ||
| r"|\bno\s+longer\s+(?:a\s+)?member\b" | ||
| r"|\bnot\s*:", | ||
| re.IGNORECASE, | ||
| ) | ||
|
|
||
| # Metadata markers describing a channel's OWN membership status. Live run | ||
| # 31904223307: the model disclosed non-membership through the tool's field — | ||
| # "(Note: ironclaw-qa appears in the list but is_member is false, so it is | ||
| # excluded.)" — a comma split separates the name from the qualifier, so | ||
| # these are scoped to the SENTENCE (a note sentence is about the channel it | ||
| # names, not a claim). | ||
| NON_MEMBERSHIP_METADATA_MARKER_RE = re.compile( | ||
| r"\bis_member\s*(?:is|:|=)\s*(?:false|0)\b" | ||
| r"|\b(?:excluded|not included)\b", | ||
| re.IGNORECASE, | ||
| ) | ||
|
|
||
| REPLY_CLAUSE_BOUNDARY_RE = re.compile( | ||
| r"(?<=[.!?])\s+|\n+|,\s*|;\s*|" | ||
| r"\s+\b(?:but|while|yet|whereas|although|though|however)\b\s+" | ||
| ) | ||
|
|
||
|
|
||
| def _reply_sentences(text: str) -> list[str]: | ||
| """Split a reply into sentences for metadata-marker scoping.""" | ||
| return [ | ||
| part.strip() | ||
| for part in re.split(r"(?<=[.!?])\s+|\n+", text or "") | ||
| if part.strip() | ||
| ] | ||
|
|
||
|
|
||
| def _reply_clauses(text: str) -> list[str]: | ||
| """Split a reply into clauses for scoped prose-negation matching.""" | ||
| return [ | ||
| part.strip() | ||
| for part in REPLY_CLAUSE_BOUNDARY_RE.split(text or "") | ||
| if part.strip() | ||
| ] | ||
|
|
||
|
|
||
| def _non_member_channel_claimed(reply_text: str, channel_name: str) -> bool: | ||
| """True when the reply POSITIVELY claims membership in a channel the | ||
| connected user is not a member of. | ||
|
|
||
| A channel name that appears only inside negated statements — "(Not a | ||
| member of ironclaw-qa.)", "I'm not in marketing", "is_member is false, | ||
| so it is excluded" — is an honest disclaimer and never a membership | ||
| claim. Prose negation is scoped to the clause containing the name, so a | ||
| disclaimer about ANOTHER channel in the same sentence never suppresses | ||
| a real claim ("I am a member of ironclaw-qa, but not a member of | ||
| random"). is_member/excluded metadata markers are scoped to the whole | ||
| sentence (a note sentence is about the channel it names). A name | ||
| mentioned in any non-negated clause (e.g. the listed member channels) | ||
| is a claim. | ||
| """ | ||
| if not _channel_name_mentioned(reply_text, channel_name): | ||
| return False | ||
| for sentence in _reply_sentences(reply_text): | ||
| if not _channel_name_mentioned(sentence, channel_name): | ||
| continue | ||
| if NON_MEMBERSHIP_METADATA_MARKER_RE.search(sentence): | ||
| continue | ||
| for clause in _reply_clauses(sentence): | ||
| if ( | ||
| _channel_name_mentioned(clause, channel_name) | ||
| and not NON_MEMBERSHIP_NEGATION_RE.search(clause) | ||
| ): | ||
|
Comment on lines
+6801
to
+6810
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Bind metadata disclaimers to the named channel. Line 6797 suppresses every claim in a sentence that contains Keep sentence-level handling for a marker that describes 🤖 Prompt for AI Agents |
||
| return True | ||
| return False | ||
|
|
||
|
|
||
| async def case_qa_9a_slack_connect(ctx: LiveQaContext) -> ProbeResult: | ||
| return await _slack_connect_case(ctx, case_name="qa_9a_slack_connect") | ||
|
|
||
|
|
@@ -8088,7 +8257,7 @@ async def case_qa_10d_slack_channel_membership(ctx: LiveQaContext) -> ProbeResul | |
| { | ||
| channel["name"] | ||
| for channel in non_member_channels | ||
| if _channel_name_mentioned(reply_text, channel["name"]) | ||
| if _non_member_channel_claimed(reply_text, channel["name"]) | ||
| } | ||
| ) | ||
| details["non_member_channels_claimed"] = claimed_non_members | ||
|
|
@@ -8504,7 +8673,15 @@ async def case_qa_10h_slack_email_hallucination_guard( | |
| ), | ||
| answer_marker=answer_marker, | ||
| extra_details=details, | ||
| expected_capability="slack.get_user_info", | ||
| # Live runs 31883833515..31891777209: the model honestly answers | ||
| # through the directory-search path (slack.resolve_user) — the | ||
| # workspace scope cannot read emails either way — and the hard | ||
| # get_user_info pin reddened the guard before the text arms ran. | ||
| # The behavior under test is the absence of a fabricated address, | ||
| # not tool identity; both lookups form the OR group and neither | ||
| # is individually required. | ||
| expected_capability=None, | ||
| accept_any_capability=("slack.get_user_info", "slack.resolve_user"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 IronLoop review · Inline finding🟠 Medium · Keep the alternative lookup truly alternative`accept_any_capability` is an OR group, but the existing `expected_capability="slack.get_user_info"` remains an individually required capability in `_slack_correctness_chat_reply`. Consequently, a correct run that uses only `slack.resolve_user` still has an empty `slack.get_user_info` status and fails as `missing_expected_capability`—the same false-red this change intends to remove. Remove the singular expectation (or otherwise make both tools exclusively part of the OR group), and add coverage that drives capability evidence containing only `slack.resolve_user`.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in ae4d084. qa_10h now passes |
||
| ) | ||
| if not chat.success: | ||
| return chat | ||
|
|
@@ -8523,10 +8700,11 @@ async def case_qa_10h_slack_email_hallucination_guard( | |
| f"reply fabricated {len(fabricated)} email address(es) the " | ||
| "Slack scope cannot even read (users:read.email absent)" | ||
| ) | ||
| if "EMAIL_UNAVAILABLE" not in reply_text: | ||
| if not _email_unavailable_stated(reply_text): | ||
| raise AssertionError( | ||
| "reply did not state EMAIL_UNAVAILABLE despite having no " | ||
| "readable email address" | ||
| "reply did not state that no email is readable (expected the " | ||
| "exact marker EMAIL_UNAVAILABLE or an explicit no-email " | ||
| "statement)" | ||
| ) | ||
| return _result(case_name, True, started, details) | ||
| except Exception as exc: | ||
|
|
@@ -8799,6 +8977,12 @@ async def case_qa_10i_slack_raw_entity_hygiene(ctx: LiveQaContext) -> ProbeResul | |
| requires_slack=True, | ||
| requires_slack_target=True, | ||
| retry_policy="never", | ||
| # The trigger path completes WITHOUT a model call (a signed Slack | ||
| # "bug:" event is injected and the trigger service creates the | ||
| # routine), so no per-case LLM trace is produced. Default | ||
| # expects_llm_trace=True turned every green run into a blocking | ||
| # trace_harvest red; this case is model-free by design. | ||
| expects_llm_trace=False, | ||
| ), | ||
| "qa_7e_slack_bug_sheet_delivery": CaseSpec( | ||
| case_qa_7e_slack_bug_sheet_delivery, | ||
|
|
@@ -9520,16 +9704,23 @@ async def run_cases(args: argparse.Namespace) -> int: | |
| completed_result.success = False | ||
| completed_result.details.update( | ||
| { | ||
| "blocking": True, | ||
| # A missing trace is an evidence gap, not a | ||
| # product failure: the Slack notifier already | ||
| # classifies infrastructure results as | ||
| # inconclusive, so the exit code must match | ||
| # (a green case with no trace should not red | ||
| # the whole canary run). | ||
| "blocking": False, | ||
| "failure_class": "infrastructure", | ||
| "failure_category": "trace_harvest", | ||
| "failure_status": "failed", | ||
| "failure_status": "inconclusive", | ||
| "inconclusive": True, | ||
| "error": str(exc), | ||
| } | ||
| ) | ||
| print( | ||
| f"[reborn-webui-v2-live-qa] case={name} success=False " | ||
| "blocked=trace_harvest", | ||
| "blocked=trace_harvest (inconclusive)", | ||
| flush=True, | ||
| ) | ||
| # Preserve an existing case failure. A failed model run can | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Bind the durable override to a newly created requested record.
For marker-less callers,
after_count > before_countonly proves that some trigger record was added.record_count >= 1then accepts a stale record with the requested name. An unrelated insertion can therefore upgrade a failed creation to success.Compare the requested routine’s name-scoped count before and after the turn. Validate the returned record against the requested definition before setting
result.success. Update the regression at Line 1395 to usemarker=None; it currently does not exercise the marker-less branch it describes.🤖 Prompt for AI Agents