Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/frontend/src/app/slides/slides-view.tsx`:
- Around line 151-153: The catch block that logs "Failed to apply pending PPTX
import" in slides-view.tsx silently swallows import errors; update the error
handling in the catch inside the function that applies the pending PPTX import
(the try/catch around the PPTX import logic in SlidesView) to surface a
user-facing error (e.g., call the app's existing toast/notification API or set
an error state that renders a visible alert/modal) and ensure the UI does not
proceed with an empty slides root without explaining the failure; include the
original error message/details in the notification and consider keeping the
original console.error for debugging while preventing the app from initializing
silently incorrect state.

In `@packages/slides/src/import/pptx/color.ts`:
- Around line 7-10: The code currently assumes identity mapping between scheme
roles and aliases (e.g., bg1/tx1 ↔ dk1/lt1) and ignores the master clrMap;
update the color resolution to consult the master clrMap and apply any remapping
before looking up scheme slots: locate the color resolution logic that
references clrMap or resolves roles (e.g., the function that maps scheme roles
like "bg1","tx1","dk1","lt1") and change it to first check for clrMap entries
for the given role/alias, translate the role via clrMap when present, then
resolve the resulting role to the scheme color; ensure both
WordprocessingML-style aliases and original role names are handled through that
single remapping step so imported text/background/hyperlink colors use the
remapped slots.

In `@packages/slides/src/import/pptx/font.ts`:
- Around line 17-19: The code currently returns the raw value from attr(latin,
'typeface') which allows whitespace-only strings; update the logic around the
face variable to trim whitespace before returning so whitespace-only values are
treated as unset — e.g., call .trim() on the result of attr(latin, 'typeface')
and if the trimmed value is empty return undefined, otherwise return the trimmed
string; reference the face variable and the attr(latin, 'typeface') call to
locate where to change.
- Around line 27-29: The Hangul detection currently checks ranges using the
local variable code but misses Hangul Jamo Extended-A and Extended-B; update the
condition that checks (code >= 0xac00 && code <= 0xd7a3) || (code >= 0x1100 &&
code <= 0x11ff) || (code >= 0x3130 && code <= 0x318f) to also include (code >=
0xA960 && code <= 0xA97F) and (code >= 0xD7B0 && code <= 0xD7FF) so the Korean
fallback covers Jamo Extended-A and Extended-B; locate and modify the boolean
expression that uses the variable named code in
packages/slides/src/import/pptx/font.ts to add those two ranges.

In `@packages/slides/src/import/pptx/geometry.ts`:
- Around line 17-21: Validate slideSizeEmu.cx and slideSizeEmu.cy in emuScale:
if either is 0, NaN, or not finite, avoid computing 1920/0 etc.; either throw a
clear, meaningful Error (e.g., "emuScale: invalid slideSizeEmu.cx/cy — must be
finite non-zero numbers") or substitute slideSizeEmu with DEFAULT_WIDESCREEN_EMU
before computing and then return the EmuScale; update emuScale to perform this
guard and use the validated values when producing sx and sy.

In `@packages/slides/src/import/pptx/group.ts`:
- Around line 89-97: applyGroupTransform currently accumulates rotation in the
returned Frame.rotation but does not rotate the child x/y around the group's
origin, so children are misplaced when the group has rotation; update
applyGroupTransform to first compute the local offset (dx = frame.x -
t.childBaseX, dy = frame.y - t.childBaseY), rotate that vector by t.rotation
(convert to radians if rotations are stored in degrees), then apply scaling and
add t.parentOffX/t.parentOffY to produce world x/y, and keep rotation =
frame.rotation + t.rotation; also consider adjusting w/h by projecting the
rotated/scaled bbox (or at minimum document a TODO) so width/height reflect
rotation-aware bounds.

In `@packages/slides/src/import/pptx/image.ts`:
- Line 70: Wrap the call to ctx.uploadImage(bytes, mime) in a try/catch so an
upload failure becomes a soft skip instead of throwing: on error log/record the
failure into the import report (include the image identifier, mime, and error
message) and return/continue as a skipped image (e.g., leave src null or a
placeholder) so processing continues; ensure you update whatever report object
is used in this module rather than rethrowing the exception and that callers of
the function handle a skipped/null src accordingly.

In `@packages/slides/src/import/pptx/master.ts`:
- Line 21: The background assignment uses a shallow spread so nested objects
like fill remain shared; update the background creation in the master module so
you deep-copy the defaults and parsed result (e.g. clone
DEFAULT_MASTER.background and the object returned from parseBackground) instead
of using { ...DEFAULT_MASTER.background } or shallow spreads; ensure the same
change is applied to the other occurrence noted (the second shallow spread on
line 43) so that nested fill objects are not shared between masters (refer to
background, parseBackground, and DEFAULT_MASTER.background to locate the spots
to fix).
- Line 27: The placeholderStyles property is being shallow-copied from
DEFAULT_MASTER (placeholderStyles: { ...DEFAULT_MASTER.placeholderStyles }),
which leaves nested style objects shared and mutable across masters; change this
to create a deep copy of DEFAULT_MASTER.placeholderStyles (e.g., use
structuredClone or a deep-clone utility or map/clone each nested style object)
so all nested style objects are new instances and mutations won't leak between
masters/imports; update the placeholderStyles assignment in master.ts to use
that deep-clone approach referencing DEFAULT_MASTER.placeholderStyles.

In `@packages/slides/src/import/pptx/report.ts`:
- Around line 20-30: The summary() method in report.ts omits the counters
unknownLayoutTypes, tableMergesIgnored, and tableBordersApproximated so those
metrics never appear in the final report; update summary() to push entries for
these counters (similar to existing lines for tablesFlattened, groupsFlattened,
shadowsDropped, textBoxesPreScaled, unknownShapes, skippedImages) using their
names and pluralized labels (e.g., `${this.unknownLayoutTypes} unknown
layout(s)`, `${this.tableMergesIgnored} table merge(s) ignored`,
`${this.tableBordersApproximated} table border(s) approximated`) before the
parts.length check so all defined counters are included in the returned string.

In `@packages/slides/src/import/pptx/shape.ts`:
- Around line 245-251: The current branch returns early when a prstGeom is
found, causing any accompanying txBody text to be dropped; update the logic in
the function that handles <p:sp> (where prstGeom, spPr, child, buildShapeElement
and parseSpElements are used) to emit both a ShapeElement and a TextElement when
both geometry and txBody exist — e.g., call buildShapeElement for the prstGeom
and also parse the txBody (or change parseSpElements to return [shape, text])
and return/appends both elements (preserving elementId, frame, sp, ctx) instead
of returning only the shape.
- Around line 406-409: The stroke width conversion currently uses a fixed
EMU->px factor (w / 9525) which ignores the slide/deck scaling; update the
conversion in shape.ts where w and width are computed to incorporate the
rendering scale (use the canvas/deck scale from ctx, e.g. ctx.scale or the
deck-level scale variable) so the result becomes Math.max(0, (w / 9525) *
deckScale) when w != null, leaving the fallback of 1 unchanged; adjust any
downstream usage of width (and related variables like ln and solidFill) to rely
on this scaled value so strokes match other scaled geometry.

In `@packages/slides/src/import/pptx/table.test.ts`:
- Around line 75-91: Rename the test case title to accurately reflect the
attributes being exercised: change the it(...) description from 'counts merges
via gridSpan/rowSpan/vMerge' to 'counts merges via gridSpan/hMerge' in the test
in table.test.ts; the assert and XML fixture (using gridSpan and hMerge) as well
as references to parseTable(frame(xml), ctx(report)) and
report.tableMergesIgnored should remain unchanged.

In `@packages/slides/src/import/pptx/table.ts`:
- Line 85: The loop advancing x uses a single colWidth (x += colWidth) which
misplaces cells when a cell has gridSpan > 1; change the increment to advance by
the effective span width for that cell (compute spanWidth as the sum of the
column widths covered by the cell or at minimum colWidth * gridSpan) and do x +=
spanWidth; update the code around the cell layout logic in table.ts where x,
colWidth and gridSpan are used (ensure you read gridSpan from the cell and
compute spanWidth before incrementing x).
- Around line 114-117: The border width conversion currently divides wEmu by
9525 (const wEmu = attrInt(ln, 'w'); const width = wEmu != null ? Math.max(0,
wEmu / 9525) : 1;) which bypasses the deck/shape stroke scaling and causes
inconsistent thickness; replace the hardcoded division with the project’s
canonical EMU→px conversion or deck-derived scale used for shape strokes (i.e.,
call the existing emu-to-px helper or apply the same deck scale/transform used
elsewhere for stroke widths) so width = wEmu != null ? Math.max(0,
emuToPx(wEmu)) : 1 (or equivalent using the deck scale), preserving the existing
per-cell approximation comment and variable names (wEmu, width, attrInt, ln).

In `@packages/slides/src/import/pptx/text.ts`:
- Around line 201-207: The code assigns rel.target directly to style.href (in
the block using hlink, rPr, NS.R, ctx.rels and style.href), which can expose
unsafe URI schemes from untrusted PPTX; update the logic to validate the URI
scheme before assignment by parsing rel.target and allowing only safe schemes
(e.g., http, https, mailto, maybe tel) or explicitly rejecting dangerous schemes
(javascript:, data:, vbscript:), and only set style.href when the scheme passes
validation (otherwise skip or null it); ensure validation handles missing
schemes and relative URLs appropriately and use the existing ctx.rels/rid lookup
flow to locate the value before validating.

---

Outside diff comments:
In `@packages/frontend/src/app/documents/document-list.tsx`:
- Around line 485-526: Add an "Import PPTX" DropdownMenuItem to the empty-state
DropdownMenuContent so the empty-state matches the main "New" dropdown: insert a
DropdownMenuItem sibling (like the existing Import DOCX item) with
disabled={importing} and onClick={handleImportPptx} that renders the appropriate
icon and label "Import PPTX"; ensure the handler name handleImportPptx
exists/imported and the disabled prop uses the same importing flag as the Import
DOCX item so behavior is consistent with createDocumentMutation calls for other
items.

---

Nitpick comments:
In `@packages/frontend/src/app/slides/slides-view.tsx`:
- Around line 147-148: The double-cast on pending.layouts/pending.slides hides a
lossy transformation between SlidesDocument.layouts and
YorkieSlidesRoot["layouts"]; add a typed conversion helper (e.g.,
toYorkieLayout(item: SlidesDocumentLayout): YorkieLayout) that explicitly
maps/removes fields like masterId and staticElements and documents the loss,
call this helper to convert pending.layouts and pending.slides before assigning
to r.layouts / r.slides, and keep migrateDocument() noted in a comment as the
complementary reconstruction step on read.

In `@packages/slides/src/import/pptx/font.test.ts`:
- Around line 10-41: Add two edge-case tests: for parsePrimaryTypeface (used in
parsePrimaryTypeface and fontEl) add a case where the typeface attribute
contains only whitespace (e.g. typeface="   ") and assert it returns undefined,
and for containsHangul add strings that include Hangul Jamo Extended-A and
Extended-B characters (characters from the Hangul Jamo Extended-A/B blocks) and
assert they return true; update the existing test suite blocks for
parsePrimaryTypeface and containsHangul to include these assertions so the
parser treats whitespace-only typeface as empty and the Hangul detector
recognizes extended Jamo ranges.

In `@packages/slides/src/import/pptx/group.test.ts`:
- Around line 19-82: Add a new unit test that covers a rotated group by creating
a grpSp XML with <a:xfrm rot="..."> (use grpSp helper), call
composeGroupTransform(IDENTITY_TRANSFORM, grp, SCALE) and then
applyGroupTransform(local, t) for a child with known local x/y/w/h; assert the
resulting world.x and world.y (and optionally w/h and rotation) using
toBeCloseTo so you verify the rotation is applied to positions, not just the
rotation field. Ensure the test name describes "rotated-group" and follows the
existing pattern used in the other tests.

In `@packages/slides/src/import/pptx/image.test.ts`:
- Around line 28-107: Add a test in
packages/slides/src/import/pptx/image.test.ts that exercises parsePic’s upload
failure path: call parsePic with a valid archive (fakeArchive with
'ppt/media/image1.png'), rels containing rId3, and provide uploadImage as an
async function that throws/rejects; assert the returned result is undefined and
that the ImportReport instance (report.skippedImages) was incremented by 1.
Reference parsePic, uploadImage, ImportReport, fakeArchive and picEl(PIC) when
locating where to add the new it(...) block and make uploadImage throw
asynchronously to simulate the failure.

In `@packages/slides/src/import/pptx/index.ts`:
- Around line 158-166: The pickRelTarget helper currently loops over
rels.values() manually; simplify it by using Array.prototype.find to locate the
first relation whose type matches the provided type and return its target (or
undefined). Update the pickRelTarget function to call
Array.from(rels.values()).find(r => r.type === type) and then return the
found?.target, keeping the same signature and behavior.
- Line 86: The inline type assertions on the fallback object (layouts: [] as
Layout[], layoutMap: new Map() as LayoutPathToInfo) are verbose—replace them by
declaring an explicitly typed constant or by annotating the function/variable
return type so the compiler infers types without inline casts; e.g., introduce a
named constant like emptyLayoutInfo typed to the object shape using the Layout
and LayoutPathToInfo types (with layoutMap typed as LayoutPathToInfo and layouts
as Layout[]) and use that constant instead of the inline as-casts where the
current fallback object is returned/used.

In `@packages/slides/src/import/pptx/master.test.ts`:
- Around line 36-40: Add a regression test that checks parseMaster returns
independent style objects: call parseMaster twice (e.g., m1 =
parseMaster(MIN_MASTER, 'm', 't') and m2 = parseMaster(MIN_MASTER, 'm', 't')),
mutate m1.placeholderStyles.title.fontSize to a new value, and assert that
m2.placeholderStyles.title.fontSize remains the original value; use parseMaster
and the placeholderStyles.title.fontSize property to locate the code under test
and ensure the test name describes the non-sharing guarantee.

In `@packages/slides/src/import/pptx/slide.ts`:
- Around line 146-151: The helper relsSiblingFor is duplicated in slide.ts and
index.ts; extract it into a single shared utility (e.g., create
packages/slides/src/import/pptx/path-utils.ts exporting function
relsSiblingFor(partPath: string)) and replace the local implementations in both
slide.ts and index.ts with an import { relsSiblingFor } from './path-utils';
ensuring the exported function signature and behavior remain identical so
callers (relsSiblingFor) continue to work unchanged.
- Around line 134-142: Replace the index-based loop over spTree.childNodes with
a for-of loop to improve readability: iterate each node in spTree.childNodes,
skip non-element nodes (nodeType !== 1), cast to Element (or use a type guard),
check el.localName === 'sp', get txBody via child(el, 'txBody'), continue if
absent, and then return parseTextBody(txBody, { report }); keep the exact logic
and early returns intact and reference the existing symbols spTree, child,
txBody, and parseTextBody.

In `@packages/slides/src/import/pptx/text.test.ts`:
- Around line 46-56: Add a regression test that ensures unsafe hyperlink schemes
(e.g., "javascript:alert(1)") are ignored: create a test (or extend the existing
"resolves hyperlink rids via the per-slide rels map") that supplies a rel entry
with target "javascript:alert(1)" (using PptxRel), parse the txBody via
parseTextBody (with txBody helper and new ImportReport()), then assert that the
resulting inline.style.href is not populated (undefined or falsy) for that
inline; reference parseTextBody, txBody, PptxRel, ImportReport, and
inline.style.href to locate where to add the check.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/slides/src/import/pptx/group.ts`:
- Around line 79-87: The pivot calculation for nested groups incorrectly ignores
the parent's rotation: replace the current direct translate+scale computation of
groupCenterX/groupCenterY -> pivotX/pivotY (which uses
parent.parentOffX/parent.parentOffY, parent.childBaseX/parent.childBaseY,
parent.scaleX/parent.scaleY) with a proper composition of the parent's affine
transform (translate, scale, then rotate) applied to the group's center; i.e.,
compute the group's center in parent-local coords and then rotate/translate it
via the parent's rotation (or build the parent 2D affine matrix and multiply the
center point) before deriving pivotX/pivotY and the related offsets used later
(also update the analogous logic around lines 90-97 that computes child
offsets).

In `@packages/slides/src/import/pptx/slide.ts`:
- Around line 137-147: The loop in parseNotes currently returns the first <p:sp>
with a <p:txBody> (using spTree, child and parseTextBody) which picks
headers/footers; change it to first check the shape's placeholder: inspect
child(el,'nvSpPr') → child(nvSpPr,'nvPr') → child(nvPr,'ph') and only accept
when ph.getAttribute('type') === 'body'; if none found, parse all txBody shapes
with parseTextBody, filter out empty results and return the largest (by text
length) as a fallback. Ensure you still skip non-element nodes and shapes
without txBody.

---

Duplicate comments:
In `@packages/slides/src/import/pptx/slide.ts`:
- Around line 57-59: The code currently shallow-copies DEFAULT_BACKGROUND into
the local background (const background = ... ? parseSlideBackground(...) : {
...DEFAULT_BACKGROUND }), which shares nested objects like fill and allows
mutations to leak; update the fallback to produce a deep clone of
DEFAULT_BACKGROUND (e.g., use structuredClone(DEFAULT_BACKGROUND) or a dedicated
deepClone utility) so each slide gets an independent background object, and
apply the same change where DEFAULT_BACKGROUND is spread at the other
occurrence; ensure parseSlideBackground usage remains unchanged but that any
fallback uses the deep-cloned copy.