fix(agents-core): normalize instructions#2981
Conversation
🦋 Changeset detectedLatest commit: 3de60d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
TL;DR — Works around a Doltgres JSON parser bug that rejects backslash-containing strings by providing a drop-in Key changes
Summary ｜ 6 files ｜ 9 commits ｜ base: Doltgres-safe JSONB backslash encoding
The implementation subclasses both
Simplified server instruction passthrough
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-scoped fix. The normalizeServerInstructions extraction is a good refactor — consolidates null-byte stripping with the new backslash-newline normalization, and the regex correctly handles , \r , and \r variants. Test accurately constructs the problematic input via string concatenation and verifies both the return value and the persisted payload. No issues found.
Claude Opus ｜ 𝕏
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
dbResultToMcpTool.test.ts:443Additional test coverage for null byte removal and CRLF variants
✅ APPROVE
Summary: Clean, well-implemented fix that properly normalizes MCP server instructions. The new normalizeServerInstructions() function correctly handles both null byte removal (pre-existing) and the new backslash-newline conversion. The test coverage adequately demonstrates the new functionality works. One optional suggestion for additional test coverage is provided as a "Consider" item.
Note: The changeset bot flagged this PR as missing a changeset. Since this is a bug fix in agents-core that affects how server instructions are stored/displayed, a patch changeset may be appropriate per AGENTS.md guidelines.
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-tests |
3 | 0 | 1 | 0 | 1 | 0 | 2 |
| Total | 3 | 0 | 1 | 0 | 1 | 0 | 2 |
Note: The tests reviewer identified test coverage gaps. The null byte test gap is pre-existing (was not tested before this PR either). The CRLF/CR variants are adequately demonstrated by the existing LF test pattern.
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
dbResultToMcpTool.test.ts:400-444 |
Missing tests for CRLF/CR newline variants | The regex pattern is straightforward and the existing LF test demonstrates it works. Additional tests would be redundant. |
dbResultToMcpTool.test.ts:400-444 |
Missing explicit undefined test | Already implicitly covered by existing tests that set mockMcpGetInstructions.mockReturnValue(undefined). |
| }, | ||
| }) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
💭 Consider Additional test coverage for edge cases
Issue: The test suite covers backslash-LF (\n) conversion but doesn't explicitly test null byte removal or other newline variants (CRLF, CR).
Why: The normalizeServerInstructions() function handles null bytes (pre-existing behavior) and three newline formats. While the current test demonstrates the pattern works, additional tests would make the contract clearer and catch regressions if the regex changes.
Fix: Consider adding a test for null byte removal:
it('removes null bytes from serverInstructions', async () => {
const instructionsWithNullBytes = 'Some text\x00with\x00null bytes';
const sanitizedInstructions = 'Some textwithnull bytes';
mockMcpGetInstructions.mockReturnValue(instructionsWithNullBytes);
// ... same db mock setup ...
const result = await dbResultToMcpTool(makeGenericToolDbResult(), db);
expect(result.capabilities?.serverInstructions).toBe(sanitizedInstructions);
});This is optional — the current coverage is adequate for the new functionality.
Preview URLsUse these stable preview aliases for testing this PR:
These point to the same Vercel preview deployment as the bot comment, but they stay stable and easier to find. Raw Vercel deployment URLs
|
…cannot serialize certain json
There was a problem hiding this comment.
PR Review Summary
(4) Total Issues | Risk: Medium
🟠⚠️ Major (2) 🟠⚠️
🟠 1) system Global prototype patch affects both manage and runtime databases
Issue: The prototype patch in dolt-safe-jsonb.ts modifies PgJsonb.prototype.mapToDriverValue and mapFromDriverValue globally. While the workaround is intended only for Doltgres (manage database), it affects all JSONB columns across both databases — including ~30+ JSONB columns in the runtime Postgres database.
Why: This introduces a hidden data transformation layer that: (1) applies unnecessary encode/decode cycles to runtime data that doesn't need them, (2) creates a collision risk if user data contains U+E000, and (3) establishes a precedent for global ORM patches to work around database-specific bugs. The runtime database stores arbitrary user-generated content (e.g., messages.content, artifacts.value) where exotic Unicode is more plausible than in configuration data.
Fix: Consider scoping the workaround to only the manage database:
- Option A (Recommended): Use Drizzle's
customType()to create adoltSafeJsonbcolumn type used only inmanage-schema.ts, leavingruntime-schema.tsusing standardjsonb(). This requires updating ~40 JSONB column definitions in manage-schema but keeps the fix explicit and contained. - Option B: Accept the global patch but document the cross-database impact explicitly and add observability for U+E000 collisions.
Refs:
🟠 2) dolt-safe-jsonb.ts Side-effect import with undocumented ordering requirements
Issue: The prototype patch is applied as a side effect when dolt-safe-jsonb.ts is imported via import './dolt-safe-jsonb' in manage-client.ts. This creates implicit ordering dependencies.
Why:
- Any code that directly imports from
drizzle-orm/pg-coreand usesPgJsonbbefore the side-effect import runs will get unpatched behavior - The test setup (
__tests__/setup.ts) imports fromtest-manage-client.tswhich does NOT importmanage-client.ts, so the patch is not applied in tests — tests operate with different JSONB serialization behavior than production
Fix:
- Import
./dolt-safe-jsonbin the test setup file to ensure test/production parity - Add a guard and explicit documentation about the import order requirement
- Consider adding a runtime check that warns if the patch hasn't been applied when manage database operations are attempted
Refs:
Inline Comments:
- 🟠 Major:
tools.ts:332Null-byte stripping removed without clear justification - 🟠 Major:
dolt-safe-jsonb.test.ts:3Missing test for the actual prototype patch
💭 Consider (2) 💭
💭 1) dolt-safe-jsonb.ts:20-22 U+E000 collision is documented but not guarded
Issue: The comment acknowledges that U+E000 in user data will be "round-tripped as a backslash", which is silent data corruption.
Why: While U+E000 is in the Private Use Area and "essentially never used", this is a silent failure mode with no detection or warning.
Fix: Consider adding runtime detection (logging a warning) when U+E000 is encountered in data being decoded. This provides observability for the edge case without breaking functionality.
💭 2) dolt-safe-jsonb.test.ts:28 No test for object keys containing backslashes
Issue: The encodeBackslashes function uses Object.entries(value) which iterates over keys but does not encode keys themselves, only values. This appears intentional but is undocumented.
Fix: Add a test documenting this design decision:
it('does not encode backslashes in object keys (values only)', () => {
const input = { 'key\\with\\backslash': 'value\\here' };
const result = encodeBackslashes(input);
expect(Object.keys(result as object)[0]).toBe('key\\with\\backslash');
expect((result as any)['key\\with\\backslash']).toBe('value\uE000here');
});🚫 REQUEST CHANGES
Summary: The new global PgJsonb prototype patch approach is clever and solves the Doltgres backslash parsing bug, but it introduces significant architectural concerns: (1) it affects both manage and runtime databases when only manage needs the workaround, (2) the test setup doesn't apply the patch creating production/test divergence, and (3) the null-byte stripping from the previous commit was dropped without explanation. Please clarify whether the global scope and null-byte removal are intentional, and consider scoping the fix to only the manage database.
Note: The changeset bot flagged this PR as missing a changeset. Since this is a bug fix in agents-core that affects how JSONB data is stored, a patch changeset is appropriate per AGENTS.md guidelines.
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
dolt-safe-jsonb.test.ts:21 |
No test for Date/RegExp/Map/Set handling | These types are not JSON-serializable and would fail at the JSON.stringify level before reaching this code. Edge case unlikely in practice. |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
1 | 0 | 0 | 0 | 1 | 0 | 0 |
pr-review-architecture |
3 | 2 | 1 | 0 | 0 | 0 | 0 |
pr-review-tests |
3 | 0 | 1 | 0 | 1 | 0 | 1 |
| Total | 7 | 2 | 2 | 0 | 2 | 0 | 1 |
| const rawServerInstructions = client.getInstructions(); | ||
| // biome-ignore lint/suspicious/noControlCharactersInRegex: Intentionally matching control chars to remove them | ||
| const serverInstructions = rawServerInstructions?.replace(/\u0000/g, ''); | ||
| const serverInstructions = client.getInstructions() ?? undefined; |
There was a problem hiding this comment.
🟠 MAJOR Null-byte stripping removed
Issue: The previous commit added normalizeServerInstructions() which stripped null bytes via value.replaceAll(String.fromCharCode(0), ''). This refactoring removes that handling entirely.
Why: The original code had an explicit biome-ignore comment stating "Intentionally matching control chars to remove them" — suggesting null bytes from MCP servers were a known issue. If MCP servers can return null bytes in instructions, they will now pass through unfiltered to the database. This may cause issues depending on how Doltgres handles null bytes in JSON strings.
Fix: Verify whether null-byte stripping is still needed:
- If the JSONB patch handles this → add a test verifying null bytes work correctly
- If null bytes are still problematic → restore stripping, either inline or in
encodeBackslashes() - If no longer needed → add a brief comment explaining why the removal is safe
Refs:
| @@ -0,0 +1,102 @@ | |||
| import { describe, expect, it } from 'vitest'; | |||
| import { decodeBackslashes, encodeBackslashes } from '../../db/manage/dolt-safe-jsonb'; | |||
|
|
|||
There was a problem hiding this comment.
🟠 MAJOR Missing test for the actual prototype patch
Issue: Tests cover the pure functions encodeBackslashes and decodeBackslashes, but don't verify that the PgJsonb.prototype patch (lines 53-62 of dolt-safe-jsonb.ts) works correctly.
Why: The prototype patching is the critical integration point that makes this fix work in production. Without a test, a regression in the patch (e.g., accidentally removing the import, incorrect this binding, wrong function call order) would go undetected while unit tests still pass.
Fix: Add a test that verifies the prototype patch:
import { PgJsonb } from 'drizzle-orm/pg-core';
import '../../db/manage/dolt-safe-jsonb'; // Apply the patch
describe('PgJsonb prototype patch', () => {
it('encodes backslashes when serializing to driver', () => {
const jsonb = new PgJsonb('test');
const result = jsonb.mapToDriverValue({ path: 'C:\\Users' });
expect(result).not.toContain('\\\\'); // No double-backslashes
expect(result).toContain('\uE000');
});
it('decodes backslashes when reading from driver', () => {
const jsonb = new PgJsonb('test');
const result = jsonb.mapFromDriverValue({ path: 'C\uE000Users' });
expect(result).toEqual({ path: 'C:\\Users' });
});
});Refs:
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta review scoped to changes since the previous review (commit 80a165fea780).
Prior Issues Resolution
All major issues from the previous review have been addressed:
| Prior Issue | Resolution |
|---|---|
| 🟠 Global prototype patch affects both databases | ✅ Fixed — Changed from PgJsonb.prototype modification to scoped DoltSafeJsonb class |
| 🟠 Side-effect import with ordering requirements | ✅ Fixed — Removed import './dolt-safe-jsonb' from manage-client.ts |
| 🟠 Null-byte stripping removed | ✅ Clarified — Null-byte handling was unrelated to the Doltgres backslash bug; removal is intentional |
| 🟠 Missing test for prototype patch | ✅ N/A — No longer using prototype patching; existing unit tests cover the encode/decode functions |
Architecture Verification
The delta correctly scopes the Doltgres workaround:
| Schema | Import Source | Effect |
|---|---|---|
manage-schema.ts (Doltgres) |
./dolt-safe-jsonb |
✅ Backslash encoding enabled |
runtime-schema.ts (Postgres) |
drizzle-orm/pg-core |
✅ Standard behavior, unaffected |
The class-based approach (DoltSafeJsonb extends PgJsonb) is cleaner than prototype patching:
- Explicit opt-in via import
- No global side effects
- No ordering dependencies
- Type-safe extension pattern (with documented
@ts-expect-errorfor internal Drizzle API)
Notes
Fragility risk (acknowledged, acceptable): The implementation extends Drizzle's internal PgJsonbBuilder.build() method not in public types. Future Drizzle updates could break this. The existing test coverage (dolt-safe-jsonb.test.ts) will catch regressions.
Changeset reminder: The changeset bot flagged this PR. A patch changeset for @inkeep/agents-core is appropriate per AGENTS.md guidelines.
✅ APPROVE
Summary: The architectural concerns from the previous review have been thoroughly addressed. The new class-based approach properly scopes the Doltgres workaround to only the manage database, eliminating the global prototype patching and side-effect import issues. Clean fix. 🎉
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-architecture |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Delta scope — reviewers verified prior issues are resolved; no new issues found.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta review scoped to changes since the previous review (commit 52bb125faae7).
Delta Changes
| Change | Assessment |
|---|---|
✅ Added changeset (smart-points-occur.md) |
Addresses prior reminder — correctly lists @inkeep/agents-core, @inkeep/agents-manage-ui, @inkeep/agents-api as patch bumps |
✅ Added null-byte stripping to encodeBackslashes() |
Good addition — consolidates null-byte handling into the JSONB layer rather than at individual call sites |
| ✅ Added test for null-byte stripping | Proper test coverage for the new behavior |
Null-Byte Handling Note
The delta adds .replaceAll('\0', '') to encodeBackslashes() on line 23 of dolt-safe-jsonb.ts. This is a cleaner solution than the original approach that was removed from tools.ts because:
- Centralized — null bytes are stripped for all JSONB columns in the manage database, not just
serverInstructions - No biome-ignore needed — the implementation uses the literal character
'\0'instead of a regex, avoiding the linter warning - Tested — covered by the new test case on line 40-42 of
dolt-safe-jsonb.test.ts
Prior Issues Status
All issues from prior reviews remain resolved:
| Prior Issue | Status |
|---|---|
| Global prototype patch scope | ✅ Resolved — class-based approach scopes to manage DB only |
| Side-effect import ordering | ✅ Resolved — no side-effect imports |
| Null-byte stripping removed | ✅ Now addressed — added to encodeBackslashes() |
| Missing changeset | ✅ Now addressed — changeset added |
✅ APPROVE
Summary: The delta since the last review consists of two improvements: adding the missing changeset and consolidating null-byte stripping into the JSONB encoding layer. Both changes are clean and well-tested. Ready to merge. 🎉
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Delta scope — minimal changes since last approval; no sub-reviewers needed.
|
@miles-kt-inkeep it looks like it was maybe a doltgres deficiency that was causing the jsonb's to not get formatted right |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since the previous review (commit ae64639e54d6).
Delta Analysis
| Change | Assessment |
|---|---|
0e72a2c93 Merge branch 'main' into fix/normalize-tool-prompts |
✅ Merge commit only — brings in unrelated changes from main |
No changes to PR files since last review. The 6 files in this PR's scope remain unchanged:
.changeset/smart-points-occur.mdpackages/agents-core/src/__tests__/data-access/dbResultToMcpTool.test.tspackages/agents-core/src/__tests__/db/dolt-safe-jsonb.test.tspackages/agents-core/src/data-access/manage/tools.tspackages/agents-core/src/db/manage/dolt-safe-jsonb.tspackages/agents-core/src/db/manage/manage-schema.ts
Prior Review Status
All issues from prior reviews remain resolved:
| Prior Issue | Status |
|---|---|
| Global prototype patch scope | ✅ Resolved — class-based DoltSafeJsonb scopes to manage DB only |
| Side-effect import ordering | ✅ Resolved — explicit import, no side effects |
| Null-byte stripping | ✅ Resolved — consolidated in encodeBackslashes() |
| Changeset | ✅ Present — patch bump for agents-core, agents-manage-ui, agents-api |
✅ APPROVE
Summary: No changes to the PR's implementation since the last approval. The merge from main is clean and doesn't affect this fix. The Doltgres-safe JSONB workaround remains well-implemented, properly scoped to the manage database, and thoroughly tested. Ready to merge. 🎉
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Delta scope — no changes to PR files since last approval; sub-reviewers not needed.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since the previous review (commit 0e72a2c93d85).
Delta Analysis
| Change | Assessment |
|---|---|
3de60d9e4 Merge branch 'main' into fix/normalize-tool-prompts |
✅ Merge commit only — brings in version bumps from main |
No changes to PR implementation files since last review. The 6 files in this PR's scope remain unchanged:
.changeset/smart-points-occur.mdpackages/agents-core/src/__tests__/data-access/dbResultToMcpTool.test.tspackages/agents-core/src/__tests__/db/dolt-safe-jsonb.test.tspackages/agents-core/src/data-access/manage/tools.tspackages/agents-core/src/db/manage/dolt-safe-jsonb.tspackages/agents-core/src/db/manage/manage-schema.ts
Implementation Summary
The PR implements a clean workaround for a Doltgres JSON parser bug:
| Component | Implementation |
|---|---|
DoltSafeJsonb class |
Extends PgJsonb with encode/decode hooks for backslashes → U+E000 |
| Scope | Only manage-schema.ts (Doltgres) — runtime-schema.ts (Postgres) unaffected |
| Null-byte handling | Consolidated in encodeBackslashes() — stripped for all manage JSONB columns |
| Test coverage | 106 lines covering encode/decode, roundtrip, edge cases |
| Changeset | ✅ Present — patch bump for agents-core, agents-manage-ui, agents-api |
Prior Issues Status
All issues from prior reviews remain resolved:
| Prior Issue | Status |
|---|---|
| Global prototype patch scope | ✅ Resolved — class-based DoltSafeJsonb scopes to manage DB only |
| Side-effect import ordering | ✅ Resolved — explicit import, no side effects |
| Null-byte stripping | ✅ Resolved — consolidated in encodeBackslashes() |
| Changeset | ✅ Present |
✅ APPROVE
Summary: No changes to the PR's implementation since the last approval. The merge from main is clean and only brings in version bumps from the Version Packages PR. The Doltgres-safe JSONB workaround remains well-implemented, properly scoped to the manage database, and thoroughly tested. Ready to merge. 🎉
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Delta scope — no changes to PR files since last approval; sub-reviewers not needed.
Ito Test Report ✅15 test cases ran. 1 additional finding, 14 passed. Overall, 14 of 15 test cases passed, confirming stable MCP list/detail/deep-link navigation, correct prompt fallback and override precedence, reliable escape/backslash round-tripping, responsive long-content rendering, deterministic save behavior under refresh/multi-tab/rapid-submit stress, and strong security boundaries (stored XSS rendered inert and cross-project URL/API tampering denied). The key issue was one High-severity defect (LOGIC-3): after saving non-prompt MCP configuration fields, details/edit pages can become blank because the read path blocks on live MCP discovery (connect/tools) instead of promptly returning persisted data when discovery is slow or unavailable. ✅ Passed (14)
ℹ️ Additional Findings (1)
|
…JSONB data Companion to #2981 (dolt-safe-jsonb write fix). That PR encodes backslashes as U+E000 on new writes, but existing data still has raw backslashes that can trigger Doltgres JSON parser errors on queries. This script re-encodes all JSONB columns across all Doltgres branches. Three modes: --scan (read-only audit), default (dry run), --apply (write). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…JSONB data Companion to #2981 (dolt-safe-jsonb write fix). That PR encodes backslashes as U+E000 on new writes, but existing data still has raw backslashes that can trigger Doltgres JSON parser errors on queries. This script re-encodes all JSONB columns across all Doltgres branches. Three modes: --scan (read-only audit), default (dry run), --apply (write). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…B data (#3008) * fix: add migration script to encode backslashes in existing Doltgres JSONB data Companion to #2981 (dolt-safe-jsonb write fix). That PR encodes backslashes as U+E000 on new writes, but existing data still has raw backslashes that can trigger Doltgres JSON parser errors on queries. This script re-encodes all JSONB columns across all Doltgres branches. Three modes: --scan (read-only audit), default (dry run), --apply (write). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply biome formatting to migration script Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add repair script for Doltgres-corrupted JSONB rows Handles rows where Doltgres mangled backslash escape sequences so badly that the stored JSON is structurally invalid (throws "Bad escaped character" on read). Two repair strategies: - tools.capabilities → NULL (auto-rediscovered on MCP health check) - data_components.props/render → attempt JSON repair + U+E000 encoding Modes: --scan (audit), default (dry run), --apply (write fixes) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply biome formatting to repair script Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle pool idle timeout to prevent ETIMEDOUT crash on exit Add idleTimeoutMillis and swallow pool-level errors so the script exits cleanly after processing all branches on remote Doltgres. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add --dump mode to write full corrupt JSONB content to file Dumps the complete raw content of each corrupt column to a text file for review. Deduplicates across branches so each unique row appears once. Usage: node scripts/fix-doltgres-corrupt-jsonb.mjs --dump [--out file.txt] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: auto-format with biome * fix: make --dump an add-on flag that works with --scan --dump now combines with --scan (or runs alone as implicit scan): --scan --dump → scan console output + full content dump to file --dump → same (implies scan) --dump --out f → custom output path Deduplicates across branches so each unique corrupt row appears once. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: show string diff in dry-run mode for repair preview Dry run now displays a unified-style diff for each corrupt column: - NULL strategy: shows current content → NULL - Repair strategy: shows current content → repaired JSON Control chars are made visible in the diff output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: auto-format with biome * feat: colorize dry-run diff output Red badge + red text for removed lines, green badge + green text for added lines, dimmed context. Much easier to spot changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: auto-format with biome * feat: character-level diff highlighting in dry-run mode Changed characters within a line now get background-colored: - Deleted chars: white on red background - Inserted chars: black on green background - Unchanged prefix/suffix stays normal red/green Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: pretty-print both sides of dry-run diff for readable output Both the corrupt and repaired JSON are now pretty-printed with matching structure before diffing. This means only the actual content changes (backslash escaping fixes) show up as diff lines, not JSON formatting noise from compact vs pretty-printed serialization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: replace line-level diff with focused inline change hunks Instead of showing huge single-line JSON diffs, the dry-run now: - Finds each individual character-level change - Shows it as a numbered hunk with 50 chars of surrounding context - Highlights the exact deleted/inserted chars with background color Example output: 2 change(s): Change 1 at position 7253: - ...${country.country}\[HIGHLIGHTED:\n]Happiness... + ...${country.country}\[HIGHLIGHTED:\\n]Happiness... Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: show literal control chars as ␊ ␉ ␍ to distinguish from \n \t \r Before: both sides showed \\n — impossible to see the difference. Now: corrupt side shows ␊ (literal newline), repaired side shows \\n (escape sequence). The actual change is immediately obvious. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add comprehensive run instructions to both migration scripts Both scripts now include full background context, prerequisites, step-by-step example commands, and flag reference in their headers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove unused backslash encoding script Production scan found 0 rows needing encoding. The actual problem (corrupt JSON) is handled by fix-doltgres-corrupt-jsonb.mjs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
No description provided.