Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/backend/src/image/image.config.ts`:
- Around line 7-8: The config currently uses hardcoded fallbacks for
IMAGE_STORAGE_ACCESS_KEY and IMAGE_STORAGE_SECRET_KEY (accessKey, secretKey)
which exposes non-dev deployments; change image.config.ts so these env vars are
not defaulted to 'minioadmin' — either only allow a default when running in a
local/dev mode (e.g., NODE_ENV === 'development' or an existing isLocalDev
check) or throw an explicit initialization error (fail fast) if they are missing
in non-dev environments; update the code paths that read accessKey/secretKey to
reflect this behavior and add a clear error message referencing
IMAGE_STORAGE_ACCESS_KEY / IMAGE_STORAGE_SECRET_KEY when failing.

In `@packages/backend/src/image/image.service.ts`:
- Around line 69-71: The code currently derives file extension from originalName
(ext = originalName.split('.').pop()) which allows mismatched suffixes; change
key generation in image.service.ts to derive the extension from the validated
mimeType instead (e.g., use a mime-to-extension map based on the incoming
mimeType used earlier), then build key as `${id}.${derivedExt}`; alternatively,
if you want stricter behavior, validate that the originalName suffix matches the
derived extension and reject uploads when they disagree; update references to
randomUUID() and key to use the mime-derived extension so stored keys conform to
VALID_ID_PATTERN used in image.controller.ts.

In `@packages/docs/src/export/docx-exporter.ts`:
- Around line 37-49: The header/footer image handling is missing:
DocxExporter.buildHeaderFooterXml is being called with an empty images array and
only document.xml.rels is written, so images referenced in header/footer are
never packaged or have header/footer-level .rels; fix by passing the actual
header/footer image list from doc.header/doc.footer into
DocxExporter.buildHeaderFooterXml (instead of []), create media parts under
word/media for each header/footer image and add corresponding Relationship
entries with unique rIds into a new header/footer rels file (e.g., write
word/_rels/header1.xml.rels and word/_rels/footer1.xml.rels), update hfRels and
hfContentTypes appropriately, and ensure the rIdCounter is used to generate
unique rIds consistent with those rels (so drawing XML in header/footer points
to the packaged media via the rels you add).

In `@packages/docs/src/import/docx-importer.ts`:
- Around line 302-309: The code currently selects a header/footer by scanning
rels for the first relationship of a given type (using the rels map and the
local variables type/targetFile), which ignores the section's w:sectPr
references; instead, read the active section's w:sectPr and find the matching
w:headerReference/w:footerReference element with the correct type attribute
("default"/"first"/"even"), take its r:id, then map that r:id through
document.xml.rels (the rels map) to get the real rel.target; replace the loop
that picks the first rel by type with logic that resolves r:id from the
section's headerReference/footerReference and then looks up rels.get(rId) (or
rels[rId]) to set targetFile so the actual section-specific header/footer is
imported.
- Around line 273-333: parseHeaderFooter fails to resolve images because
uploadImages only processes document-level relationships; update
parseHeaderFooter to load the header/footer's sibling .rels part from the zip
(the relationships for that specific targetFile), convert that .rels into a
RelEntry map and call uploadImages for that map so image targets get uploaded
and added to imageUrls, and then pass the (merged) imageUrls into
DocxImporter.convertParagraph; this requires adding an ImageUploader parameter
to parseHeaderFooter (or otherwise providing the uploader) and invoking
uploadImages(zip, relsForTarget, uploader, imageUrls) before converting
paragraph nodes so header/footer r:id references resolve.
- Around line 74-96: The resolvedInlines mapping leaves pending placeholders in
place when imageUrls lacks an entry; update the branch that handles
inline.style.image?.src.startsWith('__pending__:') so that if uploaded is
undefined you still consume the corresponding imageRefs entry (keep the refIdx++
behavior) but return the inline with the image removed (e.g., return the same
inline text and style but without the image property) instead of leaving a
"__pending__:*" src; adjust the resolvedInlines mapping around the inlines loop
(variables: inlines, imageUrls, imageRefs, resolvedInlines, emusToPx) to
implement this skip.

In `@packages/docs/src/import/docx-parser.ts`:
- Around line 99-110: The current run reconstruction collects all <w:t> text
then appends a single tab/newline, which loses original child-node order; update
the run processing (where variable r is used and
getElementsByTagNameNS/textEls/tabs/brs are referenced) to iterate r.childNodes
in document order and for each child emit its content: if it's a <w:t> node
append its textContent, if it's a <w:tab> append '\t', if it's a <w:br> append
'\n' (handle other node types by skipping or preserving as before), ensuring you
use the W namespace constant when checking nodeLocalName or namespaceURI so tabs
and breaks are emitted in the exact sequence they appear.
- Around line 63-67: The guard that checks runs' parent (r.parentElement !== pEl
&& r.parentElement?.localName !== 'hyperlink') is empty so nested <w:r> elements
still get processed; update the loop that iterates getElementsByTagNameNS(W,
'r') to skip these nested runs by adding an early continue (or return for a
map/filter) when that condition is true so any run whose parent is not the
paragraph (pEl) and not a hyperlink is ignored; locate the loop handling r and
pEl in docx-parser.ts and add the skip logic immediately after the guard.

In `@packages/docs/src/import/docx-style-map.ts`:
- Around line 49-52: The code maps any w:highlight value (via getW(rPr,
'highlight') and getWAttr(highlight, 'val')) through mapHighlightColor and sets
style.backgroundColor, which incorrectly turns w:highlight="none" into a
fallback color; change the logic in the highlight handling (around getW,
getWAttr, mapHighlightColor and style.backgroundColor) to treat the literal
value 'none' as "no highlight" and skip setting style.backgroundColor (i.e.,
only call mapHighlightColor and assign when val is present and not equal to
'none').
- Around line 25-29: The current logic in docx-style-map uses getW(rPr, 'u') and
then only sets style.underline when getWAttr(u, 'val') exists and is not "none",
which ignores a bare <w:u/> element that should enable underline; update the
check in the block that reads const u = getW(rPr, 'u') so that it treats missing
val as enabled (i.e., if val is undefined or val !== 'none' then set
style.underline = true), using the existing getW and getWAttr helpers and the
style.underline field.

In `@packages/docs/src/import/units.ts`:
- Around line 12-35: The functions pxToTwips, pxToEmus and pointsToHalfPoints
can return fractional values; change each to return an integer by rounding the
computed result (e.g. Math.round) before returning so OOXML attributes serialize
as whole numbers—update pxToTwips(px), pxToEmus(px) and pointsToHalfPoints(pts)
to round their results while leaving emusToPx, pxToEmus and halfPointsToPoints
semantics otherwise unchanged.

In `@packages/docs/src/view/doc-canvas.ts`:
- Around line 25-41: The cached-image branch in getOrLoadImage currently returns
null for a loading image without registering the new onLoad callback, causing
only the first waiter to repaint; update getOrLoadImage so that when
imageCache.get(src) returns a cached Image that is not yet complete
(cached.complete is false), you attach the provided onLoad callback to that
Image (e.g., cached.addEventListener('load', onLoad)) before returning null;
keep the existing behavior for complete/broken images and continue to use
imageCache and the img.onerror handling as-is.

In `@packages/docs/src/view/fonts.ts`:
- Around line 63-67: The try block around document.fonts.load currently includes
this.listeners.forEach so any listener exception falls into the outer catch and
flips this.status.set(key, 'error'); fix by isolating listener failures: after
awaiting document.fonts.load and setting this.status.set(key, 'loaded') (or
before leaving the try), invoke each callback inside its own try/catch (or move
listeners invocation outside the outer try) so thrown errors are caught/logged
locally and do not cause the outer catch to mark the font as 'error'; update the
code paths referenced (document.fonts.load, this.status.set,
this.listeners.forEach) accordingly.

In `@packages/frontend/src/app/docs/docs-view.tsx`:
- Line 17: The current call to takePendingImport() removes the pending import
before applying it, so if store.setDocument(...) or
resetAfterDocumentReplace(...) throws the import is lost; modify the
apply/import flow to catch any errors and re-queue the import on failure (e.g.,
push the object back into the pending-imports source) so users can retry;
specifically update the code paths that call takePendingImport() (the
import/apply block around takePendingImport and the similar block at the area
corresponding to lines 215-223) to wrap the application in try/catch and re-add
the taken pending import inside the catch before rethrowing or handling the
error.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts`:
- Around line 89-99: The image deserialization currently uses
Number(attrs['image.width']) and Number(attrs['image.height']) which can produce
NaN and persist invalid values; in the block that constructs ImageData (the code
that checks 'image.src' in attrs and assigns style.image), validate the numeric
conversions for width and height using Number(...) and then guard with
Number.isFinite(...) (and optionally check >= 0 or > 0 per UX requirements),
only assign image.width and image.height when the parsed values are finite
(otherwise omit them or set a safe default), so ImageData never gets NaN for
width/height; update the logic around attrs['image.width'],
attrs['image.height'], ImageData and style.image accordingly.

---

Minor comments:
In `@docs/design/docs/docs-docx-import-export.md`:
- Around line 441-443: Update the design doc's file list to reflect the
implemented helper: replace the entry "docx-builder.ts" with "docx-templates.ts"
in the table that currently lists "docx-exporter.ts / docx-builder.ts /
docx-style-map.ts"; also verify and update any surrounding descriptions that
reference docx-builder.ts so they accurately describe docx-templates.ts and its
role as the export helper used by docx-exporter.ts and docx-style-map.ts.

In `@docs/tasks/active/20260410-docx-import-export-todo.md`:
- Around line 1-13: The TODO file 20260410-docx-import-export-todo.md needs a
paired lessons file: add a new 20260410-docx-import-export-lessons.md alongside
it that mirrors the TODO’s frontmatter/metadata and directory placement,
summarizes post-completion learnings and decisions (key takeaways, pitfalls,
useful links like docs/design/docs/docs-docx-import-export.md), and includes
references to the implemented pieces (e.g., JSZip + DOMParser import approach,
XML generation export, S3 image handling) so readers can trace from the TODO to
the final lessons; ensure filenames match the YYYYMMDD-<slug>-lessons.md pattern
and update any local task index if one exists.

In `@packages/docs/src/export/docx-style-map.ts`:
- Around line 11-12: The code inserts style.fontFamily directly into the
w:rFonts XML attribute, which can break DOCX when the font name contains &, <, >
or quotes; update the generator in docx-style-map.ts (the block that pushes
`<w:rFonts .../>` into parts) to XML-escape the attribute value before
interpolation (e.g., use or add a small escapeXml/escapeAttr helper and call it
on style.fontFamily) so the resulting w:rFonts attributes are always
well-formed.

In `@packages/docs/src/store/block-helpers.ts`:
- Line 231: The inline insertion currently does a shallow copy ({ ...inline })
causing inline.style and nested objects like inline.style.image to remain
aliased; update the insertion to deep-clone the inline before passing to
normalizeInlines (i.e., replace { ...inline } with a proper deep copy of the
inline), implement or call a clone helper (e.g., cloneInline or structuredClone)
that copies inline.style and nested style.image so the newBlock.inlines receives
an independent object, and use that clonedInline in the
normalizeInlines([...before, clonedInline, ...after]) call.

In `@packages/docs/test/export/docx-exporter.test.ts`:
- Around line 21-105: Add a new test in docx-exporter.test.ts that creates a
Document with a paragraph containing an image inline (use generateBlockId(),
DEFAULT_BLOCK_STYLE and the Document shape), call DocxExporter.export(doc) and
assert the produced ZIP (JSZip.loadAsync on blob.arrayBuffer()) contains a media
file under word/media/ and a relationship entry in word/_rels/document.xml.rels,
then re-import via DocxImporter.import(buffer) and assert the reimported block's
inline preserves the image metadata (e.g., type or src/ref fields on the inline
object) to verify round-trip image export/import through DocxExporter.export and
DocxImporter.import.

In `@packages/docs/test/import/docx-importer.test.ts`:
- Around line 48-269: Add a test that supplies header/footer parts and
relationships to createMinimalDocx and calls DocxImporter.import, ensuring the
importer resolves header*.xml/footer*.xml via the package relationships and
attaches parsed blocks to doc.header and doc.footer; specifically, create
minimal header/footer XML (e.g. a paragraph run), include Relationships XML with
Relationship entries pointing to word/header1.xml and word/footer1.xml, pass
those files via extraFiles to createMinimalDocx, call
DocxImporter.import(buffer) and assert that doc.header and doc.footer are
defined and contain the expected blocks/text, referencing DocxImporter.import
and the doc.header/doc.footer properties to locate where to assert correct
behavior.

In `@packages/docs/test/import/docx-parser.test.ts`:
- Around line 18-40: Add a unit test covering the image/drawing branch of
parseParagraph: create a paragraph XML that includes an inline image/drawing
element (the inline-image placeholder/pending-image extraction path), call
parseParagraph(el) and assert the returned result.inlines contains an
image-placeholder inline (e.g., type or marker used by parseParagraph for
images) and that the parse result exposes the pending image entry (e.g.,
result.pendingImages or similar) with expected attributes (id/relationship/alt
text). This ensures parseParagraph's inline-image and pending-image extraction
paths are exercised alongside the existing text-run and empty-paragraph tests.

In `@packages/frontend/src/app/docs/docx-actions.ts`:
- Around line 85-88: The sanitized filename logic can produce an empty string
(e.g., title of only illegal chars or whitespace), resulting in a download name
of ".docx"; update the code around safeTitle and filename so you reapply the
"document" fallback after calling replace(...).trim(): compute safeTitle =
(title || "").replace(/[\\/:*?"<>|]+/g, "_").trim(); if safeTitle === "" set
safeTitle = "document"; then build filename from safeTitle and ensure it ends
with ".docx" (the symbols to update are safeTitle and filename, and the
replace(...).trim() call).

---

Nitpick comments:
In `@docker-compose.yaml`:
- Around line 20-31: The MinIO service configuration exposes the API and console
on all interfaces and uses hard-coded credentials; update the minio service
(service name "minio") to bind ports to localhost (replace "9000:9000" and
"9001:9001" with address-restricted mappings to 127.0.0.1) and remove hard-coded
MINIO_ROOT_USER/MINIO_ROOT_PASSWORD values so they are sourced from the
environment or an external .env/secret; ensure the command "server /data
--console-address \":9001\"" continues to work with the localhost binding and
keep the existing volume "minio-data:/data".

In `@docs/design/docs/docs-docx-import-export.md`:
- Around line 244-245: The two consecutive lines that start with "Parse ..."
(steps referencing "word/numbering.xml" and "word/_rels/document.xml.rels") are
repetitive; rephrase one or both to vary sentence openings for readability—e.g.,
change step 4 to "Handle lists by parsing `word/numbering.xml`" and step 5 to
"Map relationship IDs to media files by parsing `word/_rels/document.xml.rels`"
(or similar alternatives) so the steps read more smoothly while preserving the
same meaning.

In `@packages/docs/src/view/fonts.ts`:
- Around line 42-44: The onFontLoaded method currently pushes callbacks into
this.listeners with no removal path; change onFontLoaded(cb: () => void): () =>
void to return an unsubscribe function that removes the provided cb from
this.listeners (e.g., findIndex/splice or filter) so callers can dispose
listeners; ensure the unsubscribe is idempotent (safe if called multiple times)
and references the same callback passed to onFontLoaded to remove the exact
entry.

In `@packages/docs/test/import/units.test.ts`:
- Around line 4-31: Add edge-case and round-trip assertions to the OOXML
conversion tests: for each helper (twipsToPx, emusToPx, halfPointsToPoints,
pxToTwips, pxToEmus, pointsToHalfPoints) add assertions for 0 and negative
inputs (e.g., -value -> expected negative/zero behavior) and add round-trip
checks (e.g., px -> pxToTwips -> twipsToPx returns original px within tolerance,
px -> pxToEmus -> emusToPx returns original px, and points -> pointsToHalfPoints
-> halfPointsToPoints returns original points). Ensure tolerances match existing
toBeCloseTo usage for floating conversions and use exact equality for integer
conversions.

In `@packages/docs/test/model/types.test.ts`:
- Around line 249-261: Add new assertions to the existing test that cover image
width, height, and alt mismatches: use the InlineStyle fixtures (a, b, c)
pattern and call inlineStylesEqual with variants where only width differs, only
height differs, and only alt differs to assert false; also include positive
assertions where width/height/alt match to ensure true. Locate the test block
using the inlineStylesEqual function and InlineStyle type in types.test.ts and
add one-liners that mirror the existing src comparison checks for width, height,
and alt fields.

In `@packages/docs/test/view/fonts.test.ts`:
- Around line 4-29: Add unit tests that exercise FontRegistry.ensureFont's
lifecycle: call ensureFont for an unknown font on a FontRegistry instance,
assert getFontStatus reports 'pending', then simulate a successful load and
assert status becomes 'loaded' and any registered success listener/callback was
invoked; likewise simulate a load failure and assert status becomes 'error' and
error listeners/callbacks were invoked. Use the FontRegistry class, its
ensureFont method, and getFontStatus to locate the behavior, and register the
same listeners/callbacks your implementation exposes (e.g., onload/onerror or a
passed callback) to verify they are called exactly once and that status
transitions occur from 'pending' -> 'loaded' or 'pending' -> 'error'. Ensure
tests cover both synchronous and asynchronous resolution paths if ensureFont
returns a Promise.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/docs/src/export/docx-exporter.ts`:
- Around line 283-287: The code currently derives the image extension from the
URL (src) which is unreliable; instead, after calling fetcher(src) use the
fetched response/blob to determine the media type (e.g., use
response.headers.get('content-type') or blob.type) and map that MIME type to a
correct extension before calling nextMediaName(ext) and zip.file(`word/${path}`,
blob); if the Content-Type is missing or unknown, fallback to 'png'. Update the
logic around fetcher/src, blob, ext, nextMediaName, and zip.file to use the
blob/response-derived MIME-to-extension mapping.
- Around line 151-165: In docx-exporter.ts inside the image handling branch
(where inline.style.image is checked and imageEntries is searched), stop
silently falling through when no ImageEntry is found: if const entry =
imageEntries.find(e => e.src === inline.style.image!.src) returns undefined,
throw a descriptive Error (include the missing image src and mention
imageFetcher/ImageEntry) instead of returning a text run; this makes the failure
fast and surfaced to callers (update any callers/tests expecting a different
behavior if needed).

In `@packages/docs/src/view/fonts.ts`:
- Around line 21-27: The resolveFontFamily function currently interpolates the
raw family string into a CSS font-family value which can break when family
contains quotes or backslashes; update resolveFontFamily to sanitize/escape the
dynamic family (escape backslashes and any single quotes) before composing the
return value so the produced string is safe for CSS; keep using FONT_MAP and
SERIF_FONTS to short-circuit mapped or generic lookups but always apply escaping
to the family variable when constructing `'${family}', ${generic}` to prevent
invalid CSS from custom DOCX font names.

In `@packages/frontend/src/app/docs/docs-view.tsx`:
- Around line 221-226: The pending DOCX import can remain if
store.setDocument(pending) succeeds but editor.resetAfterDocumentReplace()
throws, so move the clearPendingImport call to immediately after a successful
store.setDocument(pending) and before editor.resetAfterDocumentReplace(); i.e.,
call clearPendingImport(documentId) right after store.setDocument(pending) so
the pending entry is removed as soon as the document is applied, then run
editor.resetAfterDocumentReplace(), keeping the try/catch around the operations
to still log failures.

---

Nitpick comments:
In `@packages/docs/src/view/fonts.ts`:
- Around line 42-44: onFontLoaded should return an unsubscribe handle and
notifications must iterate a snapshot to avoid re-entrant mutation; change
onFontLoaded to add the callback to this.listeners and return a function that
removes that callback from this.listeners (so subscribers can unsubscribe), and
when dispatching (the notifier function that currently iterates this.listeners)
iterate over a shallow copy like Array.from(this.listeners) or
[...this.listeners] before invoking callbacks to prevent mutation during
notification; apply the same unsubscribe-returning pattern and snapshot
iteration to the other listener API at the 74-81 region as well.

In `@packages/docs/test/import/docx-importer.test.ts`:
- Around line 383-395: Replace the array length assertion for the uploaded array
with the Jest matcher toHaveLength for consistency: change the assertion that
inspects uploaded.length (the expect(uploaded.length).toBe(1) in the test around
DocxImporter.import) to use expect(uploaded).toHaveLength(1) so it matches other
tests' style and intent.
- Around line 143-150: Extract the duplicated 1x1 transparent PNG byte array
into a module-level constant named MINIMAL_PNG_BYTES and replace each inline
declaration of pngBytes in the tests with this constant; locate the occurrences
of pngBytes in docx-importer.test (both around lines shown) and create the
constant near the top of the test file (or in a shared test utils module if used
across files), then update the tests to reference MINIMAL_PNG_BYTES instead of
re-declaring the Uint8Array.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/docs/src/export/docx-exporter.ts`:
- Around line 179-187: The returned image XML currently hardcodes wp:docPr
id="1" and pic:cNvPr id="1", which must be unique; update the image-rendering
routine in docx-exporter.ts to accept or use an image counter/uniqueId and
substitute it into both wp:docPr and pic:cNvPr attributes (instead of the
hardcoded "1"), ensure the counter increments per image (so each call producing
the string uses the same unique id for both wp:docPr and pic:cNvPr), and keep
using entry.rId for the relationship reference.

---

Nitpick comments:
In `@packages/docs/src/export/docx-exporter.ts`:
- Around line 203-207: The hardcoded totalTwips = 9000 used to generate gridCols
from tableData.columnWidths will mis-size tables for non-standard pages; replace
that constant with a computed content width derived from the document/section
page width minus left and right margins (e.g., compute contentTwips = (pageWidth
- marginLeft - marginRight) in twips) and use Math.round(w * contentTwips) when
building gridCols so the table fills the actual page content area; locate
totalTwips, gridCols and tableData.columnWidths in docx-exporter.ts and pull
pageWidth/margins from the document/section properties or accept them as
parameters if not available.
- Around line 58-63: The image fetching loop in DocxExporter (iterating over
doc.blocks and awaiting DocxExporter.collectImages) runs sequentially and should
be parallelized: first traverse doc.blocks to collect unique image sources
(e.g., add a new helper DocxExporter.collectImageSrcs) into a Set, then kick off
parallel fetches via Promise.all calling imageFetcher for each unique src and
updating docImageEntries/zip/media naming atomically; ensure deduplication uses
docImageEntries (or a new map) to assign and reuse nextDocRId/nextMediaName
safely (atomic increments or deterministic naming) so the rest of the export
logic that relies on those identifiers remains correct.