From a25b8e8b9c3b09f3bffc0a5b2f177348e93764f3 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 25 Jul 2026 19:03:49 +0700 Subject: [PATCH 01/11] fix: mark truncated diff/stream output so it never reads as complete --- cmd/cli/cmd_diff.go | 24 ++- cmd/cli/cmd_stream.go | 90 ++++++-- .../components/DiffView.truncation.test.tsx | 105 ++++++++++ frontend/src/components/DiffView.tsx | 18 +- internal/pdfcore/diff.go | 83 +++++++- internal/pdfcore/diff_test.go | 140 +++++++++++++ internal/pdfcore/model.go | 20 +- internal/pdfcore/stream.go | 59 ++++++ internal/pdfcore/stream_test.go | 117 +++++++++++ testdata/correctness/README.md | 51 +++++ testdata/correctness/deep-change-a.pdf | Bin 0 -> 2670 bytes testdata/correctness/deep-change-b.pdf | Bin 0 -> 2670 bytes testdata/correctness/multi-content-stream.pdf | Bin 0 -> 564 bytes .../diff_truncation_test.go | 98 +++++++++ tests/14-3-no-silent-truncation/go.mod | 3 + .../14-3-no-silent-truncation/helpers_test.go | 197 ++++++++++++++++++ .../multistream_test.go | 136 ++++++++++++ 17 files changed, 1112 insertions(+), 29 deletions(-) create mode 100644 frontend/src/components/DiffView.truncation.test.tsx create mode 100644 testdata/correctness/deep-change-a.pdf create mode 100644 testdata/correctness/deep-change-b.pdf create mode 100644 testdata/correctness/multi-content-stream.pdf create mode 100644 tests/14-3-no-silent-truncation/diff_truncation_test.go create mode 100644 tests/14-3-no-silent-truncation/go.mod create mode 100644 tests/14-3-no-silent-truncation/helpers_test.go create mode 100644 tests/14-3-no-silent-truncation/multistream_test.go diff --git a/cmd/cli/cmd_diff.go b/cmd/cli/cmd_diff.go index decadab..7b144fd 100644 --- a/cmd/cli/cmd_diff.go +++ b/cmd/cli/cmd_diff.go @@ -95,8 +95,12 @@ func execDiff(leftPath, rightPath string, jsonOut, pretty, full bool) (exitCode // the counts and must be checked explicitly or a /Producer-only or // encryption-only change would wrongly report "identical" (exit 0). func diffIsIdentical(s pdfcore.DiffSummary) bool { + // A depth-capped walk (TruncatedSubtrees > 0) left a subtree unexplored, so + // the pair cannot be certified identical even with zero visible deltas + // (Story 14.3 AC2): exit 1 ("not provably identical") is the honest signal. return s.Added == 0 && s.Removed == 0 && s.Changed == 0 && - !s.VersionChanged && !s.EncryptionChanged && !s.InfoChanged && !s.XMPChanged + !s.VersionChanged && !s.EncryptionChanged && !s.InfoChanged && !s.XMPChanged && + s.TruncatedSubtrees == 0 } // printDiffPlain renders the summary plus a path-indented delta. NON-CONTRACTUAL @@ -119,6 +123,11 @@ func printDiffPlain(out io.Writer, res *pdfcore.DiffResult, full bool) error { if s.XMPChanged { b.WriteString("XMP metadata changed\n") } + if s.TruncatedSubtrees > 0 { + // Depth-cap honesty (Story 14.3 AC2): state plainly that the walk was + // bounded so no consumer mistakes it for a complete comparison. + fmt.Fprintf(&b, "%d subtree(s) compared only to the depth cap (truncated); deeper differences cannot be ruled out\n", s.TruncatedSubtrees) + } b.WriteString("\n") if diffIsIdentical(s) { @@ -158,15 +167,24 @@ func writeDiffLines(b *strings.Builder, n *pdfcore.DiffNode, depth int, full boo fmt.Fprintf(b, " %s", n.LeftSummary) } } + if n.Truncated { + // The depth-cap tag (Story 14.3 AC1): a truncated node carries + // Status "unchanged", so without this it would print without any + // indication its subtree was left unwalked. + b.WriteString(" [truncated: depth cap]") + } b.WriteString("\n") for _, c := range n.Children { writeDiffLines(b, c, depth+1, full) } } -// diffNodeHasDelta reports whether n or any descendant is not "unchanged". +// diffNodeHasDelta reports whether n or any descendant is not "unchanged", or +// is depth-cap truncated. A truncated node reports Status "unchanged" but must +// surface in the default (non-full) delta so its [truncated: depth cap] tag is +// visible (Story 14.3 AC1). func diffNodeHasDelta(n *pdfcore.DiffNode) bool { - if n.Status != "unchanged" { + if n.Status != "unchanged" || n.Truncated { return true } for _, c := range n.Children { diff --git a/cmd/cli/cmd_stream.go b/cmd/cli/cmd_stream.go index 115143b..4db6b7b 100644 --- a/cmd/cli/cmd_stream.go +++ b/cmd/cli/cmd_stream.go @@ -138,8 +138,9 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { defer func() { _ = inspector.Close("cli") }() // Resolve the content-stream nodeID and, for --ops, the page number whose - // resources back Do classification (0 = no page-backed Do resolution). - nodeID, opsPageNum, code := resolveStreamNode(inspector, info, flags) + // resources back Do classification (0 = no page-backed Do resolution), plus + // the /Contents array length for the multi-stream truncation marker. + nodeID, opsPageNum, streamCount, code := resolveStreamNode(inspector, info, flags) if code != 0 { return code } @@ -180,6 +181,18 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { return 2 } + // Multi-stream truncation marker (Story 14.3 AC3/AC4, floor path): when the + // page's /Contents array holds more than one stream, only the first was + // decoded, so mark the result partial. A single stream (streamCount <= 1) is + // complete and carries no marker. --raw stays a verbatim byte dump of the one + // decoded stream (a marker would corrupt the bytes); the note rides plain + // text, --json, and --ops instead. + if streamCount > 1 { + result.StreamCount = streamCount + result.Shown = 1 + result.Truncated = true + } + if flags.raw { if _, err := io.WriteString(os.Stdout, result.Raw); err != nil { fmt.Fprintf(os.Stderr, "failed to write raw output: %v\n", err) @@ -212,14 +225,24 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { // PDF content-stream order (let it flow; do not tabulate). NON-CONTRACTUAL; // use --json for structured operators, --ops for NDJSON, --raw for bytes. func printStreamPlain(out io.Writer, result *pdfcore.ContentStreamData) error { + var b strings.Builder + // Multi-stream truncation note (Story 14.3 AC3, floor path): a one-line + // header so the human reader is not shown a partial (often unbalanced) + // program as if it were the whole content stream. Emitted BEFORE the + // empty-stream early return: a multi-stream page whose first stream decodes + // to zero operators must still disclose that streams 2..N exist, otherwise + // the "(empty content stream)" line would be a silent truncation. + if result.Truncated { + fmt.Fprintf(&b, "(truncated: showing stream %d of %d; page /Contents is a multi-stream array)\n", result.Shown, result.StreamCount) + } // A content-stream object can exist yet decode to zero operators (an empty // /Contents stream). Surface that as a one-line note so plain output is never // a silent zero-byte write and always ends with a newline. if len(result.Formatted) == 0 { - _, err := io.WriteString(out, "(empty content stream)\n") + b.WriteString("(empty content stream)\n") + _, err := io.WriteString(out, b.String()) return err } - var b strings.Builder for _, fl := range result.Formatted { for range fl.Indent { b.WriteString(" ") @@ -240,53 +263,62 @@ func printStreamPlain(out io.Writer, result *pdfcore.ContentStreamData) error { // resolveStreamNode maps the input flags to a content-stream nodeID. It returns // the nodeID (or "" when a page has no /Contents), the page number to back Do -// classification under --ops (0 when not a page stream), and a non-zero exit -// code on error (already reported to stderr). -func resolveStreamNode(inspector *pdfcore.Inspector, info *pdfcore.DocumentInfo, flags streamFlags) (nodeID string, opsPageNum int, code int) { +// classification under --ops (0 when not a page stream), the number of streams +// in the page's /Contents array (Story 14.3 multi-stream marker; 0 for the +// --ref/--xobject modes and single-stream pages that need no marker), and a +// non-zero exit code on error (already reported to stderr). +func resolveStreamNode(inspector *pdfcore.Inspector, info *pdfcore.DocumentInfo, flags streamFlags) (nodeID string, opsPageNum int, streamCount int, code int) { switch { case flags.xobject != "": ownerNodeID, c := xobjectOwnerNodeID(inspector, info, flags) if c != 0 { - return "", 0, c + return "", 0, 0, c } resources, err := inspector.GetXObjectResources("cli", ownerNodeID) if err != nil { writeJSONError(os.Stderr, err.Error()) - return "", 0, 2 + return "", 0, 0, 2 } entry, ok := resources[flags.xobject] if !ok || entry.NodeID == "" { writeJSONError(os.Stderr, fmt.Sprintf("XObject %q not found in resources", flags.xobject)) - return "", 0, 2 + return "", 0, 0, 2 } // A resolved form/image XObject stream has no page; Do classification is // page-scoped only (Decision: --ops resourceType only for page streams). - return entry.NodeID, 0, 0 + return entry.NodeID, 0, 0, 0 case flags.ref != "": objNum, genNum, err := parseObjectRef(flags.ref) if err != nil { writeJSONError(os.Stderr, err.Error()) - return "", 0, 1 + return "", 0, 0, 1 } - return fmt.Sprintf("obj:%d:%d", genNum, objNum), 0, 0 + return fmt.Sprintf("obj:%d:%d", genNum, objNum), 0, 0, 0 default: // Page content stream. if info.PageCount == 0 { writeJSONError(os.Stderr, "cannot determine page count for this PDF") - return "", 0, 2 + return "", 0, 0, 2 } if flags.page > info.PageCount { writeJSONError(os.Stderr, fmt.Sprintf("page %d out of range: document has %d pages", flags.page, info.PageCount)) - return "", 0, 2 + return "", 0, 0, 2 } id, err := inspector.GetPageContentStreamNodeID("cli", flags.page) if err != nil { writeJSONError(os.Stderr, err.Error()) - return "", 0, 2 + return "", 0, 0, 2 + } + // Surface the /Contents array length so a multi-stream page can be marked + // as truncated (only the first stream is decoded on the floor path). + count, err := inspector.GetPageContentStreamCount("cli", flags.page) + if err != nil { + writeJSONError(os.Stderr, err.Error()) + return "", 0, 0, 2 } - return id, flags.page, 0 + return id, flags.page, count, 0 } } @@ -379,9 +411,33 @@ func emitOps(inspector *pdfcore.Inspector, result *pdfcore.ContentStreamData, pa return 2 } } + + // Multi-stream truncation marker (Story 14.3 AC4): NDJSON has no envelope + // and Story 14-1 pins --ops to one JSON object PER OPERATOR, so the marker + // rides a DISTINCT trailing meta record with NO "op" key (a phantom + // {"op":""} record would breach that contract). It is emitted only for the + // floor path (a genuinely multi-stream page); the operator records above are + // stream 1's alone. + if result.Truncated { + meta := opsTruncationMeta{Truncated: true, StreamCount: result.StreamCount, Shown: result.Shown} + if err := enc.Encode(meta); err != nil { + fmt.Fprintf(os.Stderr, "failed to write NDJSON output: %v\n", err) + return 2 + } + } return 0 } +// opsTruncationMeta is the trailing --ops NDJSON meta record for a multi-stream +// page (Story 14.3 AC4). It deliberately has NO Op field so consumers keying on +// "op" skip it as a non-operator record, and it carries the /Contents array +// length so a script sees that only Shown of StreamCount streams were emitted. +type opsTruncationMeta struct { + Truncated bool `json:"truncated"` + StreamCount int `json:"streamCount"` + Shown int `json:"shown"` +} + // classifyDo attaches resourceType + objectRef to a Do op when its name operand // resolves to a page XObject whose /Subtype is /Image or /Form. A name that // does not resolve, or whose /Subtype is neither, leaves the op unannotated diff --git a/frontend/src/components/DiffView.truncation.test.tsx b/frontend/src/components/DiffView.truncation.test.tsx new file mode 100644 index 0000000..76dfdba --- /dev/null +++ b/frontend/src/components/DiffView.truncation.test.tsx @@ -0,0 +1,105 @@ +/** + * Story 14.3: DiffView depth-cap truncation display branch (AC5, 14.3-COMP-001). + * + * RED PHASE: DiffView's `identical` const (DiffView.tsx) mirrors Go's + * diffIsIdentical over the node counts + document flags only; it does not yet + * account for `summary.truncatedSubtrees`. Given a result whose walk was bounded + * by the depth cap (truncatedSubtrees > 0) but whose visible node counts are all + * zero, the component today computes identical === true and renders the + * "No structural differences" banner with NO truncation marker -- the exact + * quiet lie this story closes, mirrored on the GUI surface. + * + * GREEN target: `identical` gains `&& s.truncatedSubtrees === 0`, so the banner + * is suppressed, and a depth-cap marker is rendered so the bounded walk is + * visible. This is the thin display branch of a backend-verified field, kept at + * the component level (NOT E2E). + * + * Test files are excluded from the app tsc build, so the `truncatedSubtrees` + * field (not yet on DiffSummaryData) does not break `npm run typecheck`; only + * vitest exercises this file. + * + * Naming: 14.3-COMP-001 [P1]. + * Run: cd frontend && npx vitest run src/components/DiffView.truncation.test.tsx + */ +import { render, screen, waitFor } from '@testing-library/react'; +import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { DiffView } from './DiffView'; + +const mockDiffDocuments = vi.fn(); +vi.mock( + '../../bindings/unidoc-pdf-debugger/internal/pdfservice/pdfservice.js', + () => ({ + DiffDocuments: (...a: unknown[]) => mockDiffDocuments(...a), + }) +); + +/** + * A diff whose visible node counts are all zero but whose walk was bounded by + * the depth cap (truncatedSubtrees > 0). Under the bug this reports identical; + * post-fix it must NOT. The single cut node carries `truncated: true`. + */ +const depthCappedResult = { + summary: { + added: 0, + removed: 0, + changed: 0, + pageCountLeft: 1, + pageCountRight: 1, + versionChanged: false, + encryptionChanged: false, + infoChanged: false, + xmpChanged: false, + // Additive field surfaced by the Go DiffSummary (AC2). Cast through unknown + // because DiffSummaryData does not declare it yet (red-phase seam). + truncatedSubtrees: 1, + }, + root: { + path: '/Root', + status: 'unchanged', + kind: 'dict', + changedKeys: [] as string[], + leftSummary: '', + rightSummary: '', + children: [ + { + path: '/Root/Deep', + status: 'unchanged', + kind: 'ref', + changedKeys: [] as string[], + leftSummary: '<< /L >>', + rightSummary: '<< /L >>', + truncated: true, + children: [], + }, + ], + }, +}; + +beforeEach(() => { + vi.clearAllMocks(); + mockDiffDocuments.mockResolvedValue(depthCappedResult); +}); + +describe('DiffView depth-cap truncation (Story 14.3)', () => { + // 14.3-COMP-001 [P1] AC5: a result with truncatedSubtrees > 0 must NOT render + // the "No structural differences / identical" banner -- the walk was bounded, + // so identity cannot be claimed. + test('14.3-COMP-001 suppresses the identical banner when a subtree was depth-capped', async () => { + render(); + + const summary = await screen.findByTestId('diff-summary'); + const text = (summary.textContent ?? '').toLowerCase(); + expect(text).not.toMatch(/no structural differences|no differ|identical/); + }); + + // 14.3-COMP-001 [P1] AC5: the depth-cap marker is rendered somewhere in the + // view so the bounded walk is visible to the user (mirrors the CLI marker). + test('14.3-COMP-001 renders a depth-cap truncation marker', async () => { + const { container } = render(); + + await waitFor(() => expect(mockDiffDocuments).toHaveBeenCalled()); + await screen.findByTestId('diff-summary'); + const body = (container.textContent ?? '').toLowerCase(); + expect(body).toMatch(/truncat|depth cap/); + }); +}); diff --git a/frontend/src/components/DiffView.tsx b/frontend/src/components/DiffView.tsx index 2d91490..1490297 100644 --- a/frontend/src/components/DiffView.tsx +++ b/frontend/src/components/DiffView.tsx @@ -20,6 +20,8 @@ export interface DiffNodeData { leftSummary: string; rightSummary: string; children?: DiffNodeData[]; + /** True when this ref was left unwalked at the depth cap (Story 14.3 AC1). */ + truncated?: boolean; } /** The document-level tally, mirroring `pdfcore.DiffSummary`. */ @@ -33,6 +35,9 @@ export interface DiffSummaryData { encryptionChanged: boolean; infoChanged: boolean; xmpChanged: boolean; + /** Count of subtrees compared only to the depth cap (Story 14.3 AC2); when + * > 0 the walk was bounded and identity cannot be claimed. */ + truncatedSubtrees: number; } /** The diff outcome, mirroring `pdfcore.DiffResult`. */ @@ -238,6 +243,9 @@ export function DiffView({ leftTabId, rightTabId, active }: DiffViewProps) { // Mirror Go's diffIsIdentical: node counts alone miss document-level deltas // (encryption, /Version, /Info, XMP) that live off the catalog walk, so a // flags-only change must not report "No structural differences". + // A depth-capped walk (truncatedSubtrees > 0) left a subtree unexplored, so + // identity cannot be claimed even with zero visible deltas (Story 14.3 AC2/ + // 14.3-COMP-001), mirroring Go's diffIsIdentical. const identical = s.added === 0 && s.removed === 0 && @@ -245,7 +253,8 @@ export function DiffView({ leftTabId, rightTabId, active }: DiffViewProps) { !s.versionChanged && !s.encryptionChanged && !s.infoChanged && - !s.xmpChanged; + !s.xmpChanged && + s.truncatedSubtrees === 0; return (
@@ -264,6 +273,12 @@ export function DiffView({ leftTabId, rightTabId, active }: DiffViewProps) { {s.infoChanged ? ' | /Info changed' : ''} {s.xmpChanged ? ' | XMP changed' : ''}
+ {s.truncatedSubtrees > 0 && ( +
+ {s.truncatedSubtrees} subtree{s.truncatedSubtrees === 1 ? '' : 's'} truncated at the depth + cap; deeper differences cannot be ruled out. +
+ )}
@@ -329,6 +344,7 @@ export function DiffView({ leftTabId, rightTabId, active }: DiffViewProps) { {diffMarker(node.status)} {node.path} {node.leftSummary ? {node.leftSummary} : null} + {node.truncated ? [truncated: depth cap] : null}
); })} diff --git a/internal/pdfcore/diff.go b/internal/pdfcore/diff.go index f18937f..d3491e5 100644 --- a/internal/pdfcore/diff.go +++ b/internal/pdfcore/diff.go @@ -55,6 +55,19 @@ type DiffNode struct { // deterministic (sorted-key / array-index) order. Nil for scalar leaves, // single-sided (added/removed) nodes, and depth-capped refs. Children []*DiffNode `json:"children,omitempty"` + // Truncated is true ONLY when this node is a ref left unwalked at the + // maxResolveDepth depth cap and compared by shallow summary (Story 14.3 + // AC1). It is NOT set for back-edge (cycle) cuts or the visitedPairs + // cross-path dedup, both of which hide nothing (the target is fully + // accounted for elsewhere). A truncated node's shallow summaries can match + // while a deeper difference is hidden, so the run must not be called + // identical; see DiffSummary.TruncatedSubtrees. + Truncated bool `json:"truncated,omitempty"` + // capRefPair is the "numL:genL|numR:genR" key of a depth-capped ref pair, + // recorded so reconcileTruncation can clear Truncated when the SAME pair is + // fully walked on another (shallower) path. Unexported: internal to the + // walk, never marshaled across the IPC boundary. + capRefPair string } // DiffSummary is the document-level "what changed at a glance" tally. The @@ -76,6 +89,13 @@ type DiffSummary struct { EncryptionChanged bool `json:"encryptionChanged"` // trailer /Encrypt presence InfoChanged bool `json:"infoChanged"` // /Info dictionary fields XMPChanged bool `json:"xmpChanged"` // catalog /Metadata XMP packet + // TruncatedSubtrees counts the nodes cut at the maxResolveDepth depth cap + // (DiffNode.Truncated) - subtrees compared only by shallow summary, whose + // deeper contents were not walked (Story 14.3 AC2). When > 0 the walk was + // bounded, so the pair CANNOT be certified identical: the CLI withholds + // exit 0 and the "structurally identical" claim. Not omitempty so the + // honest count is always present in the JSON contract. + TruncatedSubtrees int `json:"truncatedSubtrees"` } // diffContext carries the two documents through the recursive walk so the @@ -170,6 +190,12 @@ func (ins *Inspector) DiffDocuments(leftTabID, rightTabID string) (*DiffResult, return nil, wrapPDFError(err) } + // Clear depth-cap marks on pairs that were fully walked elsewhere before + // tallying, so a shared object reached both shallow and past the cap is not + // counted as truncated (would falsely withhold "identical"). Runs once with + // the complete visitedPairs set, so it is DFS-order independent. + reconcileTruncation(root, dc.visitedPairs) + summary := DiffSummary{} countDelta(root, &summary) dc.fillSummary(&summary, leftCat, rightCat) @@ -331,11 +357,33 @@ func (dc *diffContext) diffChild(path string, lv, rv pdfcpu_types.Object, depth lres := dereferenceIfRef(dc.left, lv) rres := dereferenceIfRef(dc.right, rv) - // Back-edge or depth cap: do not recurse; compare the resolved values by - // shallow summary. This terminates deep/cyclic graphs. - if leftCycle || rightCycle || nextDepth > maxResolveDepth { + // Back-edge (cycle): a followed ref re-enters an object already on the + // current path (e.g. a page's /Parent). The target is fully accounted for + // by its first visit, so the shallow-summary comparison hides nothing - this + // is NOT truncation and must not be marked (marking it would flip every real + // multi-page PDF to a false non-identical; Story 14.3 "Only the depth cap is + // truncation"). + if leftCycle || rightCycle { return scalarLeaf(path, "ref", lres, rres) } + // Depth cap: the subtree below maxResolveDepth is ABANDONED unwalked, so the + // shallow summary can hide a deeper difference. Mark it truncated (AC1) so + // the run cannot claim "identical" while a difference was left unexplored. + if nextDepth > maxResolveDepth { + leaf := scalarLeaf(path, "ref", lres, rres) + leaf.Truncated = true + // Record the ref-pair (both sides indirect) so reconcileTruncation can + // clear this mark if the SAME pair is fully walked on a shallower path: + // a pair accounted for elsewhere hides nothing here. Without this a + // shared object reachable both shallow and past the cap over-counts + // TruncatedSubtrees and flips an identical pair to a false exit 1 + // (the visitedPairs dedup below cannot catch it - it runs after this + // return, and the deep leg may be walked before the shallow one). + if lKey != "" && rKey != "" { + leaf.capRefPair = lKey + "|" + rKey + } + return leaf + } // Global cross-path dedup: an indirect-ref pair present on BOTH sides and // already fully diffed elsewhere is compared by shallow summary rather than @@ -394,6 +442,29 @@ func (dc *diffContext) singleSided(path string, doc *DocumentState, val pdfcpu_t return node } +// reconcileTruncation clears the depth-cap Truncated mark on any node whose +// ref-pair was fully walked elsewhere in the graph (its capRefPair is in +// walkedPairs). A pair diffed on a shallow path is fully accounted for, so a +// second encounter past the depth cap hides nothing; marking it would +// over-count DiffSummary.TruncatedSubtrees and wrongly withhold the "identical" +// verdict (Story 14.3). It runs once after the walk, when walkedPairs is +// complete, so it corrects BOTH DFS orders (shallow-first: the deep leg is +// cleared here; deep-first: the deep leg is marked during the walk, then +// cleared here once the shallow leg has populated walkedPairs). Depth-cap cuts +// with no ref-pair (deep DIRECT nesting, or a ref aligned against a non-ref) +// carry no capRefPair and are left marked - they are genuinely unresolved. +func reconcileTruncation(n *DiffNode, walkedPairs map[string]bool) { + if n == nil { + return + } + if n.Truncated && n.capRefPair != "" && walkedPairs[n.capRefPair] { + n.Truncated = false + } + for _, c := range n.Children { + reconcileTruncation(c, walkedPairs) + } +} + // countDelta tallies the delta POINTS over the whole tree: added/removed // subtrees (always leaves) and changed scalar leaves. A changed CONTAINER (dict // or array with children) is only the route to a deeper change, so counting it @@ -405,6 +476,12 @@ func countDelta(n *DiffNode, s *DiffSummary) { if n == nil { return } + // A depth-capped node is counted regardless of its shallow-summary status: + // its subtree was not walked, so a matching summary does not prove equality + // (Story 14.3 AC2). + if n.Truncated { + s.TruncatedSubtrees++ + } isLeaf := len(n.Children) == 0 switch n.Status { case "added": diff --git a/internal/pdfcore/diff_test.go b/internal/pdfcore/diff_test.go index faedfc8..30cccaf 100644 --- a/internal/pdfcore/diff_test.go +++ b/internal/pdfcore/diff_test.go @@ -257,6 +257,146 @@ func containsStr(ss []string, s string) bool { return false } +// diffDeepChain builds a single-page PDF with a LINEAR chain of nested dict +// refs hanging off the catalog's /Deep key: obj4 -> obj5 -> ... each +// << /L (n+1) 0 R >>, terminating in a << /V value >> leaf. With chainLen well +// above maxResolveDepth (32) the diff's depth cap cuts the chain before it +// reaches the leaf, so a change in the leaf's /V is HIDDEN behind the shallow +// summary at the cut (Story 14.3 #2). Mirrors testdata/correctness/ +// deep-change-{a,b}.pdf without touching disk. +func diffDeepChain(chainLen int, leafValue string) []byte { + objs := []string{ + "1 0 obj\n<< /Type /Catalog /Pages 2 0 R /Deep 4 0 R >>\nendobj\n", + "2 0 obj\n<< /Type /Pages /Kids [3 0 R] /Count 1 >>\nendobj\n", + "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>\nendobj\n", + } + base := 4 // first chain object number + for i := 0; i < chainLen; i++ { + num := base + i + objs = append(objs, fmt.Sprintf("%d 0 obj\n<< /L %d 0 R >>\nendobj\n", num, num+1)) + } + objs = append(objs, fmt.Sprintf("%d 0 obj\n<< /V %s >>\nendobj\n", base+chainLen, leafValue)) + return assembleDiffPDF(1, objs...) +} + +// diffSharedDeepAndShallow builds a single-page PDF where ONE object is +// reachable two ways: directly off the catalog's /Shallow key (depth 1, fully +// walked) AND at the bottom of a linear /Deep chain long enough that the ref to +// it lands past maxResolveDepth (depth-capped). It is the topology that made the +// depth-cap over-count TruncatedSubtrees before reconcileTruncation: the shared +// pair is fully accounted for on the shallow path, so the capped encounter hides +// nothing. Catalog keys sort alphabetically, so /Deep is walked BEFORE /Shallow +// (deep-first order) -- the case a mere check-reorder would not fix. +func diffSharedDeepAndShallow(chainLen int, sharedValue string) []byte { + shared := 4 + chainLen // shared object number, immediately after the chain + objs := []string{ + fmt.Sprintf("1 0 obj\n<< /Type /Catalog /Pages 2 0 R /Shallow %d 0 R /Deep 4 0 R >>\nendobj\n", shared), + "2 0 obj\n<< /Type /Pages /Kids [3 0 R] /Count 1 >>\nendobj\n", + "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>\nendobj\n", + } + base := 4 + for i := 0; i < chainLen; i++ { + num := base + i + // The last chain object (num == shared-1) points its /L at the shared obj. + objs = append(objs, fmt.Sprintf("%d 0 obj\n<< /L %d 0 R >>\nendobj\n", num, num+1)) + } + objs = append(objs, fmt.Sprintf("%d 0 obj\n<< /V %s >>\nendobj\n", shared, sharedValue)) + return assembleDiffPDF(1, objs...) +} + +// --------------------------------------------------------------------------- +// 14.3-UNIT-001b [P1] AC2 regression (Story 14.3 code review): a shared object +// reachable both shallow (fully walked) and past the depth cap must NOT be +// counted as a truncated subtree. Before reconcileTruncation the capped +// encounter over-counted TruncatedSubtrees, flipping an IDENTICAL pair to a +// false "not identical" / exit 1 -- the same false-positive class the spec's +// adversarial review flagged. Exercises the deep-first DFS order. +// --------------------------------------------------------------------------- + +func TestDiff_SharedRefWalkedElsewhereNotTruncated(t *testing.T) { + // Chain length == maxResolveDepth lands the shared ref EXACTLY at the first + // cap (diffChild depth 32), so the shared object is the only capped node and + // no deep-only intermediate is cut. Identical inputs: the sole "truncation" + // is that shared object seen past the cap on the /Deep path, which is fully + // walked on the /Shallow path. + pdf := diffSharedDeepAndShallow(maxResolveDepth, "111") + ins, l, r := openTwoForDiff(t, "sharedA.pdf", pdf, "sharedB.pdf", pdf) + + res, err := ins.DiffDocuments(l, r) + if err != nil { + t.Fatalf("[P1] 14.3-UNIT-001b: DiffDocuments returned error: %v", err) + } + if res.Summary.TruncatedSubtrees != 0 { + t.Errorf("[P1] 14.3-UNIT-001b: TruncatedSubtrees = %d, want 0 (the shared object is fully walked on the shallow path)", res.Summary.TruncatedSubtrees) + } + if res.Summary.Added != 0 || res.Summary.Removed != 0 || res.Summary.Changed != 0 { + t.Errorf("[P1] 14.3-UNIT-001b: identical inputs reported deltas Added=%d Removed=%d Changed=%d, want all 0", res.Summary.Added, res.Summary.Removed, res.Summary.Changed) + } + // No node may still carry the Truncated mark after reconciliation. + for _, n := range collectDiffNodes(res.Root) { + if n.Truncated { + t.Errorf("[P1] 14.3-UNIT-001b: node %q still marked Truncated after reconciliation", n.Path) + } + } +} + +// --------------------------------------------------------------------------- +// 14.3-UNIT-001 [P1] AC1/AC2 (Story 14.3): a deep chain diffed past the +// maxResolveDepth cap marks the cut node DiffNode.Truncated and tallies it in +// DiffSummary.TruncatedSubtrees, and does so only at the depth-cap arm (a +// self-diff of a SHALLOW graph counts zero). This is the co-located pdfcore +// logic assertion the ATDD step deferred until the production types existed. +// --------------------------------------------------------------------------- + +func TestDiff_DepthCapMarksTruncatedSubtree(t *testing.T) { + // A chain far deeper than maxResolveDepth (32); the two sides differ only in + // the leaf /V, well below the cut, so the difference is hidden. + left := diffDeepChain(45, "111") + right := diffDeepChain(45, "222") + ins, l, r := openTwoForDiff(t, "deepA.pdf", left, "deepB.pdf", right) + + res, err := ins.DiffDocuments(l, r) + if err != nil { + t.Fatalf("[P1] 14.3-UNIT-001: DiffDocuments returned error: %v", err) + } + + if res.Summary.TruncatedSubtrees < 1 { + t.Errorf("[P1] 14.3-UNIT-001: TruncatedSubtrees = %d, want >= 1 (the deep chain is cut at the depth cap)", res.Summary.TruncatedSubtrees) + } + + nodes := collectDiffNodes(res.Root) + truncated := 0 + for _, n := range nodes { + if n.Truncated { + truncated++ + // A truncated node is the depth-capped ref, compared by shallow + // summary, so it carries Kind "ref" and no children. + if n.Kind != "ref" { + t.Errorf("[P1] 14.3-UNIT-001: truncated node %q kind = %q, want \"ref\"", n.Path, n.Kind) + } + if len(n.Children) != 0 { + t.Errorf("[P1] 14.3-UNIT-001: truncated node %q has %d children, want 0 (subtree not walked)", n.Path, len(n.Children)) + } + } + } + if truncated != res.Summary.TruncatedSubtrees { + t.Errorf("[P1] 14.3-UNIT-001: %d nodes carry Truncated but summary counts %d", truncated, res.Summary.TruncatedSubtrees) + } + + // Guardrail (the "only the depth cap is truncation" rule): a SHALLOW graph + // diffed against itself must not mark anything -- else every real PDF's + // back-edges/dedup would flip to a false non-identical. + shallow := diffOnePage() + ins2, l2, r2 := openTwoForDiff(t, "s1.pdf", shallow, "s2.pdf", shallow) + res2, err := ins2.DiffDocuments(l2, r2) + if err != nil { + t.Fatalf("[P1] 14.3-UNIT-001: shallow self-diff returned error: %v", err) + } + if res2.Summary.TruncatedSubtrees != 0 { + t.Errorf("[P1] 14.3-UNIT-001: shallow self-diff TruncatedSubtrees = %d, want 0 (cycles/dedup are not truncation)", res2.Summary.TruncatedSubtrees) + } +} + // --------------------------------------------------------------------------- // 13.6-UNIT-001 [P0] AC1/AC6: a document diffed against ITSELF (same bytes, // two tabs) yields an all-unchanged tree and a zero-delta summary. diff --git a/internal/pdfcore/model.go b/internal/pdfcore/model.go index 8746f30..ad928af 100644 --- a/internal/pdfcore/model.go +++ b/internal/pdfcore/model.go @@ -60,12 +60,22 @@ type ValueEntry struct { } // ContentStreamData holds raw and tokenized content stream data for a page. +// +// StreamCount/Truncated/Shown are the Story 14.3 multi-stream truncation marker +// (AC3/AC4): when a page's /Contents is an array of more than one stream, the +// CLI decodes only the first and sets StreamCount to the array length, Shown to +// 1, and Truncated true so no consumer mistakes the partial (often unbalanced) +// program for the whole content stream. All three are zero/false - and omitted +// from JSON - for single-stream pages and non-page streams. type ContentStreamData struct { - NodeID string `json:"nodeId"` - Raw string `json:"raw"` - Tokenized []Token `json:"tokenized"` - Formatted []FormattedLine `json:"formatted"` - Error string `json:"error"` + NodeID string `json:"nodeId"` + Raw string `json:"raw"` + Tokenized []Token `json:"tokenized"` + Formatted []FormattedLine `json:"formatted"` + Error string `json:"error"` + StreamCount int `json:"streamCount,omitempty"` + Shown int `json:"shown,omitempty"` + Truncated bool `json:"truncated,omitempty"` } // FormattedLine is one logical PDF operation in a content stream: zero or more diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index 04af35a..70cf794 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -56,6 +56,65 @@ func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (str } } +// GetPageContentStreamCount resolves a 1-based page number to the number of +// content streams in its /Contents entry: 0 when the page has no /Contents, 1 +// for a single indirect ref, and, for an array, the count of its indirect-ref +// elements (a degenerate null / non-ref element contributes no stream per ISO +// 32000-1 7.8.2, so it is not counted). It is the additive companion to +// GetPageContentStreamNodeID (whose +// single-string return discards the array length) added for the Story 14.3 +// multi-stream truncation marker; keeping it a separate method avoids rippling +// the widely-called GetPageContentStreamNodeID signature. Returns 0 (no error) +// for a page with no Contents. +func (ins *Inspector) GetPageContentStreamCount(tabID string, pageNum int) (int, error) { + doc, err := ins.GetDocument(tabID) + if err != nil { + return 0, err + } + // PageDict mutates the pdfcpu page-resolution cache; serialize (same as + // GetPageContentStreamNodeID). + doc.pdfMu.Lock() + defer doc.pdfMu.Unlock() + + var pageDict pdfcpu_types.Dict + err = safeCall(func() error { + var e error + pageDict, _, _, e = doc.PDFContext.PageDict(pageNum, false) + return e + }) + if err != nil { + return 0, wrapPDFError(err) + } + if pageDict == nil { + return 0, nil + } + + contents, found := pageDict.Find("Contents") + if !found || contents == nil { + return 0, nil + } + + switch v := contents.(type) { + case pdfcpu_types.IndirectRef: + return 1, nil + case pdfcpu_types.Array: + // Count only indirect-ref elements: per ISO 32000-1 7.8.2 the content is + // the concatenation of the array's STREAM refs, so a degenerate null or + // non-ref element contributes no stream. Counting len(v) would report a + // false "showing stream 1 of N" truncation for e.g. [ref null] where the + // single stream is in fact shown in full. + count := 0 + for _, e := range v { + if _, ok := e.(pdfcpu_types.IndirectRef); ok { + count++ + } + } + return count, nil + default: + return 0, nil + } +} + // GetPageNode resolves a 1-based page number to a fully-populated TreeNode for // that page's page dict (/Type /Page object), suitable for rooting a tree walk. // The returned node carries the page object's ObjectRef (" R") and diff --git a/internal/pdfcore/stream_test.go b/internal/pdfcore/stream_test.go index 5de6d53..3662172 100644 --- a/internal/pdfcore/stream_test.go +++ b/internal/pdfcore/stream_test.go @@ -2,10 +2,13 @@ package pdfcore import ( "errors" + "fmt" "path/filepath" "strings" "sync" "testing" + + pdfcpu_types "github.com/pdfcpu/pdfcpu/pkg/pdfcpu/types" ) // --------------------------------------------------------------------------- @@ -399,6 +402,120 @@ func TestGetPageContentStreamNodeID_NoContentsEntry(t *testing.T) { } } +// --------------------------------------------------------------------------- +// 14.3-UNIT-002 [P1] AC3 (Story 14.3, Code Review #1 fix): the anti-false- +// positive negative path of GetPageContentStreamCount. +// +// The multi-stream truncation marker fires only when streamCount >= 2. Per ISO +// 32000-1 7.8.2 a page's content is the concatenation of its /Contents array's +// STREAM refs, so a degenerate null / non-ref element contributes NO stream. +// Before the code-review fix the body returned len(v), so `[ref null]` reported +// streamCount 2 and the CLI falsely marked a single, fully-shown stream as +// "stream 1 of 2, truncated". This test pins the fixed contract: a single +// indirect ref, a one-element array `[ref]`, and the degenerate `[ref null]` +// all count 1 (no marker); only a genuine multi-ref array `[ref ref]` counts 2; +// a page with no /Contents counts 0. +// +// The `[ref null]` case is built by an in-memory page-dict mutation, not a disk +// fixture: pdfcpu rejects an on-disk /Contents array containing a null element +// at read time (DereferenceStreamDict: wrong type ), so the only way to +// drive that degenerate array into GetPageContentStreamCount is to inject it +// after a valid open. This still exercises the production count loop verbatim. +// --------------------------------------------------------------------------- + +// contentStreamObj builds a minimal, valid content-stream object body numbered +// n for the GetPageContentStreamCount fixtures. +func contentStreamObj(n int) string { + body := "BT /F1 12 Tf 100 700 Td (x) Tj ET" + return fmt.Sprintf("%d 0 obj\n<< /Length %d >>\nstream\n%s\nendstream\nendobj\n", n, len(body), body) +} + +func TestGetPageContentStreamCount(t *testing.T) { + catalog := "1 0 obj\n<< /Type /Catalog /Pages 2 0 R >>\nendobj\n" + pages := "2 0 obj\n<< /Type /Pages /Kids [3 0 R] /Count 1 >>\nendobj\n" + + cases := []struct { + name string + page string + objs []string + want int + }{ + { + name: "single indirect ref", + page: "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R >>\nendobj\n", + objs: []string{contentStreamObj(4)}, + want: 1, + }, + { + name: "one-element array [ref]", + page: "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents [4 0 R] >>\nendobj\n", + objs: []string{contentStreamObj(4)}, + want: 1, + }, + { + name: "multi-ref array [ref ref]", + page: "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents [4 0 R 5 0 R] >>\nendobj\n", + objs: []string{contentStreamObj(4), contentStreamObj(5)}, + want: 2, + }, + { + name: "no /Contents entry", + page: "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>\nendobj\n", + objs: nil, + want: 0, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + objs := append([]string{catalog, pages, tc.page}, tc.objs...) + ins, tabID := writeTempPDF(t, "count.pdf", assembleDiffPDF(1, objs...)) + got, err := ins.GetPageContentStreamCount(tabID, 1) + if err != nil { + t.Fatalf("[14.3-UNIT-002] %s: unexpected error: %v", tc.name, err) + } + if got != tc.want { + t.Errorf("[14.3-UNIT-002] %s: count = %d, want %d", tc.name, got, tc.want) + } + }) + } + + // Anti-false-positive: a degenerate [ref null] array must count 1, not 2. + // pdfcpu rejects this shape on disk, so open a valid [ref] fixture and inject + // the trailing null into the live page dict before the count call. + t.Run("degenerate array [ref null] is not truncation", func(t *testing.T) { + objs := []string{ + catalog, + pages, + "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents [4 0 R] >>\nendobj\n", + contentStreamObj(4), + } + ins, tabID := writeTempPDF(t, "refnull.pdf", assembleDiffPDF(1, objs...)) + doc, err := ins.GetDocument(tabID) + if err != nil { + t.Fatalf("[14.3-UNIT-002] GetDocument: %v", err) + } + pageDict, _, _, err := doc.PDFContext.PageDict(1, false) + if err != nil { + t.Fatalf("[14.3-UNIT-002] PageDict: %v", err) + } + arr, ok := pageDict["Contents"].(pdfcpu_types.Array) + if !ok || len(arr) != 1 { + t.Fatalf("[14.3-UNIT-002] fixture broken: /Contents = %v, want a one-element array", pageDict["Contents"]) + } + ref := arr[0].(pdfcpu_types.IndirectRef) + // Inject the degenerate [ref null] shape the fix guards against. + pageDict["Contents"] = pdfcpu_types.Array{ref, nil} + + got, err := ins.GetPageContentStreamCount(tabID, 1) + if err != nil { + t.Fatalf("[14.3-UNIT-002] unexpected error: %v", err) + } + if got != 1 { + t.Errorf("[14.3-UNIT-002] [ref null] count = %d, want 1 (a null element is not a stream; counting it would fire a false truncation marker)", got) + } + }) +} + // --------------------------------------------------------------------------- // 3.3-UNIT-001 [P0]: Tokenizer produces correct Token structs for a reference // content stream line: "BT /F1 12 Tf (Hello) Tj ET". diff --git a/testdata/correctness/README.md b/testdata/correctness/README.md index 3068cbf..41b6d0d 100644 --- a/testdata/correctness/README.md +++ b/testdata/correctness/README.md @@ -141,3 +141,54 @@ overflows: `0xFFFE + 2 = 0x10000` -> trailing = `0x0000`, carry 1 into the leading unit -> `0x00FF + 1 = 0x0100`. Result: `[0100 0000]` = `U+0100 U+0000`. Pre-fix this entry was silently dropped (the `break` on `tail > 0xFFFF`); post-fix the carry propagates correctly. + +## deep-change-a.pdf / deep-change-b.pdf + +A pair of single-page PDFs that are byte-identical EXCEPT one scalar nested far +below the catalog, used to make the `diff` depth cap honest (Story 14-3, #2). + +``` +obj 1: /Type /Catalog /Pages 2 0 R /Deep 4 0 R +obj 2: /Type /Pages /Kids [3 0 R] /Count 1 +obj 3: /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] +obj 4..47: << /L (n+1) 0 R >> (a linear chain of nested dicts) +obj 48: << /V 111 >> (deep-change-a) | << /V 222 >> (deep-change-b) +``` + +The catalog is diff depth 0; `diffChild` increments before the depth check, so +the depth-32 cap first cuts at catalog-depth 33 -- the `/Root/Deep/L/.../L` node +(32 `/L` steps). At that cut the one-level shallow summary is `<< /L >>`, +identical on both sides (refs render number-independent), so the differing +scalar at obj 48 (catalog-depth ~45, well below the cut) is never reached. + +Pre-fix `diff deep-change-a deep-change-b` reports "Documents are structurally +identical." at exit 0 -- an inverted answer. Post-fix the cut node is marked +`truncated`, `DiffSummary.truncatedSubtrees > 0`, the run is not called +identical, and the exit code is 1. The `/V` value is a fixed 3-digit token so +the two files share identical byte lengths and xref offsets. 48 objects each. + +## multi-content-stream.pdf + +Single-page PDF whose `/Contents` is an ARRAY of two content-stream refs whose +operators only balance when concatenated (Story 14-3, #5): + +``` +obj 3: /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] + /Resources << >> /Contents [4 0 R 5 0 R] +obj 4 (stream 1): q + 1 0 0 1 50 700 cm (opens a q, no matching Q) +obj 5 (stream 2): BT + /F1 24 Tf + 0 0 Td + (Hello) Tj + ET + Q (the matching Q + a text block) +``` + +Per ISO 32000-1 7.8.2 the page content is the concatenation of both streams +joined by whitespace. Pre-fix `GetPageContentStreamNodeID` returns only the +first ref's node ID, so `dump stream --page 1` decodes ONLY stream 1 (`q`, `cm`) +and presents an unbalanced partial program with no marker. Post-fix the tool +either concatenates both streams (operators from both appear) or emits a +machine-visible truncation marker (`streamCount`/`truncated`) on `--json` and +`--ops`. 5 objects total. diff --git a/testdata/correctness/deep-change-a.pdf b/testdata/correctness/deep-change-a.pdf new file mode 100644 index 0000000000000000000000000000000000000000..3d64ffc622ed49f51fae7fb5af494de2e2b36081 GIT binary patch literal 2670 zcmZveOHbTD5QOjk6?55RV)s1!kPwH2aLGnNB;tZNI9h`?LTlvRNF={L(;bL=s@W5+ zo|^W2)i&N-U0lAS_pu_^xcSG=>g>$b-+w<%uKqZl#>e@=)mP)gbaXy1xOVl$G(EW} zf1jUM(_zOZu9fD0p8E5CcXYRbm*3?b=U<1DBexp4VvXE#nex(?o?oWje*7>$yW1wO zZ^^rE@ONq(zMt1^wk~lyRpslo+Cr7D*J=kqxdPpBn0#daes)b!-vT{ummS)CjSOp!^7qN^*qgL{NT&M!zu zejfRWp!~?s>*3rwkNiYXe&pxNx%0?R1m#D5zMMUe{6tWGA|)gO?b2&#_! zLOFi{`H7(X$S;)h7m%L_%8&d)Ie!89iJ<(*FO>5ake>*vKO#R7R2})na{eOn6G8cr zUo7V@B0mw7ANj>{{vz@dLHUtiEaxvGKM`L2H;zcZ*q@i_zG_??s^7t?*LAi>+t?m0KQ`cPTic@@Y>$o~dGL0v?a>XkNB>4feX>3J!S)#VRSSKGWP1#~ zIR`cTZ=&>4lS~h4deb9Mtm#3-V-Ne0&78c=oV?9EkFMg##tkGq$Tl;de!PVyF@*2ikI`SkU& F`U~3pQ7Zrd literal 0 HcmV?d00001 diff --git a/testdata/correctness/deep-change-b.pdf b/testdata/correctness/deep-change-b.pdf new file mode 100644 index 0000000000000000000000000000000000000000..bc6ba8220936b08b9510ea7619153a61ae53d8a5 GIT binary patch literal 2670 zcmZve&2Q5{5XJBQEB2Bj#Iqm%kg6V_^g<9-LA_Kx7{#rEs<=vmRQT)J8CrJsO>>Ih z?DN>aH*0%!b#eKQ-p7Vq>y{rso3k^w{{H)EcI%JRX?k29-1=&In2*lq1=ntUG0#sf z%K!88X5Mev#I@S|&$Ir#+aBF*;N^GujLWb6$&o8Yu2>_tJj{9NOV2O!b~k-kp51Mm z*LUPyKl(d$8or*_(`;PgcB;z9rP@K2k4v?ODj%2X098IN)e)+ET&l#^t}jajRp$h% zx{hSy@?Aq+Q_ppwtZrKbb$$$uYHEboL{NSNM>RP@bRsA}!lRlVAwCg`-y=T}R2}*G znx990A}Bxd^LjWp&Lck&lpp!|dhR^(6G8crpRZ@nBR>(8ANl!u{sQt7q4)#x6G7FH zU#RCVAU_e5ANhrP{sQt7LHUtisOK*rKM|B4`GtD^0`e1~_#^TYLDi98tmiKxKM|B4 z`NewvBJvYK`H^3&=Px2Z5tJYK#d`iC@)P0Jf8%`KPvyb>e3*2S@D|NmT+Lm1%2C6*$nl}p=SV<_7VvW5IfICs&Xi_LIOlva;rF)#to^`7#vqBd_B8KAcgGBv$Hd^ zUT1l`xDwX^6Tm^g?%8w-Zua@Ez`c=HR=NOpDGSwrhlDkxDN}V$M0)=n=y>jZnK!WU ziT;8r`dC{KXM@xk50Z^hNbP*wN0payrcbcp$d7~vapIxgS~dD;wipPLrYRZJ7U$41 zAWH~4;~^-)KYkCYF6vC~=mvi#t_S`Vz7|n`dk_7Cjk7rnbNQtp!VlJ(?$sZ=vC`N!r#}(Qapq48_6-QOkc$8S literal 0 HcmV?d00001 diff --git a/tests/14-3-no-silent-truncation/diff_truncation_test.go b/tests/14-3-no-silent-truncation/diff_truncation_test.go new file mode 100644 index 0000000..bd4bf5e --- /dev/null +++ b/tests/14-3-no-silent-truncation/diff_truncation_test.go @@ -0,0 +1,98 @@ +package no_silent_truncation_test + +// Story 14.3 #2 -- the diff depth-cap quiet lie (AC1, AC2). +// +// RED PHASE: the depth-32 diff cap compares any subtree below it by SHALLOW +// SUMMARY only. deep-change-{a,b}.pdf differ by one scalar (/V 111 vs /V 222) +// nested ~45 catalog-levels deep; the cap bites at catalog-depth 33 (the +// `/Root/Deep/L/.../L` ref node) and its one-level summary `<< /L >>` is +// identical on both sides, so the differing leaf is never reached. Today the +// run reports "Documents are structurally identical." at exit 0 -- an INVERTED +// answer for a script keying on the exit code. These tests encode the GREEN +// target and MUST FAIL today. + +import ( + "strings" + "testing" +) + +// --------------------------------------------------------------------------- +// 14.3-INTG-000 [P0] fixture sanity: both deep-change fixtures parse through +// the EXISTING open path (dump objects, exit 0). Passes TODAY; guards the suite +// against an eternally-red fixture. +// --------------------------------------------------------------------------- + +func TestDeepChange_FixturesParseThroughOpenPath(t *testing.T) { + bin := buildCLI(t) + for _, name := range []string{"deep-change-a.pdf", "deep-change-b.pdf"} { + _, stderr, ec := runCLI(t, bin, "dump", "objects", fixturePath(t, name)) + if ec != 0 { + t.Fatalf("[P0] 14.3-INTG-000: fixture %q rejected by the existing open path (exit %d): %s", name, ec, stderr) + } + } +} + +// --------------------------------------------------------------------------- +// 14.3-INTG-001 [P1] AC2: a depth-cap-bounded comparison must NOT claim +// "identical". The plain-text run must withhold the identical banner and exit 1 +// (not a false 0), and must state that a subtree was compared only to the depth +// cap. RED today: prints "Documents are structurally identical.", exit 0. +// --------------------------------------------------------------------------- + +func TestDiff_DepthCappedNotIdentical_PlainText(t *testing.T) { + bin := buildCLI(t) + a := fixturePath(t, "deep-change-a.pdf") + b := fixturePath(t, "deep-change-b.pdf") + + stdout, stderr, ec := runCLI(t, bin, "diff", a, b) + if ec != 1 { + t.Fatalf("[P1] 14.3-INTG-001: a depth-capped comparison must exit 1 (not provably identical), got %d\nstdout: %s\nstderr: %s", ec, stdout, stderr) + } + low := strings.ToLower(stdout) + if strings.Contains(low, "structurally identical") { + t.Errorf("[P1] 14.3-INTG-001: a truncated comparison must NOT claim \"structurally identical\":\n%s", stdout) + } + // The document-level depth-cap note (and/or the per-node [truncated: depth + // cap] tag) must be visible so no consumer mistakes the bounded walk for a + // complete one. + if !strings.Contains(low, "truncat") && !strings.Contains(low, "depth cap") { + t.Errorf("[P1] 14.3-INTG-001: plain-text output must state the walk was bounded by the depth cap (want \"truncat\"/\"depth cap\"):\n%s", stdout) + } +} + +// --------------------------------------------------------------------------- +// 14.3-INTG-001 [P1] AC1/AC2: `diff --json` on the same pair must exit 1, count +// the depth-capped subtree in summary.truncatedSubtrees (> 0), and mark the cut +// node with "truncated": true. RED today: exit 0, no such field. +// --------------------------------------------------------------------------- + +func TestDiff_DepthCappedNotIdentical_JSON(t *testing.T) { + bin := buildCLI(t) + a := fixturePath(t, "deep-change-a.pdf") + b := fixturePath(t, "deep-change-b.pdf") + + stdout, stderr, ec := runCLI(t, bin, "diff", "--json", a, b) + if ec != 1 { + t.Fatalf("[P1] 14.3-INTG-001: `diff --json` on a depth-capped pair must exit 1, got %d\nstderr: %s", ec, stderr) + } + res := parseObject(t, "14.3-INTG-001", stdout) + + sum, ok := res["summary"].(map[string]any) + if !ok { + t.Fatalf("[P1] 14.3-INTG-001: result has no \"summary\" object: %v", res) + } + if _, present := sum["truncatedSubtrees"]; !present { + t.Fatalf("[P1] 14.3-INTG-001: summary is missing the additive \"truncatedSubtrees\" count (AC2): %v", sum) + } + if n := jsonInt(sum["truncatedSubtrees"]); n < 1 { + t.Errorf("[P1] 14.3-INTG-001: summary.truncatedSubtrees = %d, want >= 1 (the deep chain is cut once)", n) + } + + root, ok := res["root"].(map[string]any) + if !ok { + t.Fatalf("[P1] 14.3-INTG-001: result has no \"root\" DiffNode object") + } + if !anyNodeTruncated(root) { + t.Errorf("[P1] 14.3-INTG-001: no DiffNode carries \"truncated\": true; the depth-cap cut node must be marked (AC1)") + } +} diff --git a/tests/14-3-no-silent-truncation/go.mod b/tests/14-3-no-silent-truncation/go.mod new file mode 100644 index 0000000..a6818e4 --- /dev/null +++ b/tests/14-3-no-silent-truncation/go.mod @@ -0,0 +1,3 @@ +module no-silent-truncation-test + +go 1.25 diff --git a/tests/14-3-no-silent-truncation/helpers_test.go b/tests/14-3-no-silent-truncation/helpers_test.go new file mode 100644 index 0000000..9dd87c1 --- /dev/null +++ b/tests/14-3-no-silent-truncation/helpers_test.go @@ -0,0 +1,197 @@ +// Story 14-3 RED-PHASE acceptance harness for the "no silent truncation" rule +// across two machine-contract surfaces: `diff` (depth-cap under-report) and +// `dump stream` (multi-stream /Contents). +// +// Black-box: build the pdfdebug CLI binary and run it as a subprocess against +// the committed correctness-corpus fixtures deep-change-{a,b}.pdf and +// multi-content-stream.pdf. These tests assert the EXPECTED post-implementation +// contract and MUST FAIL against the current binary until Story 14-3 is +// implemented. They fail at RUNTIME (a truncated diff reports "identical" at +// exit 0; a multi-stream page shows only stream 1 with no marker), NOT at +// compile time, so the main `unidoc-pdf-debugger` module keeps building green. +// This module has its own go.mod and is not part of the main build (mirrors +// tests/14-1-trustworthy-stream-op-output and tests/13-6-structural-diff). +// +// Test pyramid: every case here is a Go integration-level black-box test +// against the built CLI binary -- the project's established acceptance level for +// the CLI machine contract (10-x, 13-1..13-6, 14-1). The diff depth-cap count +// (14.3-UNIT-001's intent) is asserted through the `diff --json` +// summary.truncatedSubtrees field rather than a co-located internal/pdfcore unit +// test: that field/`DiffNode.Truncated` do not exist yet, so a co-located test +// would break the main module's compile (and `go vet`/gate), violating the +// runtime-red convention. The thin GUI display branch is covered at the +// component (Vitest) level in frontend/src/components/DiffView.truncation.test.tsx. +// +// Naming: 14.3-INTG-NNN [Px] per the story Testing Requirements (AC5/AC6). +// +// Run: cd tests/14-3-no-silent-truncation && go test -v -count=1 ./... +package no_silent_truncation_test + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" +) + +// projectRoot walks up from the test directory to find the main module's go.mod. +func projectRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get working directory: %v", err) + } + for { + goModPath := filepath.Join(dir, "go.mod") + if content, err := os.ReadFile(goModPath); err == nil { + if strings.Contains(string(content), "module unidoc-pdf-debugger") { + return dir + } + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatalf("could not find project root (no go.mod with module unidoc-pdf-debugger found)") + } + dir = parent + } +} + +// fixturePath returns the absolute path to a committed correctness-corpus fixture. +func fixturePath(t *testing.T, name string) string { + t.Helper() + return filepath.Join(projectRoot(t), "testdata", "correctness", name) +} + +var ( + cliBuildOnce sync.Once + cliBinPath string + cliBuildErr string +) + +// buildCLI compiles the CLI binary once per test package and returns its path. +// Cached via sync.Once: the binary is identical for every test in the module. +func buildCLI(t *testing.T) string { + t.Helper() + cliBuildOnce.Do(func() { + root := projectRoot(t) + binName := "pdfdebug" + if runtime.GOOS == "windows" { + binName += ".exe" + } + tmpDir, err := os.MkdirTemp("", "pdfdebug-cli-") + if err != nil { + cliBuildErr = "failed to create temp dir: " + err.Error() + return + } + binPath := filepath.Join(tmpDir, binName) + cmd := exec.Command("go", "build", "-o", binPath, "./cmd/cli/") + cmd.Dir = root + if output, err := cmd.CombinedOutput(); err != nil { + cliBuildErr = "failed to build CLI binary: " + err.Error() + "\n" + string(output) + return + } + cliBinPath = binPath + }) + if cliBuildErr != "" { + t.Fatalf("%s", cliBuildErr) + } + return cliBinPath +} + +// runCLI executes the CLI binary with args and returns stdout, stderr, exit code. +func runCLI(t *testing.T, binPath string, args ...string) (stdout, stderr string, exitCode int) { + t.Helper() + cmd := exec.Command(binPath, args...) + var outBuf, errBuf strings.Builder + cmd.Stdout = &outBuf + cmd.Stderr = &errBuf + err := cmd.Run() + exitCode = 0 + if err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + exitCode = exitErr.ExitCode() + } else { + t.Fatalf("failed to run CLI: %v", err) + } + } + return outBuf.String(), errBuf.String(), exitCode +} + +// --- JSON helpers ------------------------------------------------------------ + +// parseObject parses stdout as a single top-level JSON object. +func parseObject(t *testing.T, id, stdout string) map[string]any { + t.Helper() + trimmed := strings.TrimSpace(stdout) + if trimmed == "" || trimmed[0] != '{' { + t.Fatalf("[%s] expected a top-level JSON object, got:\n%s", id, stdout) + } + var res map[string]any + if err := json.Unmarshal([]byte(stdout), &res); err != nil { + t.Fatalf("[%s] failed to parse JSON output: %v\nraw: %s", id, err, stdout) + } + return res +} + +// jsonInt coerces a JSON number (float64) to int; non-numbers yield 0. +func jsonInt(v any) int { + f, _ := v.(float64) + return int(f) +} + +// anyNodeTruncated reports whether the DiffNode tree rooted at node carries a +// node with "truncated": true anywhere. This is the depth-cap marker (AC1) the +// implementation adds to DiffNode; today the field is absent, so this is false. +func anyNodeTruncated(node map[string]any) bool { + if node == nil { + return false + } + if b, ok := node["truncated"].(bool); ok && b { + return true + } + children, ok := node["children"].([]any) + if !ok { + return false + } + for _, c := range children { + if cm, ok := c.(map[string]any); ok && anyNodeTruncated(cm) { + return true + } + } + return false +} + +// formattedOperators extracts the operator strings from a ContentStreamData +// `formatted` array (the `dump stream --json` shape). Empty-operator lines +// (comments / dangling operand runs) are skipped. +func formattedOperators(res map[string]any) []string { + var ops []string + formatted, ok := res["formatted"].([]any) + if !ok { + return ops + } + for _, fl := range formatted { + flm, ok := fl.(map[string]any) + if !ok { + continue + } + if op, ok := flm["operator"].(string); ok && op != "" { + ops = append(ops, op) + } + } + return ops +} + +// contains reports whether s is in xs. +func contains(xs []string, s string) bool { + for _, x := range xs { + if x == s { + return true + } + } + return false +} diff --git a/tests/14-3-no-silent-truncation/multistream_test.go b/tests/14-3-no-silent-truncation/multistream_test.go new file mode 100644 index 0000000..f7211cf --- /dev/null +++ b/tests/14-3-no-silent-truncation/multistream_test.go @@ -0,0 +1,136 @@ +package no_silent_truncation_test + +// Story 14.3 #5 -- multi-stream /Contents shows only the first stream (AC3, AC4). +// +// RED PHASE: multi-content-stream.pdf has one page whose /Contents is an array +// of two stream refs. Stream 1 is `q ... cm` (opens a graphics state, NO +// matching Q); stream 2 is `BT /F1 24 Tf 0 0 Td (Hello) Tj ET Q` (the matching +// Q plus a text block). Per ISO 32000-1 7.8.2 the page's content is the +// CONCATENATION of both. Today `dump stream --page 1` decodes ONLY stream 1 and +// emits no marker, presenting an unbalanced partial program as if it were the +// whole content stream. +// +// The GREEN target is path-dependent (Task 0's return-type decision), so each +// assertion accepts EITHER outcome and fails only the silent stream-1-only +// state that is wrong under both: +// - preferred: the output covers BOTH streams (a stream-2 operator such as Q +// or Tj is present); OR +// - floor: a machine-visible truncation marker (streamCount / truncated) is +// present so no consumer mistakes the partial for the whole. +// Today neither holds -> RED. + +import ( + "strings" + "testing" +) + +// stream2Operators are operators that appear ONLY in the second content stream; +// their presence proves the concatenation (preferred) path covered stream 2. +var stream2Operators = []string{"Q", "BT", "Tf", "Td", "Tj", "ET"} + +// --------------------------------------------------------------------------- +// 14.3-INTG-002 [P0] fixture sanity: the multi-stream fixture parses through +// the existing open path (dump objects, exit 0). Passes TODAY. +// --------------------------------------------------------------------------- + +func TestMultiStream_FixtureParsesThroughOpenPath(t *testing.T) { + bin := buildCLI(t) + _, stderr, ec := runCLI(t, bin, "dump", "objects", fixturePath(t, "multi-content-stream.pdf")) + if ec != 0 { + t.Fatalf("[P0] 14.3-INTG-002: multi-content-stream.pdf rejected by the open path (exit %d): %s", ec, stderr) + } +} + +// --------------------------------------------------------------------------- +// 14.3-INTG-002 [P1] AC3/AC4: `dump stream --page 1 --json` must NOT present a +// silent stream-1-only view. GREEN is either (preferred) operators from BOTH +// streams, or (floor) a truncation marker with the array length. RED today: +// only stream 1's operators (q, cm), no marker. +// --------------------------------------------------------------------------- + +func TestMultiStream_JSONNotSilentStreamOne(t *testing.T) { + bin := buildCLI(t) + f := fixturePath(t, "multi-content-stream.pdf") + + stdout, stderr, ec := runCLI(t, bin, "dump", "stream", "--page", "1", "--json", f) + if ec != 0 { + t.Fatalf("[P1] 14.3-INTG-002: `dump stream --page 1 --json` must exit 0, got %d\nstderr: %s", ec, stderr) + } + res := parseObject(t, "14.3-INTG-002", stdout) + + ops := formattedOperators(res) + + // Preferred path: the concatenated content carries a stream-2-only operator. + coversStream2 := false + for _, op := range stream2Operators { + if contains(ops, op) { + coversStream2 = true + break + } + } + + // Floor path: a machine-visible marker names the array length / truncation. + _, hasStreamCount := res["streamCount"] + truncated, _ := res["truncated"].(bool) + hasMarker := hasStreamCount || truncated + + if !coversStream2 && !hasMarker { + t.Errorf("[P1] 14.3-INTG-002: --json presents a silent stream-1-only view (operators %v, no stream-2 op, no streamCount/truncated marker); a multi-stream page must either concatenate all streams or carry a truncation marker (AC3/AC4)", ops) + } + + // When the floor path is taken, the marker must report the real array length. + if hasMarker && !coversStream2 { + if sc, ok := res["streamCount"]; !ok || jsonInt(sc) != 2 { + t.Errorf("[P1] 14.3-INTG-002: floor-path marker must report streamCount == 2 (the /Contents array length), got %v", res["streamCount"]) + } + } +} + +// --------------------------------------------------------------------------- +// 14.3-INTG-002 [P1] AC4: `dump stream --page 1 --ops` must NOT silently emit +// only stream 1's operators. GREEN is either (preferred) NDJSON operator +// records from BOTH streams, or (floor) a DISTINCT trailing meta record +// carrying the truncation state (streamCount, no "op" key) that rides the +// NDJSON without breaching Story 14-1's one-object-per-operator contract. RED +// today: only q + cm records, no stream-2 op, no meta record. +// --------------------------------------------------------------------------- + +func TestMultiStream_OpsNotSilentStreamOne(t *testing.T) { + bin := buildCLI(t) + f := fixturePath(t, "multi-content-stream.pdf") + + stdout, stderr, ec := runCLI(t, bin, "dump", "stream", "--page", "1", "--ops", f) + if ec != 0 { + t.Fatalf("[P1] 14.3-INTG-002: `dump stream --page 1 --ops` must exit 0, got %d\nstderr: %s", ec, stderr) + } + + coversStream2 := false + hasMetaMarker := false + for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + rec := parseObject(t, "14.3-INTG-002", line) + if op, ok := rec["op"].(string); ok && op != "" { + for _, s2 := range stream2Operators { + if op == s2 { + coversStream2 = true + } + } + continue + } + // A record with no "op" key is the floor meta marker; it must carry the + // truncation state and NOT masquerade as an operator (14-1 contract). + if _, ok := rec["streamCount"]; ok { + hasMetaMarker = true + if jsonInt(rec["streamCount"]) != 2 { + t.Errorf("[P1] 14.3-INTG-002 (--ops): meta marker streamCount = %v, want 2", rec["streamCount"]) + } + } + } + + if !coversStream2 && !hasMetaMarker { + t.Errorf("[P1] 14.3-INTG-002: --ops silently emits only stream 1's operators; a multi-stream page must emit all streams' operators or a distinct trailing truncation meta record (AC4):\n%s", stdout) + } +} From c64d54e8d2850029b9b33260a0f6b5bbdaf034b2 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 25 Jul 2026 19:03:49 +0700 Subject: [PATCH 02/11] fix: disclose multi-stream truncation on --raw; resolve page content ref once --- cmd/cli/cmd_stream.go | 29 +++-- internal/pdfcore/stream.go | 106 ++++++------------ internal/pdfcore/stream_test.go | 12 +- .../multistream_test.go | 35 ++++++ 4 files changed, 96 insertions(+), 86 deletions(-) diff --git a/cmd/cli/cmd_stream.go b/cmd/cli/cmd_stream.go index 4db6b7b..664e875 100644 --- a/cmd/cli/cmd_stream.go +++ b/cmd/cli/cmd_stream.go @@ -194,6 +194,13 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { } if flags.raw { + // A marker cannot ride stdout without corrupting the verbatim byte dump, + // so on a multi-stream page disclose the truncation on STDERR instead - + // otherwise --raw would present stream 1's bytes as the whole content + // stream with no signal on any channel (Story 14.3 AC4, --raw surface). + if result.Truncated { + fmt.Fprint(os.Stderr, multiStreamTruncationNote(result)) + } if _, err := io.WriteString(os.Stdout, result.Raw); err != nil { fmt.Fprintf(os.Stderr, "failed to write raw output: %v\n", err) return 2 @@ -220,6 +227,14 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { return 0 } +// multiStreamTruncationNote is the one-line note disclosing that a multi-stream +// page's /Contents was truncated to its first decoded stream (Story 14.3 floor +// path). Shared by the plain-text (stdout) and --raw (stderr) surfaces so their +// wording cannot drift; callers guard on result.Truncated before emitting it. +func multiStreamTruncationNote(result *pdfcore.ContentStreamData) string { + return fmt.Sprintf("(truncated: showing stream %d of %d; page /Contents is a multi-stream array)\n", result.Shown, result.StreamCount) +} + // printStreamPlain renders the decoded content stream as a human-readable // operator listing: one operator per line, operands before the operator in // PDF content-stream order (let it flow; do not tabulate). NON-CONTRACTUAL; @@ -233,7 +248,7 @@ func printStreamPlain(out io.Writer, result *pdfcore.ContentStreamData) error { // to zero operators must still disclose that streams 2..N exist, otherwise // the "(empty content stream)" line would be a silent truncation. if result.Truncated { - fmt.Fprintf(&b, "(truncated: showing stream %d of %d; page /Contents is a multi-stream array)\n", result.Shown, result.StreamCount) + b.WriteString(multiStreamTruncationNote(result)) } // A content-stream object can exist yet decode to zero operators (an empty // /Contents stream). Surface that as a one-line note so plain output is never @@ -306,14 +321,10 @@ func resolveStreamNode(inspector *pdfcore.Inspector, info *pdfcore.DocumentInfo, writeJSONError(os.Stderr, fmt.Sprintf("page %d out of range: document has %d pages", flags.page, info.PageCount)) return "", 0, 0, 2 } - id, err := inspector.GetPageContentStreamNodeID("cli", flags.page) - if err != nil { - writeJSONError(os.Stderr, err.Error()) - return "", 0, 0, 2 - } - // Surface the /Contents array length so a multi-stream page can be marked - // as truncated (only the first stream is decoded on the floor path). - count, err := inspector.GetPageContentStreamCount("cli", flags.page) + // One call resolves the page dict once and returns both the nodeID and + // the /Contents array length (the count surfaces the multi-stream + // truncation marker; only the first stream is decoded on the floor path). + id, count, err := inspector.GetPageContentStreamRef("cli", flags.page) if err != nil { writeJSONError(os.Stderr, err.Error()) return "", 0, 0, 2 diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index 70cf794..8c0d5f3 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -7,16 +7,23 @@ import ( pdfcpu_types "github.com/pdfcpu/pdfcpu/pkg/pdfcpu/types" ) -// GetPageContentStreamNodeID resolves a 1-based page number to the node ID of -// its content stream. Returns empty string (no error) when the page has no -// Contents entry. For pages with an array of content stream refs, returns the -// first ref's node ID. -func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (string, error) { +// GetPageContentStreamRef resolves a 1-based page number to its content +// stream's node ID AND the number of content streams in its /Contents entry, in +// a SINGLE page-dict resolution. Node ID is empty (no error) when the page has +// no /Contents; for an array it is the first ref's node ID (error if the first +// element is not an indirect reference). streamCount is 0 with no /Contents, 1 +// for a single indirect ref, and for an array the count of its indirect-ref +// elements only - a degenerate null / non-ref element contributes no stream per +// ISO 32000-1 7.8.2, so counting len(v) would falsely report "showing stream 1 +// of N" for e.g. [ref null] where the single stream is shown in full. Combining +// the two reads keeps the Story 14.3 multi-stream truncation marker from +// resolving (and cache-mutating) the page dict twice per dump. +func (ins *Inspector) GetPageContentStreamRef(tabID string, pageNum int) (nodeID string, streamCount int, err error) { doc, err := ins.GetDocument(tabID) if err != nil { - return "", err + return "", 0, err } - // AC1: PageDict mutates the pdfcpu page-resolution cache; serialize. + // PageDict mutates the pdfcpu page-resolution cache; serialize. doc.pdfMu.Lock() defer doc.pdfMu.Unlock() @@ -27,94 +34,51 @@ func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (str return e }) if err != nil { - return "", wrapPDFError(err) + return "", 0, wrapPDFError(err) } if pageDict == nil { - return "", nil + return "", 0, nil } contents, found := pageDict.Find("Contents") if !found || contents == nil { - return "", nil + return "", 0, nil } switch v := contents.(type) { case pdfcpu_types.IndirectRef: - return fmt.Sprintf("obj:%d:%d", v.GenerationNumber.Value(), v.ObjectNumber.Value()), nil + return fmt.Sprintf("obj:%d:%d", v.GenerationNumber.Value(), v.ObjectNumber.Value()), 1, nil case pdfcpu_types.Array: if len(v) == 0 { - return "", nil + return "", 0, nil } - ref, ok := v[0].(pdfcpu_types.IndirectRef) - if !ok { - return "", fmt.Errorf("contents array element is not an indirect reference") - } - return fmt.Sprintf("obj:%d:%d", ref.GenerationNumber.Value(), ref.ObjectNumber.Value()), nil - default: - return "", fmt.Errorf("unexpected Contents type: %T", contents) - } -} - -// GetPageContentStreamCount resolves a 1-based page number to the number of -// content streams in its /Contents entry: 0 when the page has no /Contents, 1 -// for a single indirect ref, and, for an array, the count of its indirect-ref -// elements (a degenerate null / non-ref element contributes no stream per ISO -// 32000-1 7.8.2, so it is not counted). It is the additive companion to -// GetPageContentStreamNodeID (whose -// single-string return discards the array length) added for the Story 14.3 -// multi-stream truncation marker; keeping it a separate method avoids rippling -// the widely-called GetPageContentStreamNodeID signature. Returns 0 (no error) -// for a page with no Contents. -func (ins *Inspector) GetPageContentStreamCount(tabID string, pageNum int) (int, error) { - doc, err := ins.GetDocument(tabID) - if err != nil { - return 0, err - } - // PageDict mutates the pdfcpu page-resolution cache; serialize (same as - // GetPageContentStreamNodeID). - doc.pdfMu.Lock() - defer doc.pdfMu.Unlock() - - var pageDict pdfcpu_types.Dict - err = safeCall(func() error { - var e error - pageDict, _, _, e = doc.PDFContext.PageDict(pageNum, false) - return e - }) - if err != nil { - return 0, wrapPDFError(err) - } - if pageDict == nil { - return 0, nil - } - - contents, found := pageDict.Find("Contents") - if !found || contents == nil { - return 0, nil - } - - switch v := contents.(type) { - case pdfcpu_types.IndirectRef: - return 1, nil - case pdfcpu_types.Array: - // Count only indirect-ref elements: per ISO 32000-1 7.8.2 the content is - // the concatenation of the array's STREAM refs, so a degenerate null or - // non-ref element contributes no stream. Counting len(v) would report a - // false "showing stream 1 of N" truncation for e.g. [ref null] where the - // single stream is in fact shown in full. count := 0 for _, e := range v { if _, ok := e.(pdfcpu_types.IndirectRef); ok { count++ } } - return count, nil + ref, ok := v[0].(pdfcpu_types.IndirectRef) + if !ok { + return "", 0, fmt.Errorf("contents array element is not an indirect reference") + } + return fmt.Sprintf("obj:%d:%d", ref.GenerationNumber.Value(), ref.ObjectNumber.Value()), count, nil default: - return 0, nil + return "", 0, fmt.Errorf("unexpected Contents type: %T", contents) } } +// GetPageContentStreamNodeID resolves a 1-based page number to the node ID of +// its content stream. Returns empty string (no error) when the page has no +// Contents entry. For pages with an array of content stream refs, returns the +// first ref's node ID. Thin wrapper over GetPageContentStreamRef that discards +// the stream count (bound in pdfservice.Service; signature preserved). +func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (string, error) { + nodeID, _, err := ins.GetPageContentStreamRef(tabID, pageNum) + return nodeID, err +} + // GetPageNode resolves a 1-based page number to a fully-populated TreeNode for // that page's page dict (/Type /Page object), suitable for rooting a tree walk. // The returned node carries the page object's ObjectRef (" R") and diff --git a/internal/pdfcore/stream_test.go b/internal/pdfcore/stream_test.go index 3662172..c167003 100644 --- a/internal/pdfcore/stream_test.go +++ b/internal/pdfcore/stream_test.go @@ -404,7 +404,7 @@ func TestGetPageContentStreamNodeID_NoContentsEntry(t *testing.T) { // --------------------------------------------------------------------------- // 14.3-UNIT-002 [P1] AC3 (Story 14.3, Code Review #1 fix): the anti-false- -// positive negative path of GetPageContentStreamCount. +// positive negative path of GetPageContentStreamRef's stream count. // // The multi-stream truncation marker fires only when streamCount >= 2. Per ISO // 32000-1 7.8.2 a page's content is the concatenation of its /Contents array's @@ -419,18 +419,18 @@ func TestGetPageContentStreamNodeID_NoContentsEntry(t *testing.T) { // The `[ref null]` case is built by an in-memory page-dict mutation, not a disk // fixture: pdfcpu rejects an on-disk /Contents array containing a null element // at read time (DereferenceStreamDict: wrong type ), so the only way to -// drive that degenerate array into GetPageContentStreamCount is to inject it +// drive that degenerate array into GetPageContentStreamRef is to inject it // after a valid open. This still exercises the production count loop verbatim. // --------------------------------------------------------------------------- // contentStreamObj builds a minimal, valid content-stream object body numbered -// n for the GetPageContentStreamCount fixtures. +// n for the GetPageContentStreamRef count fixtures. func contentStreamObj(n int) string { body := "BT /F1 12 Tf 100 700 Td (x) Tj ET" return fmt.Sprintf("%d 0 obj\n<< /Length %d >>\nstream\n%s\nendstream\nendobj\n", n, len(body), body) } -func TestGetPageContentStreamCount(t *testing.T) { +func TestGetPageContentStreamRefCount(t *testing.T) { catalog := "1 0 obj\n<< /Type /Catalog /Pages 2 0 R >>\nendobj\n" pages := "2 0 obj\n<< /Type /Pages /Kids [3 0 R] /Count 1 >>\nendobj\n" @@ -469,7 +469,7 @@ func TestGetPageContentStreamCount(t *testing.T) { t.Run(tc.name, func(t *testing.T) { objs := append([]string{catalog, pages, tc.page}, tc.objs...) ins, tabID := writeTempPDF(t, "count.pdf", assembleDiffPDF(1, objs...)) - got, err := ins.GetPageContentStreamCount(tabID, 1) + _, got, err := ins.GetPageContentStreamRef(tabID, 1) if err != nil { t.Fatalf("[14.3-UNIT-002] %s: unexpected error: %v", tc.name, err) } @@ -506,7 +506,7 @@ func TestGetPageContentStreamCount(t *testing.T) { // Inject the degenerate [ref null] shape the fix guards against. pageDict["Contents"] = pdfcpu_types.Array{ref, nil} - got, err := ins.GetPageContentStreamCount(tabID, 1) + _, got, err := ins.GetPageContentStreamRef(tabID, 1) if err != nil { t.Fatalf("[14.3-UNIT-002] unexpected error: %v", err) } diff --git a/tests/14-3-no-silent-truncation/multistream_test.go b/tests/14-3-no-silent-truncation/multistream_test.go index f7211cf..d591350 100644 --- a/tests/14-3-no-silent-truncation/multistream_test.go +++ b/tests/14-3-no-silent-truncation/multistream_test.go @@ -134,3 +134,38 @@ func TestMultiStream_OpsNotSilentStreamOne(t *testing.T) { t.Errorf("[P1] 14.3-INTG-002: --ops silently emits only stream 1's operators; a multi-stream page must emit all streams' operators or a distinct trailing truncation meta record (AC4):\n%s", stdout) } } + +// --------------------------------------------------------------------------- +// 14.3-INTG-002 [P1] AC4 (--raw surface): `dump stream --page 1 --raw` on a +// multi-stream page must not present stream 1's bytes as the whole content +// stream with no signal. --raw's stdout stays a verbatim byte dump (an inline +// marker would corrupt it), so the truncation must be disclosed on STDERR. +// GREEN: stdout is stream 1 only (byte-exact, no stream-2 content) AND stderr +// carries the truncation note; exit 0. Guards the floor path's --raw surface, +// which is otherwise a silent truncation. +// --------------------------------------------------------------------------- + +func TestMultiStream_RawSignalsTruncationOnStderr(t *testing.T) { + bin := buildCLI(t) + f := fixturePath(t, "multi-content-stream.pdf") + + stdout, stderr, ec := runCLI(t, bin, "dump", "stream", "--page", "1", "--raw", f) + if ec != 0 { + t.Fatalf("[P1] 14.3-INTG-002 (--raw): must exit 0, got %d\nstderr: %s", ec, stderr) + } + + // stdout stays a byte-exact dump of stream 1 ONLY: stream 1's `cm` is present, + // and no stream-2 content (`BT`, `Hello`) leaks in or corrupts the bytes. + if !strings.Contains(stdout, "cm") { + t.Errorf("[P1] 14.3-INTG-002 (--raw): stdout missing stream 1 content (expected `cm`): %q", stdout) + } + if strings.Contains(stdout, "Hello") || strings.Contains(stdout, "BT") { + t.Errorf("[P1] 14.3-INTG-002 (--raw): stdout leaked stream 2 content; --raw must dump only the first decoded stream: %q", stdout) + } + + // The truncation must be disclosed on stderr - the only channel that does not + // corrupt the raw byte stream. Without it, --raw is a silent truncation. + if !strings.Contains(stderr, "truncated") || !strings.Contains(stderr, "of 2") { + t.Errorf("[P1] 14.3-INTG-002 (--raw): multi-stream truncation must be disclosed on stderr (expected a `truncated ... of 2` note), got: %q", stderr) + } +} From bfbb976afd01df741a3dbe6b716b8d996c1371c7 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 25 Jul 2026 19:03:49 +0700 Subject: [PATCH 03/11] feat: add GetPageContentStream for full multi-stream page content --- internal/pdfcore/stream.go | 126 +++++++++++++++++++++++--------- internal/pdfcore/stream_test.go | 36 +++++---- 2 files changed, 110 insertions(+), 52 deletions(-) diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index 8c0d5f3..efda53b 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -7,21 +7,18 @@ import ( pdfcpu_types "github.com/pdfcpu/pdfcpu/pkg/pdfcpu/types" ) -// GetPageContentStreamRef resolves a 1-based page number to its content -// stream's node ID AND the number of content streams in its /Contents entry, in -// a SINGLE page-dict resolution. Node ID is empty (no error) when the page has -// no /Contents; for an array it is the first ref's node ID (error if the first -// element is not an indirect reference). streamCount is 0 with no /Contents, 1 -// for a single indirect ref, and for an array the count of its indirect-ref -// elements only - a degenerate null / non-ref element contributes no stream per -// ISO 32000-1 7.8.2, so counting len(v) would falsely report "showing stream 1 -// of N" for e.g. [ref null] where the single stream is shown in full. Combining -// the two reads keeps the Story 14.3 multi-stream truncation marker from -// resolving (and cache-mutating) the page dict twice per dump. -func (ins *Inspector) GetPageContentStreamRef(tabID string, pageNum int) (nodeID string, streamCount int, err error) { +// pageContentStreamNodeIDs resolves a 1-based page number to the ordered node +// IDs of its content stream(s) in a SINGLE page-dict resolution: empty slice +// (no error) when the page has no /Contents, one ID for a single indirect ref, +// and, for an array, every indirect-ref element in order (a degenerate null / +// non-ref element is skipped per ISO 32000-1 7.8.2 - it is not a stream). An +// array whose FIRST element is not an indirect ref is an error (preserving the +// GetPageContentStreamNodeID contract). The order is the concatenation order: +// a page's content is the join of these streams (7.8.2). +func (ins *Inspector) pageContentStreamNodeIDs(tabID string, pageNum int) ([]string, error) { doc, err := ins.GetDocument(tabID) if err != nil { - return "", 0, err + return nil, err } // PageDict mutates the pdfcpu page-resolution cache; serialize. doc.pdfMu.Lock() @@ -34,49 +31,112 @@ func (ins *Inspector) GetPageContentStreamRef(tabID string, pageNum int) (nodeID return e }) if err != nil { - return "", 0, wrapPDFError(err) + return nil, wrapPDFError(err) } if pageDict == nil { - return "", 0, nil + return nil, nil } contents, found := pageDict.Find("Contents") if !found || contents == nil { - return "", 0, nil + return nil, nil } switch v := contents.(type) { case pdfcpu_types.IndirectRef: - return fmt.Sprintf("obj:%d:%d", v.GenerationNumber.Value(), v.ObjectNumber.Value()), 1, nil + return []string{fmt.Sprintf("obj:%d:%d", v.GenerationNumber.Value(), v.ObjectNumber.Value())}, nil case pdfcpu_types.Array: if len(v) == 0 { - return "", 0, nil + return nil, nil + } + if _, ok := v[0].(pdfcpu_types.IndirectRef); !ok { + return nil, fmt.Errorf("contents array element is not an indirect reference") } - count := 0 + ids := make([]string, 0, len(v)) for _, e := range v { - if _, ok := e.(pdfcpu_types.IndirectRef); ok { - count++ + ref, ok := e.(pdfcpu_types.IndirectRef) + if !ok { + continue // null / non-ref element: not a stream, skip } + ids = append(ids, fmt.Sprintf("obj:%d:%d", ref.GenerationNumber.Value(), ref.ObjectNumber.Value())) } - ref, ok := v[0].(pdfcpu_types.IndirectRef) - if !ok { - return "", 0, fmt.Errorf("contents array element is not an indirect reference") - } - return fmt.Sprintf("obj:%d:%d", ref.GenerationNumber.Value(), ref.ObjectNumber.Value()), count, nil + return ids, nil default: - return "", 0, fmt.Errorf("unexpected Contents type: %T", contents) + return nil, fmt.Errorf("unexpected Contents type: %T", contents) } } // GetPageContentStreamNodeID resolves a 1-based page number to the node ID of -// its content stream. Returns empty string (no error) when the page has no -// Contents entry. For pages with an array of content stream refs, returns the -// first ref's node ID. Thin wrapper over GetPageContentStreamRef that discards -// the stream count (bound in pdfservice.Service; signature preserved). +// its FIRST content stream. Returns empty string (no error) when the page has +// no Contents entry. Bound in pdfservice.Service (GoToPage), so its signature +// is preserved; callers wanting the whole page content use GetPageContentStream. func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (string, error) { - nodeID, _, err := ins.GetPageContentStreamRef(tabID, pageNum) - return nodeID, err + ids, err := ins.pageContentStreamNodeIDs(tabID, pageNum) + if err != nil { + return "", err + } + if len(ids) == 0 { + return "", nil + } + return ids[0], nil +} + +// GetPageContentStreamRef returns the page's FIRST content-stream node ID and +// the count of content streams in its /Contents entry. Thin adapter over +// pageContentStreamNodeIDs, used by the CLI page-stream resolver's truncation +// marker. +func (ins *Inspector) GetPageContentStreamRef(tabID string, pageNum int) (nodeID string, streamCount int, err error) { + ids, err := ins.pageContentStreamNodeIDs(tabID, pageNum) + if err != nil { + return "", 0, err + } + if len(ids) == 0 { + return "", 0, nil + } + return ids[0], len(ids), nil +} + +// GetPageContentStream returns the page's COMPLETE content stream: the decoded +// content of every stream in its /Contents entry, concatenated in array order +// and joined by a single newline (ISO 32000-1 7.8.2 requires whitespace between +// streams so a token spanning a boundary - e.g. an operator split across two +// streams, or `q` in stream 1 with its `Q` in stream 2 - does not fuse), then +// tokenized and formatted as ONE program. Returns nil (no error) when the page +// has no /Contents. A per-stream decode failure surfaces as a ContentStreamData +// carrying Error (not a Go error), matching GetContentStream. The returned +// NodeID is the first stream's node (a representative anchor). +func (ins *Inspector) GetPageContentStream(tabID string, pageNum int) (*ContentStreamData, error) { + ids, err := ins.pageContentStreamNodeIDs(tabID, pageNum) + if err != nil { + return nil, err + } + if len(ids) == 0 { + return nil, nil + } + + // Decode each stream via GetContentStream (which locks pdfMu per call, so we + // must NOT hold pdfMu here - pageContentStreamNodeIDs already released it). + raws := make([]string, 0, len(ids)) + for _, id := range ids { + cs, err := ins.GetContentStream(tabID, id) + if err != nil { + return nil, err + } + if cs.Error != "" { + // Surface the first stream's decode/type failure verbatim. + return &ContentStreamData{NodeID: id, Error: cs.Error}, nil + } + raws = append(raws, cs.Raw) + } + + combined := strings.Join(raws, "\n") + result := &ContentStreamData{NodeID: ids[0], Raw: combined} + if combined != "" { + result.Tokenized = tokenizeContentStream(combined) + result.Formatted = Format(result.Tokenized) + } + return result, nil } // GetPageNode resolves a 1-based page number to a fully-populated TreeNode for diff --git a/internal/pdfcore/stream_test.go b/internal/pdfcore/stream_test.go index c167003..8c52328 100644 --- a/internal/pdfcore/stream_test.go +++ b/internal/pdfcore/stream_test.go @@ -403,34 +403,31 @@ func TestGetPageContentStreamNodeID_NoContentsEntry(t *testing.T) { } // --------------------------------------------------------------------------- -// 14.3-UNIT-002 [P1] AC3 (Story 14.3, Code Review #1 fix): the anti-false- -// positive negative path of GetPageContentStreamRef's stream count. +// 14.3-UNIT-002 [P1] AC3 (Story 14.3): pageContentStreamNodeIDs enumerates the +// page's content-stream refs (the concatenation order/set). // -// The multi-stream truncation marker fires only when streamCount >= 2. Per ISO -// 32000-1 7.8.2 a page's content is the concatenation of its /Contents array's -// STREAM refs, so a degenerate null / non-ref element contributes NO stream. -// Before the code-review fix the body returned len(v), so `[ref null]` reported -// streamCount 2 and the CLI falsely marked a single, fully-shown stream as -// "stream 1 of 2, truncated". This test pins the fixed contract: a single -// indirect ref, a one-element array `[ref]`, and the degenerate `[ref null]` -// all count 1 (no marker); only a genuine multi-ref array `[ref ref]` counts 2; -// a page with no /Contents counts 0. +// Per ISO 32000-1 7.8.2 a page's content is the concatenation of its /Contents +// array's STREAM refs, so a degenerate null / non-ref element contributes NO +// stream and must be skipped (not counted, not concatenated). This test pins +// that: a single indirect ref, a one-element array `[ref]`, and the degenerate +// `[ref null]` all enumerate to 1 stream; a genuine multi-ref array `[ref ref]` +// enumerates 2; a page with no /Contents enumerates 0. // // The `[ref null]` case is built by an in-memory page-dict mutation, not a disk // fixture: pdfcpu rejects an on-disk /Contents array containing a null element // at read time (DereferenceStreamDict: wrong type ), so the only way to -// drive that degenerate array into GetPageContentStreamRef is to inject it -// after a valid open. This still exercises the production count loop verbatim. +// drive that degenerate array into pageContentStreamNodeIDs is to inject it +// after a valid open. This still exercises the production enumeration verbatim. // --------------------------------------------------------------------------- // contentStreamObj builds a minimal, valid content-stream object body numbered -// n for the GetPageContentStreamRef count fixtures. +// n for the pageContentStreamNodeIDs fixtures. func contentStreamObj(n int) string { body := "BT /F1 12 Tf 100 700 Td (x) Tj ET" return fmt.Sprintf("%d 0 obj\n<< /Length %d >>\nstream\n%s\nendstream\nendobj\n", n, len(body), body) } -func TestGetPageContentStreamRefCount(t *testing.T) { +func TestPageContentStreamNodeIDs(t *testing.T) { catalog := "1 0 obj\n<< /Type /Catalog /Pages 2 0 R >>\nendobj\n" pages := "2 0 obj\n<< /Type /Pages /Kids [3 0 R] /Count 1 >>\nendobj\n" @@ -469,7 +466,8 @@ func TestGetPageContentStreamRefCount(t *testing.T) { t.Run(tc.name, func(t *testing.T) { objs := append([]string{catalog, pages, tc.page}, tc.objs...) ins, tabID := writeTempPDF(t, "count.pdf", assembleDiffPDF(1, objs...)) - _, got, err := ins.GetPageContentStreamRef(tabID, 1) + ids, err := ins.pageContentStreamNodeIDs(tabID, 1) + got := len(ids) if err != nil { t.Fatalf("[14.3-UNIT-002] %s: unexpected error: %v", tc.name, err) } @@ -506,12 +504,12 @@ func TestGetPageContentStreamRefCount(t *testing.T) { // Inject the degenerate [ref null] shape the fix guards against. pageDict["Contents"] = pdfcpu_types.Array{ref, nil} - _, got, err := ins.GetPageContentStreamRef(tabID, 1) + ids, err := ins.pageContentStreamNodeIDs(tabID, 1) if err != nil { t.Fatalf("[14.3-UNIT-002] unexpected error: %v", err) } - if got != 1 { - t.Errorf("[14.3-UNIT-002] [ref null] count = %d, want 1 (a null element is not a stream; counting it would fire a false truncation marker)", got) + if got := len(ids); got != 1 { + t.Errorf("[14.3-UNIT-002] [ref null] stream count = %d, want 1 (a null element is not a stream, so it is skipped in concatenation)", got) } }) } From 9f58e34ffe28c1ca4923e217f86d380014ade488 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 25 Jul 2026 19:03:49 +0700 Subject: [PATCH 04/11] refactor: concatenate page streams in dump stream; drop floor marker --- cmd/cli/cmd_stream.go | 176 +++++++----------- internal/pdfcore/model.go | 25 +-- internal/pdfcore/stream.go | 15 -- .../multistream_test.go | 29 ++- 4 files changed, 87 insertions(+), 158 deletions(-) diff --git a/cmd/cli/cmd_stream.go b/cmd/cli/cmd_stream.go index 664e875..698a153 100644 --- a/cmd/cli/cmd_stream.go +++ b/cmd/cli/cmd_stream.go @@ -137,70 +137,47 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { } defer func() { _ = inspector.Close("cli") }() - // Resolve the content-stream nodeID and, for --ops, the page number whose - // resources back Do classification (0 = no page-backed Do resolution), plus - // the /Contents array length for the multi-stream truncation marker. - nodeID, opsPageNum, streamCount, code := resolveStreamNode(inspector, info, flags) + // Resolve the input mode: --ref/--xobject address a single stream; the page + // mode (opsPageNum > 0) resolves to the page's COMPLETE content, and the + // page number also backs Do classification under --ops. + nodeID, opsPageNum, code := resolveStreamNode(inspector, info, flags) if code != 0 { return code } - // No Contents entry on a page: valid PDF, just no stream. - if nodeID == "" { - if flags.ops { - // NDJSON contract: zero lines on stdout, condition on stderr, exit 0. - fmt.Fprintln(os.Stderr, "page has no content stream") - return 0 + var result *pdfcore.ContentStreamData + if opsPageNum > 0 { + // Page mode: a page's content is the CONCATENATION of every stream in its + // /Contents array (ISO 32000-1 7.8.2), assembled and tokenized as one + // program - no stream is dropped or presented as if it were the whole. + r, err := inspector.GetPageContentStream("cli", opsPageNum) + if err != nil { + writeJSONError(os.Stderr, err.Error()) + return 2 } - result := &pdfcore.ContentStreamData{Raw: "", Error: "page has no content stream"} - if flags.json { - if err := emit(os.Stdout, result, flags.pretty); err != nil { - writeJSONError(os.Stderr, fmt.Sprintf("failed to write output: %v", err)) - return 2 - } - return 0 + if r == nil { + // No /Contents on the page: valid PDF, just no stream. + return writeNoContentStream(flags) } - // Plain default: surface the no-stream condition as a one-line note (the - // page is valid, just has no /Contents). - if _, err := io.WriteString(os.Stdout, "(page has no content stream)\n"); err != nil { - fmt.Fprintf(os.Stderr, "failed to write output: %v\n", err) + result = r + } else { + // --ref / --xobject: a single addressed stream. + r, err := inspector.GetContentStream("cli", nodeID) + if err != nil { + writeJSONError(os.Stderr, err.Error()) return 2 } - return 0 + result = r } - result, err := inspector.GetContentStream("cli", nodeID) - if err != nil { - writeJSONError(os.Stderr, err.Error()) - return 2 - } - // GetContentStream returns a non-error ContentStreamData{Error:...} for - // "node is not a stream object" and decode failures. Map that to exit 2. + // GetContentStream/GetPageContentStream return a non-error ContentStreamData{ + // Error:...} for "node is not a stream object" and decode failures. exit 2. if result.Error != "" { writeJSONError(os.Stderr, result.Error) return 2 } - // Multi-stream truncation marker (Story 14.3 AC3/AC4, floor path): when the - // page's /Contents array holds more than one stream, only the first was - // decoded, so mark the result partial. A single stream (streamCount <= 1) is - // complete and carries no marker. --raw stays a verbatim byte dump of the one - // decoded stream (a marker would corrupt the bytes); the note rides plain - // text, --json, and --ops instead. - if streamCount > 1 { - result.StreamCount = streamCount - result.Shown = 1 - result.Truncated = true - } - if flags.raw { - // A marker cannot ride stdout without corrupting the verbatim byte dump, - // so on a multi-stream page disclose the truncation on STDERR instead - - // otherwise --raw would present stream 1's bytes as the whole content - // stream with no signal on any channel (Story 14.3 AC4, --raw surface). - if result.Truncated { - fmt.Fprint(os.Stderr, multiStreamTruncationNote(result)) - } if _, err := io.WriteString(os.Stdout, result.Raw); err != nil { fmt.Fprintf(os.Stderr, "failed to write raw output: %v\n", err) return 2 @@ -227,12 +204,28 @@ func execStreamDump(filePath string, flags streamFlags) (exitCode int) { return 0 } -// multiStreamTruncationNote is the one-line note disclosing that a multi-stream -// page's /Contents was truncated to its first decoded stream (Story 14.3 floor -// path). Shared by the plain-text (stdout) and --raw (stderr) surfaces so their -// wording cannot drift; callers guard on result.Truncated before emitting it. -func multiStreamTruncationNote(result *pdfcore.ContentStreamData) string { - return fmt.Sprintf("(truncated: showing stream %d of %d; page /Contents is a multi-stream array)\n", result.Shown, result.StreamCount) +// writeNoContentStream renders the "page has no /Contents" condition on the +// selected surface and exits 0 (a valid page, just no stream): zero NDJSON +// lines with the note on stderr for --ops, a JSON object carrying the note for +// --json, and a one-line note on stdout otherwise (plain and --raw). +func writeNoContentStream(flags streamFlags) int { + if flags.ops { + fmt.Fprintln(os.Stderr, "page has no content stream") + return 0 + } + if flags.json { + result := &pdfcore.ContentStreamData{Raw: "", Error: "page has no content stream"} + if err := emit(os.Stdout, result, flags.pretty); err != nil { + writeJSONError(os.Stderr, fmt.Sprintf("failed to write output: %v", err)) + return 2 + } + return 0 + } + if _, err := io.WriteString(os.Stdout, "(page has no content stream)\n"); err != nil { + fmt.Fprintf(os.Stderr, "failed to write output: %v\n", err) + return 2 + } + return 0 } // printStreamPlain renders the decoded content stream as a human-readable @@ -241,15 +234,6 @@ func multiStreamTruncationNote(result *pdfcore.ContentStreamData) string { // use --json for structured operators, --ops for NDJSON, --raw for bytes. func printStreamPlain(out io.Writer, result *pdfcore.ContentStreamData) error { var b strings.Builder - // Multi-stream truncation note (Story 14.3 AC3, floor path): a one-line - // header so the human reader is not shown a partial (often unbalanced) - // program as if it were the whole content stream. Emitted BEFORE the - // empty-stream early return: a multi-stream page whose first stream decodes - // to zero operators must still disclose that streams 2..N exist, otherwise - // the "(empty content stream)" line would be a silent truncation. - if result.Truncated { - b.WriteString(multiStreamTruncationNote(result)) - } // A content-stream object can exist yet decode to zero operators (an empty // /Contents stream). Surface that as a one-line note so plain output is never // a silent zero-byte write and always ends with a newline. @@ -276,60 +260,53 @@ func printStreamPlain(out io.Writer, result *pdfcore.ContentStreamData) error { return err } -// resolveStreamNode maps the input flags to a content-stream nodeID. It returns -// the nodeID (or "" when a page has no /Contents), the page number to back Do -// classification under --ops (0 when not a page stream), the number of streams -// in the page's /Contents array (Story 14.3 multi-stream marker; 0 for the -// --ref/--xobject modes and single-stream pages that need no marker), and a -// non-zero exit code on error (already reported to stderr). -func resolveStreamNode(inspector *pdfcore.Inspector, info *pdfcore.DocumentInfo, flags streamFlags) (nodeID string, opsPageNum int, streamCount int, code int) { +// resolveStreamNode maps the input flags to a content-stream target. For +// --ref/--xobject it returns the addressed stream's nodeID with opsPageNum 0. +// For page mode it returns nodeID "" and opsPageNum = the (validated) page +// number: the caller fetches the page's complete content via +// GetPageContentStream (a single page-dict resolution), and opsPageNum also +// backs Do classification under --ops. code is non-zero on error (reported). +func resolveStreamNode(inspector *pdfcore.Inspector, info *pdfcore.DocumentInfo, flags streamFlags) (nodeID string, opsPageNum int, code int) { switch { case flags.xobject != "": ownerNodeID, c := xobjectOwnerNodeID(inspector, info, flags) if c != 0 { - return "", 0, 0, c + return "", 0, c } resources, err := inspector.GetXObjectResources("cli", ownerNodeID) if err != nil { writeJSONError(os.Stderr, err.Error()) - return "", 0, 0, 2 + return "", 0, 2 } entry, ok := resources[flags.xobject] if !ok || entry.NodeID == "" { writeJSONError(os.Stderr, fmt.Sprintf("XObject %q not found in resources", flags.xobject)) - return "", 0, 0, 2 + return "", 0, 2 } // A resolved form/image XObject stream has no page; Do classification is // page-scoped only (Decision: --ops resourceType only for page streams). - return entry.NodeID, 0, 0, 0 + return entry.NodeID, 0, 0 case flags.ref != "": objNum, genNum, err := parseObjectRef(flags.ref) if err != nil { writeJSONError(os.Stderr, err.Error()) - return "", 0, 0, 1 + return "", 0, 1 } - return fmt.Sprintf("obj:%d:%d", genNum, objNum), 0, 0, 0 + return fmt.Sprintf("obj:%d:%d", genNum, objNum), 0, 0 default: - // Page content stream. + // Page mode: validate the range only. GetPageContentStream (caller) + // resolves and concatenates the page's content in a single page-dict pass. if info.PageCount == 0 { writeJSONError(os.Stderr, "cannot determine page count for this PDF") - return "", 0, 0, 2 + return "", 0, 2 } if flags.page > info.PageCount { writeJSONError(os.Stderr, fmt.Sprintf("page %d out of range: document has %d pages", flags.page, info.PageCount)) - return "", 0, 0, 2 + return "", 0, 2 } - // One call resolves the page dict once and returns both the nodeID and - // the /Contents array length (the count surfaces the multi-stream - // truncation marker; only the first stream is decoded on the floor path). - id, count, err := inspector.GetPageContentStreamRef("cli", flags.page) - if err != nil { - writeJSONError(os.Stderr, err.Error()) - return "", 0, 0, 2 - } - return id, flags.page, count, 0 + return "", flags.page, 0 } } @@ -423,32 +400,9 @@ func emitOps(inspector *pdfcore.Inspector, result *pdfcore.ContentStreamData, pa } } - // Multi-stream truncation marker (Story 14.3 AC4): NDJSON has no envelope - // and Story 14-1 pins --ops to one JSON object PER OPERATOR, so the marker - // rides a DISTINCT trailing meta record with NO "op" key (a phantom - // {"op":""} record would breach that contract). It is emitted only for the - // floor path (a genuinely multi-stream page); the operator records above are - // stream 1's alone. - if result.Truncated { - meta := opsTruncationMeta{Truncated: true, StreamCount: result.StreamCount, Shown: result.Shown} - if err := enc.Encode(meta); err != nil { - fmt.Fprintf(os.Stderr, "failed to write NDJSON output: %v\n", err) - return 2 - } - } return 0 } -// opsTruncationMeta is the trailing --ops NDJSON meta record for a multi-stream -// page (Story 14.3 AC4). It deliberately has NO Op field so consumers keying on -// "op" skip it as a non-operator record, and it carries the /Contents array -// length so a script sees that only Shown of StreamCount streams were emitted. -type opsTruncationMeta struct { - Truncated bool `json:"truncated"` - StreamCount int `json:"streamCount"` - Shown int `json:"shown"` -} - // classifyDo attaches resourceType + objectRef to a Do op when its name operand // resolves to a page XObject whose /Subtype is /Image or /Form. A name that // does not resolve, or whose /Subtype is neither, leaves the op unannotated diff --git a/internal/pdfcore/model.go b/internal/pdfcore/model.go index ad928af..53119aa 100644 --- a/internal/pdfcore/model.go +++ b/internal/pdfcore/model.go @@ -59,23 +59,16 @@ type ValueEntry struct { RefTarget string `json:"refTarget"` } -// ContentStreamData holds raw and tokenized content stream data for a page. -// -// StreamCount/Truncated/Shown are the Story 14.3 multi-stream truncation marker -// (AC3/AC4): when a page's /Contents is an array of more than one stream, the -// CLI decodes only the first and sets StreamCount to the array length, Shown to -// 1, and Truncated true so no consumer mistakes the partial (often unbalanced) -// program for the whole content stream. All three are zero/false - and omitted -// from JSON - for single-stream pages and non-page streams. +// ContentStreamData holds raw and tokenized content stream data for a page. For +// a multi-stream page (/Contents is an array) the CLI assembles this from the +// concatenation of every stream (see Inspector.GetPageContentStream), so Raw / +// Tokenized / Formatted already cover the whole page content. type ContentStreamData struct { - NodeID string `json:"nodeId"` - Raw string `json:"raw"` - Tokenized []Token `json:"tokenized"` - Formatted []FormattedLine `json:"formatted"` - Error string `json:"error"` - StreamCount int `json:"streamCount,omitempty"` - Shown int `json:"shown,omitempty"` - Truncated bool `json:"truncated,omitempty"` + NodeID string `json:"nodeId"` + Raw string `json:"raw"` + Tokenized []Token `json:"tokenized"` + Formatted []FormattedLine `json:"formatted"` + Error string `json:"error"` } // FormattedLine is one logical PDF operation in a content stream: zero or more diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index efda53b..3e9b981 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -82,21 +82,6 @@ func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (str return ids[0], nil } -// GetPageContentStreamRef returns the page's FIRST content-stream node ID and -// the count of content streams in its /Contents entry. Thin adapter over -// pageContentStreamNodeIDs, used by the CLI page-stream resolver's truncation -// marker. -func (ins *Inspector) GetPageContentStreamRef(tabID string, pageNum int) (nodeID string, streamCount int, err error) { - ids, err := ins.pageContentStreamNodeIDs(tabID, pageNum) - if err != nil { - return "", 0, err - } - if len(ids) == 0 { - return "", 0, nil - } - return ids[0], len(ids), nil -} - // GetPageContentStream returns the page's COMPLETE content stream: the decoded // content of every stream in its /Contents entry, concatenated in array order // and joined by a single newline (ISO 32000-1 7.8.2 requires whitespace between diff --git a/tests/14-3-no-silent-truncation/multistream_test.go b/tests/14-3-no-silent-truncation/multistream_test.go index d591350..58bd644 100644 --- a/tests/14-3-no-silent-truncation/multistream_test.go +++ b/tests/14-3-no-silent-truncation/multistream_test.go @@ -136,16 +136,14 @@ func TestMultiStream_OpsNotSilentStreamOne(t *testing.T) { } // --------------------------------------------------------------------------- -// 14.3-INTG-002 [P1] AC4 (--raw surface): `dump stream --page 1 --raw` on a -// multi-stream page must not present stream 1's bytes as the whole content -// stream with no signal. --raw's stdout stays a verbatim byte dump (an inline -// marker would corrupt it), so the truncation must be disclosed on STDERR. -// GREEN: stdout is stream 1 only (byte-exact, no stream-2 content) AND stderr -// carries the truncation note; exit 0. Guards the floor path's --raw surface, -// which is otherwise a silent truncation. +// 14.3-INTG-002 [P1] AC3/AC4 (--raw surface): `dump stream --page 1 --raw` on a +// multi-stream page must dump the CONCATENATION of all streams' decoded bytes +// (ISO 32000-1 7.8.2), not just stream 1. GREEN: stdout carries content from +// BOTH stream 1 (`cm`) and stream 2 (`Hello`), exit 0, and no truncation note +// (nothing was dropped). Guards that --raw is not a silent partial. // --------------------------------------------------------------------------- -func TestMultiStream_RawSignalsTruncationOnStderr(t *testing.T) { +func TestMultiStream_RawConcatenatesAllStreams(t *testing.T) { bin := buildCLI(t) f := fixturePath(t, "multi-content-stream.pdf") @@ -154,18 +152,17 @@ func TestMultiStream_RawSignalsTruncationOnStderr(t *testing.T) { t.Fatalf("[P1] 14.3-INTG-002 (--raw): must exit 0, got %d\nstderr: %s", ec, stderr) } - // stdout stays a byte-exact dump of stream 1 ONLY: stream 1's `cm` is present, - // and no stream-2 content (`BT`, `Hello`) leaks in or corrupts the bytes. + // stdout must carry BOTH streams' decoded bytes: stream 1's `cm` and stream + // 2's `Hello`/`Q`, joined per 7.8.2. if !strings.Contains(stdout, "cm") { t.Errorf("[P1] 14.3-INTG-002 (--raw): stdout missing stream 1 content (expected `cm`): %q", stdout) } - if strings.Contains(stdout, "Hello") || strings.Contains(stdout, "BT") { - t.Errorf("[P1] 14.3-INTG-002 (--raw): stdout leaked stream 2 content; --raw must dump only the first decoded stream: %q", stdout) + if !strings.Contains(stdout, "Hello") || !strings.Contains(stdout, "Q") { + t.Errorf("[P1] 14.3-INTG-002 (--raw): stdout missing stream 2 content; --raw must dump the concatenation of all streams (expected `Hello` and `Q`): %q", stdout) } - // The truncation must be disclosed on stderr - the only channel that does not - // corrupt the raw byte stream. Without it, --raw is a silent truncation. - if !strings.Contains(stderr, "truncated") || !strings.Contains(stderr, "of 2") { - t.Errorf("[P1] 14.3-INTG-002 (--raw): multi-stream truncation must be disclosed on stderr (expected a `truncated ... of 2` note), got: %q", stderr) + // Nothing was truncated, so no truncation note should appear on either channel. + if strings.Contains(stdout, "truncated") || strings.Contains(stderr, "truncated") { + t.Errorf("[P1] 14.3-INTG-002 (--raw): unexpected truncation note after full concatenation\nstdout: %q\nstderr: %q", stdout, stderr) } } From 65205f93b69fc1944b9bf008302ae892962dee93 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 25 Jul 2026 19:50:19 +0700 Subject: [PATCH 05/11] test: assert pdfMu lock in the helper a delegating method calls --- .../inspector_concurrency_test.go | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/tests/10-5-inspector-concurrency-lifecycle-and-safecall-sev1/inspector_concurrency_test.go b/tests/10-5-inspector-concurrency-lifecycle-and-safecall-sev1/inspector_concurrency_test.go index 3c8ad9a..bfe8685 100644 --- a/tests/10-5-inspector-concurrency-lifecycle-and-safecall-sev1/inspector_concurrency_test.go +++ b/tests/10-5-inspector-concurrency-lifecycle-and-safecall-sev1/inspector_concurrency_test.go @@ -134,12 +134,26 @@ var methodFileMap = map[string]string{ "GetXRefTable": "internal/pdfcore/xreftable.go", } +// pdfMuLockOwner maps a method that only DELEGATES to the helper that performs +// the locked work, so the grep looks where the mutex actually lives. The +// guarantee asserted is unchanged (the page-dict resolution happens under +// doc.pdfMu); it is just one call deeper. GetPageContentStreamNodeID resolves +// through pageContentStreamNodeIDs, which locks, so that both it and +// GetPageContentStream share one page-dict resolution. +var pdfMuLockOwner = map[string]string{ + "GetPageContentStreamNodeID": "pageContentStreamNodeIDs", +} + // Test_10_5_AC1_MethodsAcquirePdfMu [P0] AC#1: every method in // pdfMuRequiredMethods MUST contain a `doc.pdfMu.Lock()` call and a // `defer doc.pdfMu.Unlock()` immediately after the `GetDocument` call. // We assert the two substrings appear in the function body via a // per-method file read + regex scoped to the function range. // +// A method listed in pdfMuLockOwner is checked in two steps instead: it must +// call its declared helper, and the HELPER must carry the lock pattern. A +// method that neither locks nor delegates still fails. +// // The function boundary is approximated by `func (ins *Inspector) (` // up to the next `func (` at the same depth -- adequate for grep purposes. func Test_10_5_AC1_MethodsAcquirePdfMu(t *testing.T) { @@ -150,15 +164,26 @@ func Test_10_5_AC1_MethodsAcquirePdfMu(t *testing.T) { t.Fatalf("internal test bug: methodFileMap missing %q", method) } src := readSource(t, path) - body := extractFunctionBody(t, src, method) + target := method + if owner, ok := pdfMuLockOwner[method]; ok { + caller := extractFunctionBody(t, src, method) + if caller == "" { + t.Fatalf("[P0] 10-5-AC1: could not locate `func (ins *Inspector) %s(` in %s", method, path) + } + if !strings.Contains(caller, owner+"(") { + t.Errorf("[P0] 10-5-AC1: %s in %s must either lock doc.pdfMu itself or delegate to %s, which does", method, path, owner) + } + target = owner + } + body := extractFunctionBody(t, src, target) if body == "" { - t.Fatalf("[P0] 10-5-AC1: could not locate `func (ins *Inspector) %s(` in %s", method, path) + t.Fatalf("[P0] 10-5-AC1: could not locate `func (ins *Inspector) %s(` in %s", target, path) } if !strings.Contains(body, "doc.pdfMu.Lock()") { - t.Errorf("[P0] 10-5-AC1: %s in %s must call `doc.pdfMu.Lock()` (AC1: acquire per-document mutex immediately after GetDocument)", method, path) + t.Errorf("[P0] 10-5-AC1: %s in %s must call `doc.pdfMu.Lock()` (AC1: acquire per-document mutex immediately after GetDocument)", target, path) } if !strings.Contains(body, "defer doc.pdfMu.Unlock()") { - t.Errorf("[P0] 10-5-AC1: %s in %s must call `defer doc.pdfMu.Unlock()` (AC1: deferred Unlock pattern)", method, path) + t.Errorf("[P0] 10-5-AC1: %s in %s must call `defer doc.pdfMu.Unlock()` (AC1: deferred Unlock pattern)", target, path) } }) } From 79768fafdea5cad1a478a5e77cf5a899a2f97246 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 25 Jul 2026 19:50:19 +0700 Subject: [PATCH 06/11] chore: correct stale comments on failing-stream NodeID and truncation test --- .../components/DiffView.truncation.test.tsx | 28 ++++++++----------- internal/pdfcore/stream.go | 9 ++++-- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/DiffView.truncation.test.tsx b/frontend/src/components/DiffView.truncation.test.tsx index 76dfdba..5573921 100644 --- a/frontend/src/components/DiffView.truncation.test.tsx +++ b/frontend/src/components/DiffView.truncation.test.tsx @@ -1,22 +1,16 @@ /** * Story 14.3: DiffView depth-cap truncation display branch (AC5, 14.3-COMP-001). * - * RED PHASE: DiffView's `identical` const (DiffView.tsx) mirrors Go's - * diffIsIdentical over the node counts + document flags only; it does not yet - * account for `summary.truncatedSubtrees`. Given a result whose walk was bounded - * by the depth cap (truncatedSubtrees > 0) but whose visible node counts are all - * zero, the component today computes identical === true and renders the - * "No structural differences" banner with NO truncation marker -- the exact - * quiet lie this story closes, mirrored on the GUI surface. + * DiffView's `identical` const (DiffView.tsx) mirrors Go's diffIsIdentical, and + * that includes `summary.truncatedSubtrees === 0`. Given a result whose walk was + * bounded by the depth cap (truncatedSubtrees > 0) but whose visible node counts + * are all zero, the component must NOT compute identical === true and must NOT + * render the "No structural differences" banner without a truncation marker -- + * the quiet lie this story closes, mirrored on the GUI surface. * - * GREEN target: `identical` gains `&& s.truncatedSubtrees === 0`, so the banner - * is suppressed, and a depth-cap marker is rendered so the bounded walk is - * visible. This is the thin display branch of a backend-verified field, kept at - * the component level (NOT E2E). - * - * Test files are excluded from the app tsc build, so the `truncatedSubtrees` - * field (not yet on DiffSummaryData) does not break `npm run typecheck`; only - * vitest exercises this file. + * Before the fix these cases were red: `identical` ignored truncatedSubtrees, so + * the banner appeared on a bounded walk. This is the thin display branch of a + * backend-verified field, kept at the component level (NOT E2E). * * Naming: 14.3-COMP-001 [P1]. * Run: cd frontend && npx vitest run src/components/DiffView.truncation.test.tsx @@ -49,8 +43,8 @@ const depthCappedResult = { encryptionChanged: false, infoChanged: false, xmpChanged: false, - // Additive field surfaced by the Go DiffSummary (AC2). Cast through unknown - // because DiffSummaryData does not declare it yet (red-phase seam). + // Additive field surfaced by the Go DiffSummary (AC2); declared on + // DiffSummaryData in DiffView.tsx. truncatedSubtrees: 1, }, root: { diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index 3e9b981..9e3ce47 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -89,8 +89,10 @@ func (ins *Inspector) GetPageContentStreamNodeID(tabID string, pageNum int) (str // streams, or `q` in stream 1 with its `Q` in stream 2 - does not fuse), then // tokenized and formatted as ONE program. Returns nil (no error) when the page // has no /Contents. A per-stream decode failure surfaces as a ContentStreamData -// carrying Error (not a Go error), matching GetContentStream. The returned -// NodeID is the first stream's node (a representative anchor). +// carrying Error (not a Go error), matching GetContentStream. On success the +// returned NodeID is the first stream's node (a representative anchor); on a +// decode failure it is the node of the stream that FAILED, which may be any +// element of the array, so the caller can tell which one broke. func (ins *Inspector) GetPageContentStream(tabID string, pageNum int) (*ContentStreamData, error) { ids, err := ins.pageContentStreamNodeIDs(tabID, pageNum) if err != nil { @@ -109,7 +111,8 @@ func (ins *Inspector) GetPageContentStream(tabID string, pageNum int) (*ContentS return nil, err } if cs.Error != "" { - // Surface the first stream's decode/type failure verbatim. + // Surface the first FAILING stream's decode/type failure verbatim. + // NodeID is that stream, not ids[0], so the error names the culprit. return &ContentStreamData{NodeID: id, Error: cs.Error}, nil } raws = append(raws, cs.Raw) From 283d2b02a5dd02344ead2fac3b28241b943b8147 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Tue, 28 Jul 2026 16:11:00 +0700 Subject: [PATCH 07/11] fix: report a malformed contents array element instead of skipping it --- internal/pdfcore/stream.go | 21 ++++++++++++++----- internal/pdfcore/stream_test.go | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index 9e3ce47..7f5fd32 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -10,9 +10,11 @@ import ( // pageContentStreamNodeIDs resolves a 1-based page number to the ordered node // IDs of its content stream(s) in a SINGLE page-dict resolution: empty slice // (no error) when the page has no /Contents, one ID for a single indirect ref, -// and, for an array, every indirect-ref element in order (a degenerate null / -// non-ref element is skipped per ISO 32000-1 7.8.2 - it is not a stream). An -// array whose FIRST element is not an indirect ref is an error (preserving the +// and, for an array, every indirect-ref element in order. A null element is +// skipped - it is not a stream and carries no content (ISO 32000-1 7.8.2) - but +// any other non-ref element is an ERROR naming its index and type, never a +// silent skip, since dropping it could omit part of the page. An array whose +// FIRST element is not an indirect ref is likewise an error (preserving the // GetPageContentStreamNodeID contract). The order is the concatenation order: // a page's content is the join of these streams (7.8.2). func (ins *Inspector) pageContentStreamNodeIDs(tabID string, pageNum int) ([]string, error) { @@ -54,10 +56,19 @@ func (ins *Inspector) pageContentStreamNodeIDs(tabID string, pageNum int) ([]str return nil, fmt.Errorf("contents array element is not an indirect reference") } ids := make([]string, 0, len(v)) - for _, e := range v { + for i, e := range v { + // A null element carries no content, so skipping it drops nothing + // (pdfcpu decodes PDF null to a Go nil element). Anything else that + // is not an indirect ref is malformed - /Contents elements must be + // refs to streams (7.8.2) and streams are always indirect (7.3.8) - + // and is reported rather than skipped: silently dropping it could + // omit part of the page content, the exact failure this avoids. + if e == nil { + continue + } ref, ok := e.(pdfcpu_types.IndirectRef) if !ok { - continue // null / non-ref element: not a stream, skip + return nil, fmt.Errorf("contents array element %d is not an indirect reference: %T", i, e) } ids = append(ids, fmt.Sprintf("obj:%d:%d", ref.GenerationNumber.Value(), ref.ObjectNumber.Value())) } diff --git a/internal/pdfcore/stream_test.go b/internal/pdfcore/stream_test.go index 8c52328..88fd79b 100644 --- a/internal/pdfcore/stream_test.go +++ b/internal/pdfcore/stream_test.go @@ -512,6 +512,42 @@ func TestPageContentStreamNodeIDs(t *testing.T) { t.Errorf("[14.3-UNIT-002] [ref null] stream count = %d, want 1 (a null element is not a stream, so it is skipped in concatenation)", got) } }) + + // A non-null, non-ref element is malformed rather than empty: /Contents + // elements are refs to streams (7.8.2) and streams are always indirect + // (7.3.8). Skipping it could omit real page content, so it must be reported. + // Same in-memory injection as above - pdfcpu rejects the shape on disk. + t.Run("[ref junk] errors instead of silently skipping", func(t *testing.T) { + objs := []string{ + catalog, + pages, + "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents [4 0 R] >>\nendobj\n", + contentStreamObj(4), + } + ins, tabID := writeTempPDF(t, "refjunk.pdf", assembleDiffPDF(1, objs...)) + doc, err := ins.GetDocument(tabID) + if err != nil { + t.Fatalf("[14.3-UNIT-002] GetDocument: %v", err) + } + pageDict, _, _, err := doc.PDFContext.PageDict(1, false) + if err != nil { + t.Fatalf("[14.3-UNIT-002] PageDict: %v", err) + } + arr, ok := pageDict["Contents"].(pdfcpu_types.Array) + if !ok || len(arr) != 1 { + t.Fatalf("[14.3-UNIT-002] fixture broken: /Contents = %v, want a one-element array", pageDict["Contents"]) + } + ref := arr[0].(pdfcpu_types.IndirectRef) + pageDict["Contents"] = pdfcpu_types.Array{ref, pdfcpu_types.Integer(42)} + + ids, err := ins.pageContentStreamNodeIDs(tabID, 1) + if err == nil { + t.Fatalf("[14.3-UNIT-002] [ref 42] returned %d ids and no error; a non-null non-ref element must be reported, not skipped", len(ids)) + } + if !strings.Contains(err.Error(), "element 1") { + t.Errorf("[14.3-UNIT-002] error must name the offending index, got: %v", err) + } + }) } // --------------------------------------------------------------------------- From a0bc0e02a56dede47e00e06e79180990872986e5 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Tue, 28 Jul 2026 16:11:00 +0700 Subject: [PATCH 08/11] chore: say where multi-stream page content is assembled --- internal/pdfcore/model.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/pdfcore/model.go b/internal/pdfcore/model.go index 53119aa..7a9c6dd 100644 --- a/internal/pdfcore/model.go +++ b/internal/pdfcore/model.go @@ -60,9 +60,9 @@ type ValueEntry struct { } // ContentStreamData holds raw and tokenized content stream data for a page. For -// a multi-stream page (/Contents is an array) the CLI assembles this from the -// concatenation of every stream (see Inspector.GetPageContentStream), so Raw / -// Tokenized / Formatted already cover the whole page content. +// a multi-stream page (/Contents is an array) Inspector.GetPageContentStream +// concatenates every stream before tokenizing, so Raw / Tokenized / Formatted +// already cover the whole page content for every caller, not just the CLI. type ContentStreamData struct { NodeID string `json:"nodeId"` Raw string `json:"raw"` From 823d97632aa9f703fbbe3630db8b4aa525d2b9f0 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 1 Aug 2026 13:46:37 +0700 Subject: [PATCH 09/11] fix(14-3): auto-expand to depth-capped diff nodes so the marker is reachable Frontend hasDelta now treats node.truncated as a delta (mirroring the Go diffNodeHasDelta), so a PDF whose only anomaly is a depth-capped subtree auto-expands to the [truncated: depth cap] row instead of burying it under an unexpanded ancestor. Tighten the component test to assert the per-node row marker + node path, not just the summary note (PR review, alip must-fix). --- .../components/DiffView.truncation.test.tsx | 21 +++++++++++++------ frontend/src/components/DiffView.tsx | 7 +++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/DiffView.truncation.test.tsx b/frontend/src/components/DiffView.truncation.test.tsx index 5573921..963ecfd 100644 --- a/frontend/src/components/DiffView.truncation.test.tsx +++ b/frontend/src/components/DiffView.truncation.test.tsx @@ -86,14 +86,23 @@ describe('DiffView depth-cap truncation (Story 14.3)', () => { expect(text).not.toMatch(/no structural differences|no differ|identical/); }); - // 14.3-COMP-001 [P1] AC5: the depth-cap marker is rendered somewhere in the - // view so the bounded walk is visible to the user (mirrors the CLI marker). - test('14.3-COMP-001 renders a depth-cap truncation marker', async () => { - const { container } = render(); + // 14.3-COMP-001 [P1] AC5: the per-node [truncated: depth cap] ROW renders, not + // just the summary note. The depth-capped node reports status "unchanged", so + // hasDelta must treat `truncated` as a delta for its ancestors to auto-expand; + // otherwise the marker sits under an unexpanded ancestor and is unreachable. + // Asserts the bracketed row text (distinct from the summary note's "truncated + // at the depth cap") AND the cut node's path, so it genuinely covers the + // DiffView.tsx per-node marker branch rather than passing on the summary note. + test('14.3-COMP-001 auto-expands to the depth-cap node and renders its row marker', async () => { + render(); await waitFor(() => expect(mockDiffDocuments).toHaveBeenCalled()); await screen.findByTestId('diff-summary'); - const body = (container.textContent ?? '').toLowerCase(); - expect(body).toMatch(/truncat|depth cap/); + + // The cut node itself is rendered (its ancestors auto-expanded to reach it). + expect(screen.getAllByText('/Root/Deep').length).toBeGreaterThan(0); + // ...carrying the per-node marker: bracketed row text, which the summary + // note ("... truncated at the depth cap ...") does not contain. + expect(screen.getAllByText(/\[truncated: depth cap\]/).length).toBeGreaterThan(0); }); }); diff --git a/frontend/src/components/DiffView.tsx b/frontend/src/components/DiffView.tsx index 1490297..7b3c467 100644 --- a/frontend/src/components/DiffView.tsx +++ b/frontend/src/components/DiffView.tsx @@ -56,9 +56,12 @@ export interface DiffViewProps { active: boolean; } -/** Reports whether a node or any descendant is not "unchanged". */ +/** Reports whether a node or any descendant is not "unchanged", or is a + * depth-cap truncated ref. A truncated node reports status "unchanged" but must + * still route auto-expansion so its [truncated: depth cap] marker is reachable + * without hand-expanding every level (mirrors Go's diffNodeHasDelta). */ function hasDelta(node: DiffNodeData): boolean { - if (node.status !== 'unchanged') return true; + if (node.status !== 'unchanged' || node.truncated) return true; return (node.children ?? []).some(hasDelta); } From 08739ee40f1eb208586af5d3df764e89a902951c Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 1 Aug 2026 13:46:37 +0700 Subject: [PATCH 10/11] refactor(14-3): consistent /Contents null handling; honest reconciliation naming Drop the pageContentStreamNodeIDs v[0] guard so a null is skipped at any index (the loop already errors on non-null non-refs with index+type); add a [null ref] case. Rename reconcileTruncation's walkedPairs -> enteredPairs and document that the set is populated on diff ENTRY, not full-walk completion, with the argument for why the truncation count still cannot wrongly reach zero; pin it with TestReconcileTruncation_EntryNotCompletion (PR review, alip should-fix). --- internal/pdfcore/diff.go | 50 ++++++++++++++++++++------------- internal/pdfcore/diff_test.go | 35 +++++++++++++++++++++++ internal/pdfcore/stream.go | 14 ++++----- internal/pdfcore/stream_test.go | 36 ++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 27 deletions(-) diff --git a/internal/pdfcore/diff.go b/internal/pdfcore/diff.go index d3491e5..4db7b69 100644 --- a/internal/pdfcore/diff.go +++ b/internal/pdfcore/diff.go @@ -103,12 +103,14 @@ type DiffSummary struct { type diffContext struct { left *DocumentState right *DocumentState - // visitedPairs records the (left,right) indirect-ref pairs already fully - // diffed, keyed "numL:genL|numR:genR". It is a GLOBAL (never-cleared) - // cross-path dedup: a subgraph shared by many referrers (e.g. one /Resources - // referenced by every page) is walked once, and a crafted diamond graph - // cannot blow up exponentially. Distinct from the path-scoped visited sets, - // which cut only back-edges (cycles) on the current ancestor path. + // visitedPairs records the (left,right) indirect-ref pairs whose diff has + // been ENTERED (recorded before recursing, not on completion), keyed + // "numL:genL|numR:genR". It is a GLOBAL (never-cleared) cross-path dedup: a + // subgraph shared by many referrers (e.g. one /Resources referenced by every + // page) is walked once, and a crafted diamond graph cannot blow up + // exponentially. Distinct from the path-scoped visited sets, which cut only + // back-edges (cycles) on the current ancestor path. See reconcileTruncation + // for why entry (vs full-walk) semantics still keep the truncation count honest. visitedPairs map[string]bool } @@ -443,25 +445,35 @@ func (dc *diffContext) singleSided(path string, doc *DocumentState, val pdfcpu_t } // reconcileTruncation clears the depth-cap Truncated mark on any node whose -// ref-pair was fully walked elsewhere in the graph (its capRefPair is in -// walkedPairs). A pair diffed on a shallow path is fully accounted for, so a -// second encounter past the depth cap hides nothing; marking it would -// over-count DiffSummary.TruncatedSubtrees and wrongly withhold the "identical" -// verdict (Story 14.3). It runs once after the walk, when walkedPairs is -// complete, so it corrects BOTH DFS orders (shallow-first: the deep leg is -// cleared here; deep-first: the deep leg is marked during the walk, then -// cleared here once the shallow leg has populated walkedPairs). Depth-cap cuts -// with no ref-pair (deep DIRECT nesting, or a ref aligned against a non-ref) -// carry no capRefPair and are left marked - they are genuinely unresolved. -func reconcileTruncation(n *DiffNode, walkedPairs map[string]bool) { +// ref-pair was ENTERED elsewhere in the graph (its capRefPair is in +// enteredPairs). It runs once after the walk, when enteredPairs is complete, so +// it corrects BOTH DFS orders (shallow-first: the deep leg is cleared here; +// deep-first: the deep leg is marked during the walk, then cleared once the +// shallow leg has populated enteredPairs). Depth-cap cuts with no ref-pair (deep +// DIRECT nesting, or a ref aligned against a non-ref) carry no capRefPair and +// are left marked - they are genuinely unresolved. +// +// enteredPairs is populated on diff ENTRY (diffChild records the pair before +// recursing), NOT on completion of a full walk, so a pair entered at +// nextDepth == maxResolveDepth - whose own children are then cut one level +// deeper - is in the set exactly like a pair walked to its leaves. Clearing a +// past-the-cap encounter of such a pair is nonetheless honest, and the count +// cannot wrongly reach zero, because the entry walk of that pair EXAMINED its +// direct children: each child is either a counted delta, a fully-walked +// subtree, or itself a depth-cap cut carrying its OWN capRefPair. That deeper +// mark survives reconciliation unless ITS pair was also entered elsewhere - +// i.e. unless it too was genuinely reached. So the deepest genuinely-unresolved +// pair always retains a mark; clearing the shallower, redundant encounter only +// removes double-counting. (Pinned by TestReconcileTruncation_EntryNotCompletion.) +func reconcileTruncation(n *DiffNode, enteredPairs map[string]bool) { if n == nil { return } - if n.Truncated && n.capRefPair != "" && walkedPairs[n.capRefPair] { + if n.Truncated && n.capRefPair != "" && enteredPairs[n.capRefPair] { n.Truncated = false } for _, c := range n.Children { - reconcileTruncation(c, walkedPairs) + reconcileTruncation(c, enteredPairs) } } diff --git a/internal/pdfcore/diff_test.go b/internal/pdfcore/diff_test.go index 30cccaf..1b3d656 100644 --- a/internal/pdfcore/diff_test.go +++ b/internal/pdfcore/diff_test.go @@ -340,6 +340,41 @@ func TestDiff_SharedRefWalkedElsewhereNotTruncated(t *testing.T) { } } +// TestReconcileTruncation_EntryNotCompletion pins the honesty guarantee behind +// the entry-populated enteredPairs set (PR review, diff.go:reconcileTruncation): +// enteredPairs records a pair on diff ENTRY, not on completion of a full walk, +// so a pair entered near the depth cap whose own subtree is then cut is in the +// set exactly like a fully-walked pair. Clearing a redundant past-the-cap +// encounter of such a pair must NOT zero TruncatedSubtrees, because the deeper, +// genuinely-unresolved pair retains its own mark. Asserted directly on +// reconcileTruncation + countDelta so it does not depend on a brittle +// depth-33 PDF fixture. +func TestReconcileTruncation_EntryNotCompletion(t *testing.T) { + // childP: a redundant past-cap encounter of pair "P" that WAS entered + // elsewhere -> in enteredPairs -> its mark clears. + // childQ: pair "Q" that was NEVER entered elsewhere (genuinely unresolved, + // e.g. a child of P cut when P's own entry walk hit the cap) -> survives. + childP := &DiffNode{Path: "/Root/P", Status: "unchanged", Kind: "ref", Truncated: true, capRefPair: "P"} + childQ := &DiffNode{Path: "/Root/Q", Status: "unchanged", Kind: "ref", Truncated: true, capRefPair: "Q"} + root := &DiffNode{Path: "/Root", Status: "unchanged", Kind: "dict", Children: []*DiffNode{childP, childQ}} + + enteredPairs := map[string]bool{"P": true} // P entered elsewhere; Q was not + + reconcileTruncation(root, enteredPairs) + + if childP.Truncated { + t.Errorf("childP: a pair entered elsewhere should have its redundant mark cleared") + } + if !childQ.Truncated { + t.Errorf("childQ: a pair never entered elsewhere is genuinely unresolved and must stay truncated") + } + var s DiffSummary + countDelta(root, &s) + if s.TruncatedSubtrees != 1 { + t.Errorf("TruncatedSubtrees = %d, want 1: clearing an entered-but-not-fully-walked pair must not zero the count while a genuinely-unresolved pair remains", s.TruncatedSubtrees) + } +} + // --------------------------------------------------------------------------- // 14.3-UNIT-001 [P1] AC1/AC2 (Story 14.3): a deep chain diffed past the // maxResolveDepth cap marks the cut node DiffNode.Truncated and tallies it in diff --git a/internal/pdfcore/stream.go b/internal/pdfcore/stream.go index 7f5fd32..89e63a9 100644 --- a/internal/pdfcore/stream.go +++ b/internal/pdfcore/stream.go @@ -52,17 +52,15 @@ func (ins *Inspector) pageContentStreamNodeIDs(tabID string, pageNum int) ([]str if len(v) == 0 { return nil, nil } - if _, ok := v[0].(pdfcpu_types.IndirectRef); !ok { - return nil, fmt.Errorf("contents array element is not an indirect reference") - } ids := make([]string, 0, len(v)) for i, e := range v { // A null element carries no content, so skipping it drops nothing - // (pdfcpu decodes PDF null to a Go nil element). Anything else that - // is not an indirect ref is malformed - /Contents elements must be - // refs to streams (7.8.2) and streams are always indirect (7.3.8) - - // and is reported rather than skipped: silently dropping it could - // omit part of the page content, the exact failure this avoids. + // (pdfcpu decodes PDF null to a Go nil element) - at ANY index, + // including the first. Anything else that is not an indirect ref is + // malformed - /Contents elements must be refs to streams (7.8.2) and + // streams are always indirect (7.3.8) - and is reported (with index + + // type) rather than skipped: silently dropping it could omit part of + // the page content, the exact failure this avoids. if e == nil { continue } diff --git a/internal/pdfcore/stream_test.go b/internal/pdfcore/stream_test.go index 88fd79b..d312dbe 100644 --- a/internal/pdfcore/stream_test.go +++ b/internal/pdfcore/stream_test.go @@ -513,6 +513,42 @@ func TestPageContentStreamNodeIDs(t *testing.T) { } }) + // A null at index 0 is skipped just like a null anywhere else (PR review: + // null handling must not depend on position). [null ref] enumerates the one + // real stream, exactly like [ref null]. + t.Run("degenerate array [null ref] skips the leading null", func(t *testing.T) { + objs := []string{ + catalog, + pages, + "3 0 obj\n<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents [4 0 R] >>\nendobj\n", + contentStreamObj(4), + } + ins, tabID := writeTempPDF(t, "nullref.pdf", assembleDiffPDF(1, objs...)) + doc, err := ins.GetDocument(tabID) + if err != nil { + t.Fatalf("[14.3-UNIT-002] GetDocument: %v", err) + } + pageDict, _, _, err := doc.PDFContext.PageDict(1, false) + if err != nil { + t.Fatalf("[14.3-UNIT-002] PageDict: %v", err) + } + arr, ok := pageDict["Contents"].(pdfcpu_types.Array) + if !ok || len(arr) != 1 { + t.Fatalf("[14.3-UNIT-002] fixture broken: /Contents = %v, want a one-element array", pageDict["Contents"]) + } + ref := arr[0].(pdfcpu_types.IndirectRef) + // Inject the null AT INDEX 0. + pageDict["Contents"] = pdfcpu_types.Array{nil, ref} + + ids, err := ins.pageContentStreamNodeIDs(tabID, 1) + if err != nil { + t.Fatalf("[14.3-UNIT-002] [null ref] unexpected error: %v (a leading null must be skipped, not rejected)", err) + } + if got := len(ids); got != 1 { + t.Errorf("[14.3-UNIT-002] [null ref] stream count = %d, want 1 (leading null skipped like any other null)", got) + } + }) + // A non-null, non-ref element is malformed rather than empty: /Contents // elements are refs to streams (7.8.2) and streams are always indirect // (7.3.8). Skipping it could omit real page content, so it must be reported. From f30ac3832f367a699ea41e84c14db96d29b13c26 Mon Sep 17 00:00:00 2001 From: Ade Anom A Date: Sat, 1 Aug 2026 13:46:37 +0700 Subject: [PATCH 11/11] chore(14-3): drop leftover builder/blank line; document --raw join + GUI gap Remove the now-pointless strings.Builder on the empty-stream plain path and the stray blank line left by the --ops meta removal. Document the --raw multi-stream concatenation contract change and the remaining not-handled cases (indirect-ref -to-array /Contents; GUI Go-to-Page still shows stream 1) in the fixture README (PR review, alip nits + should-fix disclosure). --- cmd/cli/cmd_stream.go | 6 ++---- testdata/correctness/README.md | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/cmd/cli/cmd_stream.go b/cmd/cli/cmd_stream.go index 698a153..4f5b15a 100644 --- a/cmd/cli/cmd_stream.go +++ b/cmd/cli/cmd_stream.go @@ -233,15 +233,14 @@ func writeNoContentStream(flags streamFlags) int { // PDF content-stream order (let it flow; do not tabulate). NON-CONTRACTUAL; // use --json for structured operators, --ops for NDJSON, --raw for bytes. func printStreamPlain(out io.Writer, result *pdfcore.ContentStreamData) error { - var b strings.Builder // A content-stream object can exist yet decode to zero operators (an empty // /Contents stream). Surface that as a one-line note so plain output is never // a silent zero-byte write and always ends with a newline. if len(result.Formatted) == 0 { - b.WriteString("(empty content stream)\n") - _, err := io.WriteString(out, b.String()) + _, err := io.WriteString(out, "(empty content stream)\n") return err } + var b strings.Builder for _, fl := range result.Formatted { for range fl.Indent { b.WriteString(" ") @@ -399,7 +398,6 @@ func emitOps(inspector *pdfcore.Inspector, result *pdfcore.ContentStreamData, pa return 2 } } - return 0 } diff --git a/testdata/correctness/README.md b/testdata/correctness/README.md index 41b6d0d..05011d9 100644 --- a/testdata/correctness/README.md +++ b/testdata/correctness/README.md @@ -188,7 +188,21 @@ obj 5 (stream 2): BT Per ISO 32000-1 7.8.2 the page content is the concatenation of both streams joined by whitespace. Pre-fix `GetPageContentStreamNodeID` returns only the first ref's node ID, so `dump stream --page 1` decodes ONLY stream 1 (`q`, `cm`) -and presents an unbalanced partial program with no marker. Post-fix the tool -either concatenates both streams (operators from both appear) or emits a -machine-visible truncation marker (`streamCount`/`truncated`) on `--json` and -`--ops`. 5 objects total. +and presents an unbalanced partial program. Post-fix `dump stream --page 1` +concatenates ALL of the page's content streams (via `GetPageContentStream`), +newline-joined, and tokenizes them as one program - operators from both streams +appear on `--json`, `--ops`, plain text, and `--raw`. 5 objects total. + +**`--raw` contract change:** for a multi-stream page, `--raw` now emits every +stream's decoded bytes joined by a single injected `\n` (the whole page content +per 7.8.2), not one stream verbatim. A script that diffed `--raw` output against +one extracted stream's bytes must instead compare against the concatenation. + +**Still not handled (visible/deferred, not silent):** +- `/Contents` given as an indirect ref to an array (`/Contents 6 0 R` where obj6 + is `[4 0 R 5 0 R]`): errors visibly (`node is not a stream object`, exit 2) + rather than concatenating - pdfcpu does not pre-dereference `/Contents`. +- GUI "Go to Page" still lands on the first content stream alone: `GoToPage` + returns `GetPageContentStreamNodeID` (a single tree node) and `DetailPanel` + fetches it via `GetContentStream`. `GetPageContentStream` is not yet bound + into `pdfservice`, so the concatenation fix is CLI-only for now.