fix(core): preserve tab-indented notebook formatting#4373
Merged
Conversation
Contributor
📋 Review SummaryThis PR fixes tab-indented notebook JSON preservation in 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
yiliang114
approved these changes
May 21, 2026
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — clean, minimal fix with solid test coverage.
The regex widening from /\n( +)"/ to /\n([ \t]+)"/ correctly picks up tab and mixed-whitespace indentation, and the conditional logic to return a string vs. number aligns well with JSON.stringify's space parameter. Space-only behavior stays backward-compatible.
The three new NotebookEditTool error-path tests (TARGET_IS_DIRECTORY, FILE_CHANGED_SINCE_READ, PRIOR_READ_VERIFICATION_FAILED) are a nice addition — good to have those edge cases pinned down.
yiliang114
approved these changes
May 21, 2026
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — the regex widening from /\n( +)"/ to /\n([ \t]+)"/ is a clean, minimal fix. Space-only behavior stays backward-compatible, and the conditional number | string return aligns nicely with JSON.stringify's space parameter. Test coverage for tab, mixed-whitespace, and the three error-path edge cases is solid. CI ubuntu failure is a pre-existing flaky issue (reproduces on main), unrelated to this change.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
yiliang114
added a commit
that referenced
this pull request
May 21, 2026
CI dry-run of a DEEP review (PR #4373) showed the posted comment would sandwich the real review between ~17 orchestrator-narration segments ("Launching 6 agents", "Let me compile the review", "All agents unanimous"). The bundled multi-agent skill streams narration throughout; only one large assistant segment is the actual consolidated review. buildOutput now picks the largest segment for DEEP (single-shot LIGHT/STANDARD still join all segments, since their whole output is the review). The header gains an `emitted` field recording how many segments actually reached the comment.
Collaborator
|
LGTM. No critical issues found. |
Collaborator
|
Reverse audit: No issues found. The review loop terminates. Now presenting the complete review. Review: PR #4373 — fix(core): preserve tab-indented notebook formattingSummaryThis PR widens 6 review agents + 1 reverse audit round found 0 confirmed issues. All 4 initial findings were rejected as pedantic nitpicks on verification. FindingsNo confirmed findings. Needs Human ReviewNo low-confidence findings. Validation EvidenceMISSING — The PR description lists test commands but provides no reviewer-facing validation evidence (no command transcripts, before/after output, logs, JSON traces, screenshots, or GIFs demonstrating the fix in action). For a user-visible formatting fix, a before/after example of
VerdictApprove — No high-confidence critical issues. Good to merge. Tip: type Now saving the review report and cache Reviewed by |
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
4 participants
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.
Summary
.ipynbJSON formatting whennotebook_editserializes edited notebooks.Design
inferNotebookJsonFormatnow detects leading tabs as well as spaces in pretty-printed notebook JSON. Space-only indentation still returns a numeric indent to preserve the existing behavior, while tab or mixed whitespace indentation returns the matched whitespace string, whichJSON.stringifyaccepts as itsspaceargument.The prior-read behavior is unchanged. The new
NotebookEditTooltests pin the existing safeguards so review feedback around those edge cases is covered without expanding the tool surface.Testing
npm test --workspace=packages/core -- --run src/utils/notebook.test.ts src/tools/notebook-edit.test.tsnpm run build --workspace=packages/corenpm run typecheck --workspace=packages/corenpm run lint --workspace=packages/coreReview focus / test areas
Follow-up to #3900.