Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ Oracle case naming:

Case JSON (VBA pipeline): `test/e2e/oracle/cases-full/*.json`. Each declares `kind: “shape”` or `kind: “smartart”` with layout/dimensions.

Python-pptx pipeline cases: `test/e2e/oracle/cases-pypptx/*.json` (204 cases: 62 text,
Python-pptx pipeline cases: `test/e2e/oracle/cases-pypptx/*.json` (205 cases: 63 text,
31 shape-adjustment, 28 zero-adjustment flowchart, 2 shape-effect, 1 text-effect, 20 static 3D,
8 table, 8 formula, 20 composite, and 24 chart).
Prefix `oracle-pypptx-{category}-{NNNN}-{slug}` where category is `text`, `shape-adj`,
Expand All @@ -269,7 +269,8 @@ Prefix `oracle-pypptx-{category}-{NNNN}-{slug}` where category is `text`, `shape
IDs 0040-0055 are the CJK wrap/autofit/spacing interaction matrix; IDs 0056-0059 cover
`defRPr`/`fontRef` text-color precedence; ID 0060 covers styled soft breaks; ID 0061 covers the
native tab-stop matrix; ID 0062 covers every non-horizontal DrawingML vertical mode, anchors,
stacked-character advance, and mixed CJK/Hangul fallback.
stacked-character advance, and mixed CJK/Hangul fallback; ID 0063 covers duplicate unlinked
placeholder indices and type-based layout inheritance.

### Step 2: Identify the Shape

Expand Down Expand Up @@ -684,7 +685,7 @@ Report: `test/e2e/reports/oracle-failures/full-ground-truth-one-shot.json`
| `scripts/run_all_shapes_eval.py` | `--shape-id-min N --shape-id-max N` | Batch evaluate shapes via POST `/api/evaluate/{case}` |
| `scripts/analyze_edge.py` | `<case> [--slide N]` | Canny edge IoU analysis with visual overlay output |
| `scripts/one_shot_full_ground_truth.py` | `--macro-host ... --cases-dir ...` | Bulk generate PPTX+PDF ground truth from oracle cases |
| `scripts/generate_pypptx_cases.py` | `[--case PATTERN]` | Generate 204 python-pptx cases across the documented matrices |
| `scripts/generate_pypptx_cases.py` | `[--case PATTERN]` | Generate 205 python-pptx cases across the documented matrices |
| `scripts/reflection_metrics.py` | `--case-report PATH` | Validate local reflection fields and erasure sensitivity |

Key mechanics:
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht
### Fixed

- Fixed explicit text tabs, all DrawingML vertical text flows, stacked WordArt spacing, CJK/Korean
font fallback, and stretched picture-filled runs, including lazy package media.
font fallback, stretched picture-filled runs including lazy package media, and placeholder style
inheritance when layouts reuse an unlinked placeholder index.

## [1.3.0] - 2026-09-14

Expand Down
5 changes: 5 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ Responsibilities:
serialization consumes that slide.
- Build model-level text indexes and search results that are independent of mounted DOM.

Slide placeholders with an ordinary explicit `idx` inherit from the layout placeholder with that
index. A missing `idx`, or the unlinked sentinel `4294967295`, instead matches by the effective
placeholder type so duplicate sentinel values cannot select an unrelated layout style. Layout to
master inheritance remains type based.

## 3) Render Layer

Core modules:
Expand Down
4 changes: 3 additions & 1 deletion docs/TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ This generates/reuses ground truth for all SmartArt layouts available on the loc

For text, shape-adjustment, zero-adjustment flowchart, ordinary-shape and text effects, bounded
static DrawingML 3D, table, composite, and chart interaction cases, use the python-pptx generator.
It currently defines 204 cases: 62 text, 31 shape-adjustment, 28 flowchart, 2 shape-effect, 1
It currently defines 205 cases: 63 text, 31 shape-adjustment, 28 flowchart, 2 shape-effect, 1
text-effect, 20 static 3D, 8 table, 8 formula, 20 composite, and 24 chart cases. Chart cases 23–24
exercise readable two-dimensional fallbacks for native 3D column and pie charts; they do not claim
three-dimensional parity.
Expand Down Expand Up @@ -377,6 +377,8 @@ observation control.
ID 0062 is a nine-slide native vertical-text matrix covering `eaVert`, `mongolianVert`, `vert`,
`vert270`, `wordArtVert`, and `wordArtVertRtl`, plus top/center/bottom anchors, 12/24/36 pt stacked
character advance, and mixed CJK/Hangul fallback behavior.
ID 0063 reproduces duplicate unlinked placeholder indices and verifies that body geometry and text
styles resolve by placeholder type instead of layout order.

```bash
cd test/e2e
Expand Down
58 changes: 15 additions & 43 deletions src/model/Presentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
* (themes, masters, layouts, slides) into a single PresentationData structure.
*/

import { masterPlaceholderType } from './placeholderMatching';
import {
findMatchingLayoutPlaceholder,
findMatchingMasterPlaceholder,
getPlaceholderInfo,
} from './placeholderMatching';
import { PptxFiles } from '../parser/ZipParser';
import type { MediaResolver } from '../utils/media';
import { parseXml, SafeXmlNode } from '../parser/XmlParser';
Expand Down Expand Up @@ -420,28 +424,6 @@ export function buildPresentation(
// Placeholder Position Inheritance
// ---------------------------------------------------------------------------

/**
* Extract placeholder info (type, idx) from a raw placeholder XML node
* stored in layout/master.
*/
function getPhInfo(phNode: SafeXmlNode): { type?: string; idx?: number } {
// Try nvSpPr > nvPr > ph, or nvPicPr > nvPr > ph
for (const wrapper of ['nvSpPr', 'nvPicPr', 'nvGrpSpPr', 'nvGraphicFramePr', 'nvCxnSpPr']) {
const nvWrapper = phNode.child(wrapper);
if (nvWrapper.exists()) {
const nvPr = nvWrapper.child('nvPr');
const ph = nvPr.child('ph');
if (ph.exists()) {
const type = ph.attr('type');
const idxStr = ph.attr('idx');
const idx = idxStr !== undefined ? Number(idxStr) : undefined;
return { type, idx: idx !== undefined && !isNaN(idx) ? idx : undefined };
}
}
}
return {};
}

/**
* Extract xfrm position/size from a raw placeholder XML node.
*/
Expand All @@ -464,17 +446,6 @@ function getPhXfrm(phNode: SafeXmlNode): { position: Position; size: Size } | un
return undefined;
}

/**
* Find a matching layout placeholder (PlaceholderEntry); use entry.absoluteXfrm when present.
*/
function findMatchingLayoutPlaceholder(
placeholders: PlaceholderEntry[],
idx?: number,
): PlaceholderEntry | undefined {
// Slide placeholders inherit from layout by idx (default 0), regardless of type.
return placeholders.find((entry) => (getPhInfo(entry.node).idx ?? 0) === (idx ?? 0));
}

function getMasterPlaceholderEntries(master: MasterData): PlaceholderEntry[] {
return master.placeholderEntries ?? master.placeholders.map((node) => ({ node }));
}
Expand Down Expand Up @@ -523,7 +494,7 @@ function getPhBodyPr(phNode: SafeXmlNode): SafeXmlNode | undefined {
function inheritPlaceholderType(target: PlaceholderInfo, sourceNode: SafeXmlNode): void {
if (target.type) return;

const source = getPhInfo(sourceNode);
const source = getPlaceholderInfo(sourceNode);
if (source.type) {
target.type = source.type;
}
Expand Down Expand Up @@ -573,17 +544,18 @@ export function resolveNodePlaceholderInheritance(
): void {
if (!node.placeholder) return;

const { idx } = node.placeholder;
const layoutMatch = layout ? findMatchingLayoutPlaceholder(layout.placeholders, idx) : undefined;
const layoutMatch = layout
? findMatchingLayoutPlaceholder(layout.placeholders, node.placeholder, (entry) =>
getPlaceholderInfo(entry.node),
)
: undefined;
if (layoutMatch) inheritPlaceholderType(node.placeholder, layoutMatch.node);
// Layout -> master uses type, never idx: master idx values may collide with unrelated types.
const masterMatch = master
? getMasterPlaceholderEntries(master).find(
(entry) =>
(getPhInfo(entry.node).type ?? 'obj') ===
masterPlaceholderType(
layoutMatch ? getPhInfo(layoutMatch.node).type : node.placeholder?.type,
),
? findMatchingMasterPlaceholder(
getMasterPlaceholderEntries(master),
layoutMatch ? getPlaceholderInfo(layoutMatch.node).type : node.placeholder?.type,
(entry) => getPlaceholderInfo(entry.node),
)
: undefined;

Expand Down
49 changes: 49 additions & 0 deletions src/model/placeholderMatching.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,52 @@
import { SafeXmlNode } from '../parser/XmlParser';
import type { PlaceholderInfo } from './nodes/BaseNode';

export const UNLINKED_PLACEHOLDER_IDX = 0xffffffff;

const PLACEHOLDER_WRAPPERS = [
'nvSpPr',
'nvPicPr',
'nvGrpSpPr',
'nvGraphicFramePr',
'nvCxnSpPr',
] as const;

export function getPlaceholderInfo(node: SafeXmlNode): PlaceholderInfo {
for (const wrapper of PLACEHOLDER_WRAPPERS) {
const ph = node.child(wrapper).child('nvPr').child('ph');
if (!ph.exists()) continue;

const idx = ph.numAttr('idx');
return {
type: ph.attr('type'),
idx: idx !== undefined && Number.isFinite(idx) ? idx : undefined,
};
}
return {};
}

export function findMatchingLayoutPlaceholder<T>(
placeholders: readonly T[],
target: PlaceholderInfo,
getInfo: (placeholder: T) => PlaceholderInfo,
): T | undefined {
if (target.idx !== undefined && target.idx !== UNLINKED_PLACEHOLDER_IDX) {
return placeholders.find((placeholder) => (getInfo(placeholder).idx ?? 0) === target.idx);
}

const targetType = target.type ?? 'obj';
return placeholders.find((placeholder) => (getInfo(placeholder).type ?? 'obj') === targetType);
}

export function findMatchingMasterPlaceholder<T>(
placeholders: readonly T[],
layoutType: string | undefined,
getInfo: (placeholder: T) => PlaceholderInfo,
): T | undefined {
const targetType = masterPlaceholderType(layoutType);
return placeholders.find((placeholder) => (getInfo(placeholder).type ?? 'obj') === targetType);
}

/**
* Layout placeholder categories that inherit from the master's body/title placeholder.
* Master inheritance uses this type mapping, not the layout/slide idx namespace.
Expand Down
54 changes: 14 additions & 40 deletions src/renderer/TextRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
* with full 7-level style inheritance.
*/

import { masterPlaceholderType } from '../model/placeholderMatching';
import {
findMatchingLayoutPlaceholder,
findMatchingMasterPlaceholder,
getPlaceholderInfo,
masterPlaceholderType,
} from '../model/placeholderMatching';
import { SafeXmlNode } from '../parser/XmlParser';
import { RenderContext } from './RenderContext';
import type { TextBody, TextParagraph, TextRun } from '../model/nodes/ShapeNode';
Expand Down Expand Up @@ -110,38 +115,6 @@ function getPlaceholderCategory(
return 'other';
}

/**
* Find a placeholder node in a list by matching type and/or idx.
*/
function findPlaceholderNode(
placeholders: SafeXmlNode[],
info: PlaceholderInfo,
matchBy: 'idx' | 'type',
): SafeXmlNode | undefined {
for (const ph of placeholders) {
// Navigate to the ph element to read its attributes
let phEl: SafeXmlNode | undefined;
const nvSpPr = ph.child('nvSpPr');
if (nvSpPr.exists()) {
phEl = nvSpPr.child('nvPr').child('ph');
}
if (!phEl || !phEl.exists()) {
const nvPicPr = ph.child('nvPicPr');
if (nvPicPr.exists()) {
phEl = nvPicPr.child('nvPr').child('ph');
}
}
if (!phEl || !phEl.exists()) continue;

const phType = phEl.attr('type');
const phIdx = phEl.numAttr('idx');

if (matchBy === 'idx' && (phIdx ?? 0) === (info.idx ?? 0)) return ph;
if (matchBy === 'type' && (phType ?? 'obj') === masterPlaceholderType(info.type)) return ph;
}
return undefined;
}

/**
* Extract lstStyle from a placeholder shape node.
*/
Expand Down Expand Up @@ -1222,16 +1195,13 @@ export function renderTextBody(
): void {
const textBody = resolveTextFields(sourceTextBody, ctx);
const layoutPh = placeholder
? findPlaceholderNode(
? findMatchingLayoutPlaceholder(
ctx.layout.placeholders.map((entry) => entry.node),
placeholder,
'idx',
getPlaceholderInfo,
)
: undefined;
const layoutPhProps = layoutPh?.child('nvSpPr').child('nvPr').child('ph');
const layoutPhType = layoutPhProps?.exists()
? layoutPhProps.attr('type')
: layoutPh?.child('nvPicPr').child('nvPr').child('ph').attr('type');
const layoutPhType = layoutPh ? getPlaceholderInfo(layoutPh).type : undefined;
// Follow the parent's parent: a local slide type does not rewire layout -> master.
const masterInfo = layoutPh ? { type: layoutPhType } : placeholder;
const category = getPlaceholderCategory(
Expand Down Expand Up @@ -1309,7 +1279,11 @@ export function renderTextBody(

// Level 4: master placeholder lstStyle
if (masterInfo) {
const masterPh = findPlaceholderNode(ctx.master.placeholders, masterInfo, 'type');
const masterPh = findMatchingMasterPlaceholder(
ctx.master.placeholders,
masterInfo.type,
getPlaceholderInfo,
);
if (masterPh) {
const lstStyle = getPlaceholderLstStyle(masterPh);
mergeParagraphProps(merged, findStyleAtLevel(lstStyle, level));
Expand Down
13 changes: 7 additions & 6 deletions test/e2e/oracle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,17 +237,18 @@ Report (default):
## Python-pptx Ground Truth Pipeline

A second pipeline uses `python-pptx` for PPTX creation and native PowerPoint automation for
ground-truth export. It defines 204 cases under `oracle/cases-pypptx/` with the
ground-truth export. It defines 205 cases under `oracle/cases-pypptx/` with the
`oracle-pypptx-*` prefix:

- **Text** (62 cases): fonts, sizes, styles, alignment, colors, bullets, vertical text,
- **Text** (63 cases): fonts, sizes, styles, alignment, colors, bullets, vertical text,
placeholder inheritance, plus a 16-case CJK wrap/autofit/line-spacing interaction matrix whose
final four cases cover square/wide/tall `spAutoFit` growth and explicit-overflow opt-out, and a
four-case `defRPr`/`fontRef` color-precedence matrix with explicit-run and inverse controls. The
final three cases isolate styled soft breaks, an eleven-slide native tab-stop matrix, and a
final four cases isolate styled soft breaks, an eleven-slide native tab-stop matrix, a
nine-slide matrix for every non-horizontal DrawingML vertical mode, anchors, stacked-character
advance, and mixed CJK/Hangul fallback. Explicit left/center/right/decimal tabs and the vertical
tab row are verified; the RTL tab row remains an observation control
advance, and mixed CJK/Hangul fallback, plus duplicate unlinked placeholder indices. Explicit
left/center/right/decimal tabs and the vertical tab row are verified; the RTL tab row remains an
observation control
- **Shape adjustments** (31 cases): adjustment handles for roundRect, chevron, arrow, star, donut, cross, trapezoid, blockArc, bevel, triangle, pentagon, can, heart, moon, brace
- **Zero-adjustment flowcharts** (28 cases, 84 slides): presets in shape IDs 61-88, each with
square explicit paint, wide theme-reference paint, and grouped-tall rendering
Expand Down Expand Up @@ -297,7 +298,7 @@ Generate cases:
cd test/e2e
.venv/bin/python3 scripts/generate_pypptx_cases.py

# Focus one or more exact/glob patterns; this example selects text IDs 0040-0062.
# Focus one or more exact/glob patterns; this example selects text IDs 0040-0063.
.venv/bin/python3 scripts/generate_pypptx_cases.py \
--case 'oracle-pypptx-text-00[456]*'

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"name": "oracle-pypptx-text-0063-unlinked-placeholder-idx-collision",
"generator": "python-pptx",
"slides": [
{
"nodes": [
{
"kind": "pypptx-generated"
}
]
}
],
"coverage": {
"oracle": "native-powerpoint",
"requiredFonts": [
"Arial"
],
"features": [
"placeholder.idx=4294967295",
"placeholder.layout-collision",
"placeholder.layout-match=type",
"text.inheritance.master-body"
]
}
}
Loading
Loading