Skip to content

[Detail Bug] Markdown enrichments allow Unicode whitespace-only content, rendering empty cards #97

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_c113766b-ac9b-4139-a772-713aef6fc0dc

Introduced in #2 by @WilliamAGH on Sep 5, 2025

Summary

  • Context: The Background record is a domain value type representing a background information enrichment element in a Java chat application, used when parsing Markdown containing inline markers like {{background:content}}.
  • Bug: The constructor validation uses content.trim().isEmpty() which fails to detect Unicode whitespace-only content (e.g., \u3000 IDEOGRAPHIC SPACE), allowing visually empty enrichment cards to be created.
  • Actual vs. expected: Content consisting solely of Unicode whitespace characters should be rejected as empty, but instead bypasses validation and creates enrichment cards with no visible text content.
  • Impact: Users see enrichment cards rendered with headers ("Helpful Hints", "Background Context", etc.) but content areas containing only whitespace characters. This affects all five enrichment types.

Code with Bug

// Background.java:10-13
public Background {
    if (content == null || content.trim().isEmpty()) { // <-- BUG 🔴 trim() only removes ASCII whitespace
        throw new IllegalArgumentException("Background content cannot be null or empty");
    }
}
// EnrichmentPlaceholderizer.java:323-329
String content = context.markdown.substring(contentStartIndex, closingIndex).trim();  // <-- BUG 🔴 trim() preserves Unicode whitespace

if (content.isEmpty()) {
    return new EnrichmentProcessingResult(...);
}

The same constructor bug exists in: Hint.java, Warning.java, Example.java, Reminder.java.

Explanation

Java’s String.trim() only strips characters <= U+0020, so Unicode whitespace like IDEOGRAPHIC SPACE (\u3000) and EM SPACE (\u2003) survives both:

  • the upstream extraction .trim() + isEmpty() check, and
  • the enrichment record constructor’s .trim().isEmpty() validation.

As a result, Markdown such as {{hint:\u3000}} produces an enrichment card. Rendering shows headers and a content <p> </p> (whitespace-only), which is semantically empty but still treated as “has content” by the pipeline.

Codebase Inconsistency

The codebase predominantly uses String.isBlank() (216 instances) versus the legacy trim().isEmpty() pattern (10 instances). The enrichment constructors are part of the small set still using trim().isEmpty(), making them inconsistent with the codebase standard for “blank” checks.

Recommended Fix

  • In EnrichmentPlaceholderizer, use Unicode-aware stripping:
    String content = context.markdown.substring(contentStartIndex, closingIndex).strip();
  • In all enrichment constructors (Background, Hint, Warning, Example, Reminder), enforce the domain invariant with:
    if (content == null || content.isBlank()) {
        throw new IllegalArgumentException("Content cannot be null or empty");
    }

History

This bug was introduced in commit 3979bc1. The commit migrated MarkdownService from regex-based to AST-based processing and created six new enrichment record types (Background, Hint, Warning, Example, Reminder, ProcessingWarning) in a single refactor. Each type's compact constructor used content.trim().isEmpty() for null/empty validation, but Java's String.trim() only strips ASCII whitespace (characters <= U+0020), missing Unicode whitespace characters like IDEOGRAPHIC SPACE (U+3000). The bug slipped in because the developer likely assumed trim() handled all whitespace, and no test coverage existed for Unicode whitespace edge cases at the time.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions