Skip to content

fix: remove empty tagged messages after tool drop (Issue #97)#98

Merged
alfonso-magic-context merged 7 commits into
cortexkit:masterfrom
Zireael:fix/issue-97-multiplying-numbers
Jun 4, 2026
Merged

fix: remove empty tagged messages after tool drop (Issue #97)#98
alfonso-magic-context merged 7 commits into
cortexkit:masterfrom
Zireael:fix/issue-97-multiplying-numbers

Conversation

@Zireael

@Zireael Zireael commented May 24, 2026

Copy link
Copy Markdown
Contributor

Problem (#97)

Magic Context tags visible message parts with §N§ prefixes so the agent can reference them (ctx_reduce(§3§)). Two related failure modes were reported:

1. Empty tagged messages after tool drop

When execute passes drop most tool structure, user messages that contain only a tag prefix (e.g. §424§) were not removed. hasMeaningfulPart treated the raw § characters as content. These ghost messages pollute the transcript and confuse the model.

2. Multiplying / bleeding tag numbers in chat UI

Assistant and user-visible text accumulated leading numbers and cargo-cult tag notation, e.g.:

  • 2030 2030 2030° Run clippy again:
  • 99 Actually executing now. Running fmt:
  • Blank user messages that are only a number
  • Mid-text §40827§ cargo-cult from models mimicking MC notation

Root cause (persistence vs transform mismatch):

  1. The transform layer injects §N§ on each pass via prependTag / stripTagPrefix.
  2. OpenCode experimental.text.complete (and Pi message_end persistence) previously stripped all § characters globally. Well-formed §2030§ became bare 2030 — digit residue with no section signs.
  3. Transform stripTagPrefix only removed leading §-shaped prefixes, not bare digits. Each transform pass prepended a fresh §N§ in front of the residue → numbers stacked in persisted/UI text.
  4. A blind leading-digit regex was considered but rejected: it breaks legitimate content (99 files…, 6 8 9 tasks…, 2024 roadmap, numbered lists).

Issue #97’s original fix targeted tagger/DB counter drift and empty tagged messages; it did not fully address the persistence/transform split above.


What this PR changes

Tool drop — remove empty tagged messages

  • hasMeaningfulPart in tool-drop-target.ts now uses shared stripTagPrefix before checking whether text is non-empty (well-formed and malformed leading §N§ shapes).
  • Tests in tool-drop-target.test.ts cover tag-only and malformed-prefix cases.

Persistence boundary — pair-aware strip (OpenCode + Pi)

  • New stripPersistedAssistantText() in tag-content-primitives.ts:
    1. Leading ^(§N§\s*)+ runs
    2. Global removal of whole §N§ pairs (not § alone)
    3. Global malformed §N">… hybrids (compartment/XML confusion)
    4. Stray lone § after pair removal
  • Wired into text-complete.ts and pi-plugin/strip-tag-prefix.ts so assistant text is clean before jsonl persistence and Pi UI render.

Transform boundary — § notation only

  • stripTagPrefix / prependTag strip only leading §-shaped MC notation (well-formed + malformed). They do not strip bare leading digits.
  • Prevents false positives on real user/assistant content while still deduplicating tag prefixes before re-injection.

Temporal awareness

  • OpenCode temporal-awareness.ts and Pi temporal-awareness-pi.ts use shared peelLeadingMcTagNotation() so gap markers insert after malformed prefixes too, not only §N§ regex matches.

Out of scope (follow-up)

  • Session-aware legacy heal for already-persisted bare-digit pollution without § (e.g. 2030 2030 2030° with no section signs). New assistant completions with well-formed §N§ pairs are cleaned at persistence; old residue may remain until rewritten.

Test plan

  • bun test packages/plugin/src/hooks/magic-context/tag-content-primitives.test.ts
  • bun test packages/plugin/src/hooks/magic-context/text-complete.test.ts
  • bun test packages/plugin/src/hooks/magic-context/tool-drop-target.test.ts
  • bun test packages/pi-plugin/src/strip-tag-prefix.test.ts
  • Manual: reproduce session with tool-drop + assistant cargo-cult; confirm UI shows no stacked leading numbers on new completions
  • Manual: confirm 99 files… / numbered-list user text is preserved after transform

Closes #97

Greptile Summary

This PR fixes two related failures caused by a mismatch between how Magic Context tags are injected at the transform layer and stripped at the persistence boundary. It introduces stripPersistedAssistantText as a shared pipeline (whole §N§ pair removal, malformed hybrid removal, stray § cleanup, then trim) consumed by both OpenCode's text-complete.ts and Pi's strip-tag-prefix.ts, and updates hasMeaningfulPart to run stripTagPrefix before emptiness checks so tag-only ghost messages are pruned after tool drops.

  • Ghost message fix: hasMeaningfulPart now calls stripTagPrefix before .trim().length > 0, correctly treating a part whose text is only §424§ (or malformed §15298\">§15298§) as empty and removing the message.
  • Persistence boundary: stripPersistedAssistantText removes whole §N§ pairs globally (not bare § characters), eliminating digit residue that previously survived global §-only stripping and stacked on re-injection passes.
  • Transform layer preserved: stripTagPrefix and prependTag still only strip leading §-shaped notation, explicitly preserving legitimate leading digits (99 files…, 2024 roadmap, numbered lists).

Confidence Score: 5/5

Safe to merge; changes are narrowly scoped to tag-stripping helpers and the emptiness check in hasMeaningfulPart, all guarded by updated unit tests.

The ghost-message fix in hasMeaningfulPart is minimal and the logic is straightforward. The new stripPersistedAssistantText pipeline replaces duplicate inline regexes in two places with a well-tested shared function. The transform-layer strip (stripTagPrefix) is deliberately unchanged for legitimate-digit preservation. Prior review concerns (trailing-space residue, missing byteSize/isThinkingPart tests, JSDoc placement) are all addressed in this revision.

No files require special attention.

Important Files Changed

Filename Overview
packages/plugin/src/hooks/magic-context/tag-content-primitives.ts Adds stripPersistedAssistantText (4-step pipeline) plus helper exports and a multi-pass loop in stripTagPrefix. Implementation is correct and well-reasoned.
packages/plugin/src/hooks/magic-context/tool-drop-target.ts Core fix: hasMeaningfulPart now calls stripTagPrefix before checking trimmed length, correctly identifying tag-only ghost messages as empty.
packages/plugin/src/hooks/magic-context/text-complete.ts Replaces two-step inline strip with a call to stripPersistedAssistantText, centralising persistence-boundary logic.
packages/pi-plugin/src/strip-tag-prefix.ts Simplified to a thin wrapper over stripPersistedAssistantText, removing 60+ lines of duplicated logic.
packages/plugin/src/hooks/magic-context/temporal-awareness.ts Refactors inline tag-prefix peeling to use peelLeadingMcTagNotation, now also handling malformed prefixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Assistant completion] -->|text| B[stripPersistedAssistantText]
    B --> B1["1. Strip leading well-formed runs"]
    B1 --> B2["2. Remove all complete pairs globally"]
    B2 --> B3["3. Remove malformed hybrids globally"]
    B3 --> B4["4. Remove stray section signs"]
    B4 --> B5["5. trim()"]
    B5 --> C[Clean persisted text]
    D["Tool drop hasMeaningfulPart"] -->|text part| E[stripTagPrefix leading only]
    E --> F{empty after trim?}
    F -- yes --> G[Remove ghost message]
    F -- no --> H[Keep message]
Loading

Reviews (3): Last reviewed commit: "fix: address greptile review — trim pers..." | Re-trigger Greptile

Strips tag prefixes before checking if a text part is meaningful. Prevents empty user messages (containing only section tags) from confusing the LLM and causing hallucinated number multiplication.

Closes cortexkit#97

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 2 files

Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more

Re-trigger cubic

Build a portable plugin+cli tree with TUI sources and runtime deps instead of uploading dist-only folders that break OpenCode TUI loading.

@cubic-dev-ai cubic-dev-ai Bot 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.

1 issue found across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread .github/workflows/windows-build.yml Outdated
Zireael added 2 commits May 29, 2026 05:52
Replace digit-residue regex with pair-aware persistence cleanup so assistant text-complete and Pi message_end remove whole §N§ tokens without breaking legitimate leading numbers (99 files, 2024 roadmap).

Transform stripTagPrefix stays §-notation-only; temporal awareness uses shared peelLeadingMcTagNotation. Unifies tool-drop hasMeaningfulPart.

Related to cortexkit#97
Remove branch-scoped workflow and packaging script; not part of cortexkit#97 tag/persistence fixes.
Biome config enforces 4-space indentation but these files used tabs.
Also fixes missing trailing newline in strip-tag-prefix.ts.
Run: bun run --cwd packages/pi-plugin lint passes clean.

Related to cortexkit#97
Comment thread packages/plugin/src/hooks/magic-context/tag-content-primitives.ts
Comment thread packages/plugin/src/hooks/magic-context/text-complete.ts Outdated
Zireael added 2 commits June 4, 2026 18:37
Run biome format on packages/plugin to fix indentation and line
spacing in files introduced by PR cortexkit#97. Also collapses overly-split
.toBe() assertions in test files.

Related to cortexkit#97
…, move JSDoc

- stripPersistedAssistantText: add .trim() to prevent whitespace-only
  assistant messages after malformed-prefix cleanup (Greptile P2)
- Restore unit tests for byteSize and isThinkingPart removed during
  rewrite (Greptile P2)
- Move file-level JSDoc after import in text-complete.ts (Greptile P2)

Related to cortexkit#97
@Zireael

Zireael commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

It's green! 😀

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 10 files

Re-trigger cubic

@alfonso-magic-context alfonso-magic-context merged commit 2d08921 into cortexkit:master Jun 4, 2026
11 checks passed
ualtinok added a commit that referenced this pull request Jun 4, 2026
Post-merge cleanup of PR #98:
- Pi's strip-tag-prefix.ts and temporal-awareness-pi.ts imported the shared
  tag-content-primitives via a raw cross-package path
  (../../plugin/src/...). Switch to the `@magic-context/core/...` workspace
  alias used everywhere else in Pi (the only two ../../plugin/src imports in
  the package).
- Fix a garbled test description: "transform ? ? notation only" → the intended
  "transform §N§ notation only" (the § chars were mangled on the way in).

No behavior change. Plugin 1796/0, Pi 403/0, tsc + biome clean.

Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
KorenKrita added a commit to KorenKrita/magic-context that referenced this pull request Jul 7, 2026
cortexkit#97)

Persistence-boundary strip now removes whole §N§ pairs (not bare §), eliminating the digit-residue that stacked across turns into runaway 'multiplying numbers' overflow (cortexkit#97). hasMeaningfulPart strips the tag prefix before the emptiness check so tag-only ghost messages are pruned. Shared tag-strip helpers in tag-content-primitives.ts are reused by both OpenCode and Pi.

Thanks to @Zireael for the fix.
KorenKrita added a commit to KorenKrita/magic-context that referenced this pull request Jul 7, 2026
Post-merge cleanup of PR cortexkit#98:
- Pi's strip-tag-prefix.ts and temporal-awareness-pi.ts imported the shared
  tag-content-primitives via a raw cross-package path
  (../../plugin/src/...). Switch to the `@magic-context/core/...` workspace
  alias used everywhere else in Pi (the only two ../../plugin/src imports in
  the package).
- Fix a garbled test description: "transform ? ? notation only" → the intended
  "transform §N§ notation only" (the § chars were mangled on the way in).

No behavior change. Plugin 1796/0, Pi 403/0, tsc + biome clean.

Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
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.

[Bug] Chat corruption with repeating chat number markers, leading to catastrophic overflow failure

2 participants