Add defensive check for data URL prefix in summarizeDataUrl

[pavankumar-vh]

Overview

Add a defensive check for the data URL prefix in summarizeDataUrl function in common/src/util/cache-debug.ts.

Bug Description

The summarizeDataUrl function was called from summarizeLargeValue which checks if a string starts with data: before calling it. However, if summarizeDataUrl were ever called from a different context or if the prefix check were removed, the function would incorrectly try to parse non-data-URL strings, potentially producing misleading summaries.

Fix

Added a defensive check at the start of summarizeDataUrl that returns the value unchanged if it doesn't start with data:. This makes the function safer to use independently and prevents incorrect parsing of malformed inputs.

Testing

Added comprehensive test coverage demonstrating:

1. Valid data URLs are properly summarized (image/png with base64 payload)
2. Non-data-URL strings (https://example.com/image.png) pass through unchanged
3. Edge case: data URL-like strings without comma are handled gracefully

All 3 tests pass.

Files Changed

• common/src/util/cache-debug.ts - Added defensive prefix check
• common/src/util/__tests__/cache-debug.test.ts - New test file with 3 test cases

Scope

This change only touches common/ which is an approved contribution area per the Contributing Guide.

[codebuff-team]

Thanks for the tests - they're clear and correctly demonstrate current behavior of summarizeDataUrl/normalizeProviderRequestBodyForCacheDebug.

That said, the actual code change in cache-debug.ts is defensive against a scenario that doesn't exist today: summarizeDataUrl is only called from summarizeLargeValue after it has already verified value.startsWith('data:'). Since the function isn't exported and has exactly one call site, this check is dead code right now - it can never be hit by the current codebase, so it adds a branch nobody exercises in production, only in a purpose-built test.

If you'd found (or anticipated) a real second call site, or if the function were exported for external use, this would be worth it. As-is, it's speculative hardening rather than a bug fix - the PR description itself says 'if summarizeDataUrl were ever called from a different context,' which is a hypothetical, not an observed bug.

Two options that would make this land better:

1. Skip the guard and instead add the test coverage against the actual entry point (normalizeProviderRequestBodyForCacheDebug) to lock in existing correct behavior - that part of your tests is valuable on its own.
2. If you want the defensive check, make summarizeDataUrl exported/reusable first so the guard has a real reason to exist, and explain that use case.

Appreciate the test-first thinking; just want to see the code change justified by an actual reachable path or an exported surface.

[pavankumar-vh]

Closing this PR - the defensive check in summarizeDataUrl is dead code since the function is only called from one place that already validates the input. The maintainer feedback was correct that this is speculative hardening without a real bug being fixed.