Skip to content

fix(dspy): Normalize COMPLETE sentinel detection in dataset summary loop - #86

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-dspy-normalize-complete-sentinel-detection-in-9009fb
Open

fix(dspy): Normalize COMPLETE sentinel detection in dataset summary loop#86
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-dspy-normalize-complete-sentinel-detection-in-9009fb

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 6, 2026

Copy link
Copy Markdown

Warning

GitHub issue creation failed

Detail attempted to publish this bug to GitHub, but the issue could not be created. This fix PR was created without that issue, and missing tracker references are shown as Unknown issue.

You can review and merge this PR normally. Please review your tracker integration settings before the next publish run.

Detail bug report: View on Detail

📝 Changes Description

Closes Unknown issue

  • Bug: In dspy/propose/dataset_summary_generator.py, create_dataset_summary detects a "COMPLETE" early-stop signal with a brittle 8-character prefix check, output["observations"][:8].upper() == "COMPLETE". It mis-detects in both directions:
    • False negative: the DatasetDescriptorWithPriorObservations prompt asks the model to say 'COMPLETE' (single-quoted). When the model echoes that quoted form, the parsed value is 'COMPLETE'; its first 8 chars ('COMPLET) don't match, so the sentinel is missed, the quoted token is appended verbatim to the observations, and the skips >= 5 early-stop never fires (wasting LM calls). The corrupted observations are then fed to ObservationSummarizer and onward to GroundedProposer's dataset_description.
    • False positive: any real observation whose first 8 chars match "COMPLETE" (e.g. "Completely analyzed the data", "Completed the review...") is silently dropped as if it were the sentinel.
  • Fix: Replace the prefix match with normalized, word-boundary detection:
    obs = output["observations"].strip().strip("\"'").strip()
    if obs.upper().startswith("COMPLETE") and (len(obs) == 8 or not obs[8].isalpha()):
    Stripping surrounding quotes/whitespace closes the false negative; requiring a non-alpha boundary after COMPLETE closes the false positive (so "Completely..." is preserved while COMPLETE, 'COMPLETE', "COMPLETE", and COMPLETE. are still treated as the sentinel).
  • Testing:
    • Added tests/propose/test_dataset_summary_generator.py (7 cases) exercising the real detector line through DSPy's ChatAdapter parse path with DummyLM. Sentinels (COMPLETE, 'COMPLETE', "COMPLETE", COMPLETE.) are skipped rather than appended to the observations passed to ObservationSummarizer; real observations starting with "Complete" ("Completely analyzed the data", "Completed the review of all samples") are preserved; repeated quoted 'COMPLETE' triggers the skips >= 5 early-stop (asserted via LM call count) instead of leaking quoted tokens into the summary input.
    • Regression check (not versioned): reverted the detector to the buggy line and re-ran the new tests — 5 of 7 fail (quoted sentinels appended; "Complete..." observations dropped; early-stop not triggered), confirming the new tests guard both failure modes. The 2 that still pass (COMPLETE, COMPLETE.) are intentional no-regression guards for the bare/punctuated forms the old code already handled. Fix restored after.
    • End-to-end smoke (not versioned, requires a live LM): ran GroundedProposer with a non-empty trainset and use_dataset_summary=True against a local ollama/llama3.2:3b model — the produced data_summary contained no stray COMPLETE tokens and instruction proposal succeeded.
    • Routine checks all pass: the existing tests/propose/test_grounded_proposer.py, the tests/teleprompt/ regression suite, the full default test suite (-m 'not extra and not deno'), the extra/deno suites, the llm_call suite, and ruff check / ruff format. No regressions.
    • Could not run: 19 provider-API-key-gated tests in tests/clients/test_lm_direct_live.py were skipped (not failed) because OPENAI_API_KEY / ANTHROPIC_API_KEY / GEMINI_API_KEY were not set in the environment.

✅ Contributor Checklist

  • Pre-Commit checks are passing (locally and remotely)
  • Title of your PR / MR corresponds to the required format
  • Commit message follows required format {label}(dpy): {message}

⚠️ Warnings

This change was produced with assistance from Detail (an AI coding agent). The bug was reproduced against DSPy's real ChatAdapter parse path before patching, and the fix was validated end-to-end against a local Ollama model; every line submitted was understood and tested. Submitting this PR myself per the repo's guidance — please review.


Automatic Fixes PRs can be configured here.

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR normalizes quoted COMPLETE responses in the dataset-summary loop and adds regression coverage for sentinel handling.

  • Strips surrounding whitespace and quotes before sentinel detection.
  • Preserves observations beginning with “Completely” or “Completed.”
  • Adds tests for quoted and punctuated sentinels and repeated-sentinel early termination.
  • The new boundary condition remains too broad for legitimate prose beginning with the standalone word “Complete.”

Confidence Score: 4/5

The PR should not merge until legitimate observations beginning with the standalone word “Complete” are distinguished from the sentinel.

The revised detector accepts every non-letter after COMPLETE as a sentinel boundary, causing realistic free-form observations such as “Complete analysis…” to be dropped and potentially triggering premature early termination.

Files Needing Attention: dspy/propose/dataset_summary_generator.py, tests/propose/test_dataset_summary_generator.py

Important Files Changed

Filename Overview
dspy/propose/dataset_summary_generator.py Normalizes COMPLETE detection, but still drops legitimate observations beginning with “Complete” followed by whitespace or punctuation.
tests/propose/test_dataset_summary_generator.py Adds focused sentinel and early-stop regression tests, but omits the standalone-word boundary case that exposes the remaining false positive.

Reviews (1): Last reviewed commit: "fix(dspy): Normalize COMPLETE sentinel d..." | Re-trigger Greptile

output = dspy.Predict(DatasetDescriptorWithPriorObservations, n=1, temperature=1.0)(prior_observations=observations, examples=order_input_keys_in_string(trainset[b:upper_lim].__repr__()))
if len(output["observations"]) >= 8 and output["observations"][:8].upper() == "COMPLETE":
obs = output["observations"].strip().strip("\"'").strip()
if obs.upper().startswith("COMPLETE") and (len(obs) == 8 or not obs[8].isalpha()):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Complete Prose Is Discarded

A legitimate observation such as Complete analysis of the samples starts with COMPLETE and has a non-alphabetic ninth character, so this condition treats it as the sentinel. The observation is discarded and increments skips; repeated matches can stop dataset inspection early, producing an incomplete dataset summary. The detector should accept only COMPLETE or an explicitly supported punctuation-only form, rather than every non-letter suffix.

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