fix: overlap dedupe drops enclosing beliefs (issue #3) - #4
Merged
Conversation
Overlap dedup suppressed any intersecting lower-confidence match, so an
assumption/causal clause wrapping a trailing 'I'll proceed' / 'I believe'
intention lost the enclosing belief entirely (flagship example dropped the
assumption).
- dedupeOverlaps: a candidate that encloses a kept match (starts strictly
before it, ends at or after it) is an enclosing belief and survives;
identical spans, same-start containment ('I will assume X'), and
crossing/partial overlaps keep the pre-existing first-ranked-wins
semantics. Sibling qualification markers (contradiction/self-correction
over one clause, e.g. 'But actually, X') still collapse as before.
- patterns: new CLAUSE_COMMA_END terminates causal/assumption captures at a
comma that introduces a first-person belief/intention clause
(', I believe ...' etc.), so the kept enclosing belief isn't polluted
with the trailing intention's text.
- tests: regression coverage for both issue-#3 repro phrases plus the
true-duplicate identical-span collapse case.
Closes #3
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Pull request overview
Fixes belief extraction overlap deduplication so enclosing causal/assumption beliefs are preserved when they contain a higher-confidence nested intention (issue #3), and refines clause termination so outer belief text doesn’t absorb the trailing intention clause.
Changes:
- Updated overlap dedupe logic in
dedupeOverlapsto keep enclosing beliefs while still collapsing strict containment and selected qualification-family overlaps. - Added
CLAUSE_COMMA_ENDand applied it to causal/assumption patterns to stop captures at, I <belief/intention marker>. - Added regression tests covering the issue #3 repro phrases and intended duplicate-span behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/lens/patterns.ts | Adds comma-based clause termination and applies it to causal/assumption patterns. |
| src/lens/extract.ts | Adjusts overlap dedupe semantics to preserve enclosing beliefs while keeping qualification-family collapse behavior. |
| src/lens/extract.test.ts | Adds regression tests for containment-aware dedupe and related pattern interactions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+44
to
+47
| * Intentionally narrow: only ', I <marker>' boundaries, so commas inside an | ||
| * unbroken clause ("the endpoint, port, and path are correct") never split. | ||
| */ | ||
| const CLAUSE_COMMA_END = String.raw`(?:,(?=\s*i\s+(?:believe|think|'ll|will|intend|plan\b))|[;!?\n]|\.(?=\s|$)|$)`; |
Comment on lines
+134
to
+140
| * Containment is belief-preserving (issue #3): a kept match only suppresses a | ||
| * later candidate it STRICTLY contains. A candidate whose span encloses or | ||
| * exactly matches the kept span is the enclosing belief (e.g. an assumption | ||
| * clause wrapping a trailing "I'll proceed" intention) or the | ||
| * duplicate-correct replacement, so it is kept and strictly-contained | ||
| * followers still collapse against it. Crossing/partial overlaps keep the | ||
| * pre-existing first-come (confidence-ranked) wins semantics. |
Comment on lines
+417
to
+429
| it('still collapses two patterns matching the identical span (first pattern wins)', async () => { | ||
| // "approach" here matches BOTH planning-explicit ("the plan is X") and | ||
| // uncertainty-hedge ("this could be wrong ...") over the identical span | ||
| // "the plan is to use the approach that could be wrong". They must still | ||
| // collapse to a single belief, resolved by confidence then pattern order. | ||
| const beliefs = await extractBeliefs( | ||
| 'The plan is to use the approach that could be wrong.', | ||
| fixedOpts('issue-3c'), | ||
| ); | ||
| expect(beliefs.length).toBe(1); | ||
| expect(beliefs[0]!.type).toBe('planning'); | ||
| expect(beliefs[0]!.belief.toLowerCase()).toContain('the plan is'); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Overlap deduplication in
src/lens/extract.tssuppressed any intersecting lower-confidence match. When a lower-confidence match fully CONTAINS a higher-confidence inner match, the outer belief (an assumption wrapping an intention, a causal clause feedingI believe) was silently dropped - the exact flagship shape the product exists to surface.Fix
dedupeOverlaps(extract.ts): a candidate whose span encloses a kept match (starts strictly before it, ends at or after it) is an enclosing belief and is kept alongside the inner match. Identical spans still collapse (first-ranked pattern wins); same-start containment (I will assume X) and crossing/partial overlaps keep pre-existing semantics. Sibling qualification markers (contradiction/self-correction over one clause, e.g.But actually, X) still collapse to the higher-confidence claim.CLAUSE_COMMA_END(patterns.ts): causal/assumption captures now also terminate at a comma introducing a first-person belief/intention clause (, I believe ...,, I'll ...), so the kept enclosing belief isn't polluted with the trailing intention's text.Verification (real run)
npm run check(tsc --noEmit + vitest run): 36 test files, 344/344 tests passing.Live extraction evidence (vitest one-off against the extractor):
Pre-existing contracts preserved: "But actually, X" still yields exactly one self-correction (0.5), "I will assume X" still lands as a single intention, "I'm assuming X" still yields one assumption.
Closes #3