feat(skills): dark-launch office document-generation skills (#1273 PR3) - #1491
Merged
Conversation
Productize the svg-pptx native PPT route from the ppt-quality-eval baseline into a bundled product skill. Vendors the MIT svg_to_pptx converter (LICENSE + VENDORED.md provenance preserved) plus the two machine gates (check_text_budget pre-convert, check_pptx_assets pre-deliver). SKILL.md is reworded for the product flow and uses stable in-skill script paths instead of the eval SVG_PPTX_SCRIPT env var. Description marks it PREVIEW so it does not compete with the default officecli-pptx routing.
Native xlsx (openpyxl) and docx (python-docx) generation skills with the same machine-check-over-model-judgement discipline as office-pptx: real formulas / Heading styles, explicit fonts and number formats, and a pre-delivery structure gate (check_xlsx.py / check_docx.py) that must print OK. Descriptions marked PREVIEW so they do not compete with officecli-xlsx / officecli-docx.
Documents two paths: generate via HTML then PawWork's bundled Chromium printToPDF (CDP), and parse via uv + pdfplumber (MIT) / pypdf (BSD). Hard rule: never use PyMuPDF / fitz (AGPL). Marked PREVIEW; no default routing changed.
Contributor
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds four new "skill" bundles enabling native document generation/parsing: office-docx (python-docx), office-pdf (Chromium/pdfplumber/pypdf), office-pptx (a vendored SVG-to-PPTX DrawingML converter with animation, native chart/table embedding, and package assembly), and office-xlsx (openpyxl). Each includes SKILL.md documentation, pyproject.toml dependencies, and Python structural validation gate scripts. Also adds a base-directory output line to the skill tool and ignores Python bytecode. ChangesCore skill tool output
office-docx Skill
office-pdf Skill
office-pptx Skill
office-xlsx Skill
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as pptx_package.cli
participant Builder as create_pptx_with_native_svg
participant Converter as convert_svg_to_slide_shapes
participant Native as native_objects
participant Notes as notes.py
participant Narration as narration.py
CLI->>Builder: invoke with SVG files and options
Builder->>Converter: convert each SVG to slide shape XML
Converter->>Native: convert_native_object for chart/table markers
Native-->>Converter: ShapeResult (table/chart XML + assets)
Converter-->>Builder: slide XML + media/relationships
Builder->>Notes: create_notes_slide_xml per slide
Builder->>Narration: inject_narration and apply_recorded_timing
Builder-->>CLI: final .pptx package
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…e pyc The pre-commit smoke test ran the vendored converter in place, which wrote CPython bytecode into skills/office-pptx/scripts/**/__pycache__/. Remove those 31 tracked .pyc files and add a repo-wide Python bytecode ignore so bundled skill sources stay source-only.
The skill tool only printed the base directory as a file:// URL, but bundled skills instruct the model to reuse that directory in shell commands (cp, uv run script paths), where a file:// URL breaks. Add one output line with the plain path; the existing URL line is unchanged.
P1: - SKILL_DIR contract: all four SKILL.md now tell the model to use the new plain-filesystem-path line from the skill tool output (never the file:// URL, with a strip-prefix fallback). - office-pptx Gate 2 now runs via 'uv run python' like every other command, instead of a bare python3 that Windows packaging does not guarantee. P2: - office-xlsx: formula-injection rule — only model-authored computed cells may be formulas; user data starting with =/+/-/@ must be force-stored as text; links via cell.hyperlink, never HYPERLINK(). check_xlsx.py adds a tripwire (HYPERLINK/WEBSERVICE/IMPORT*/DDE-style) and counts formulas by data_type=='f' only, so force-texted user data no longer false-FAILs. - check_xlsx.py --require-chart now counts xl/charts/chart*.xml package parts instead of relying on openpyxl's private ws._charts read-back (round-trip verified for bar/line/pie/area/scatter on openpyxl 3.1.5). - check_text_budget.py: tolerate unit suffixes like '60px' in x/font-size and add future annotations import (mirrors eval-side commit 95f30f4). - Dark-launch limitation stated at the top of each SKILL.md: preview is description-enforced only, hard switch belongs to PR 4. P3: - check_pptx_assets.py: minimal artifact-summary.json schema check (renderer string + non-empty slides array) and honest docstring scope (mirrors eval-side wording). - office-pdf SKILL.md: both parsers pinned for offline resolution, but one per task is enough.
github-actions
Bot
added
the
harness
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
skills/office-pptx/scripts/svg_to_pptx/animation_config.py (1)
191-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider warning on non-dict
transition/animationvalues in_validate_scope_effects.Non-dict
transitionoranimationfields are silently skipped rather than flagged. For example,"transition": "fade"(a string instead of an object) would produce no warning, leaving the user unaware their config is malformed. A brief type check with a warning would improve the validation surface.♻️ Optional: add type warnings for non-dict transition/animation
def _validate_scope_effects(scope: dict[str, Any], label: str, warnings: list[str]) -> None: transition = scope.get('transition', {}) + if transition and not isinstance(transition, dict): + warnings.append(f'animations.json {label} "transition" must be an object') if isinstance(transition, dict): effect = transition.get('effect') if effect is not None and not _valid_transition_effect(str(effect)): warnings.append(f'animations.json {label} has unknown transition effect: {effect}') animation = scope.get('animation', {}) + if animation and not isinstance(animation, dict): + warnings.append(f'animations.json {label} "animation" must be an object') if isinstance(animation, dict): effect = animation.get('effect') if effect is not None and not _valid_animation_effect(str(effect)): warnings.append(f'animations.json {label} has unknown animation effect: {effect}')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/office-pptx/scripts/svg_to_pptx/animation_config.py` around lines 191 - 201, The _validate_scope_effects helper currently skips malformed transition/animation values when they are not dicts, so add explicit type warnings for those fields before checking effects. Update _validate_scope_effects in animation_config.py to detect non-dict scope['transition'] and scope['animation'] values and append a warning that includes the label and the unexpected type/value, while keeping the existing unknown-effect validation for valid dicts.skills/office-pptx/scripts/svg_to_pptx/pptx_package/cli.py (1)
45-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated trigger/effect-resolution logic vs.
builder._slide_animation_settings.
_recorded_narration_on_click_slidesre-derives the effective animation effect/trigger fromanimation_config+ CLI overrides independently ofbuilder.py's_slide_animation_settings. If the config schema or override precedence changes in one place, this on-click validation can silently drift out of sync and let narration+on-click conflicts through undetected.Consider extracting the shared "resolve effective animation setting from CLI arg + config + overrides" logic into a single helper both files call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/office-pptx/scripts/svg_to_pptx/pptx_package/cli.py` around lines 45 - 71, The on-click narration check in _recorded_narration_on_click_slides is duplicating the same effective animation resolution logic that already exists in builder._slide_animation_settings, so the two paths can drift. Refactor the shared CLI/config/override precedence into a single helper and have both _recorded_narration_on_click_slides and _slide_animation_settings call it to resolve the effective effect and trigger consistently. Keep the helper responsible for interpreting animation_config, animation_cli_overrides, and the animation/animation_trigger inputs so validation stays aligned with the builder behavior.
🤖 Prompt for all review comments with AI agents
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 `@skills/office-docx/scripts/check_docx.py`:
- Around line 39-53: The paragraph-count gate in check_docx.py is treating
headings as body text, so a document with only Heading styles can incorrectly
pass. Update the counting logic around non_empty, headings, and the
min_paragraphs check so --min-paragraphs is based on real body paragraphs only
(exclude paragraphs whose style.name starts with "Heading"), while keeping the
existing heading and table checks unchanged.
In `@skills/office-docx/SKILL.md`:
- Around line 17-25: The uv check in the office-docx skill does not match the
required contract message. Update the shell snippet in the skill setup block so
the fallback path for the `uv --version` failure prints the exact required
string, and keep the rest of the flow unchanged; use the `SKILL_DIR` setup and
`uv --version` guard as the place to fix it.
In `@skills/office-pdf/SKILL.md`:
- Line 43: The missing-uv check in SKILL.md does not match the documented
runtime-contract message exactly because the current `uv --version` fallback
emits shell noise and a different string. Update the setup check so it
explicitly verifies `uv` without triggering `uv: command not found`, and make
the failure path in the relevant setup section print the exact contract message
referenced by the office-pdf runtime contract.
In `@skills/office-pptx/scripts/check_text_budget.py`:
- Around line 40-42: The attr() helper in check_text_budget.py can match short
attribute names inside longer ones, so update the regex in attr() to require a
word boundary before the attribute name for both search patterns. Keep the
existing fallback logic, but make sure names like x, y, dx, dy, and rx only
match exact attributes when parsing source in attr().
In `@skills/office-pptx/scripts/svg_to_pptx/animation_config.py`:
- Around line 204-233: The default transition in build_scaffold is hardcoded to
a value that can fail validation when pptx_animations is unavailable and
TRANSITIONS is empty. Update the defaults block in build_scaffold to choose a
safe fallback transition effect (for example, none) whenever TRANSITIONS has no
supported entries, while keeping fade only when it is actually valid. Use the
build_scaffold function and the TRANSITIONS availability check to locate and
adjust the scaffold’s default transition settings.
In `@skills/office-pptx/scripts/svg_to_pptx/pptx_package/builder.py`:
- Around line 222-243: The icacls subprocess invocation in builder.py can hang
indefinitely during export, so add a timeout to the subprocess.run call in the
ACL-adjustment logic and handle subprocess.TimeoutExpired alongside OSError.
Update the warning path in the same helper that builds the warnings list so a
timeout records a clear message for output_path, then returns warnings instead
of blocking the export.
- Around line 482-514: The relationship matcher in _REL_TARGET_RE is too
restrictive and fails to capture normal OOXML self-closing Relationship elements
because it excludes any slash in the attributes. Update the regex in builder.py
so _verify_internal_rels_targets() can iterate over all <Relationship .../>
entries, then keep the existing Target and TargetMode handling unchanged so
dangling internal targets are still detected correctly.
In `@skills/office-pptx/scripts/svg_to_pptx/pptx_package/media.py`:
- Around line 147-160: The cache write path in media handling discards a
successfully rendered PNG when `os.replace(tmp_path, cached)` fails, because the
temp file is then unlinked and `shutil.copy(cached, png_path)` may run with no
cached file available. Update the logic in the PNG caching flow to preserve the
rendered output by falling back to copying from `tmp_path` directly to
`png_path` when the cache move fails, and only delete the temp file after a
successful copy. Keep the fix localized around the `os.replace`,
`tmp_path.unlink`, and `shutil.copy` sequence in the PNG render/caching
function.
---
Nitpick comments:
In `@skills/office-pptx/scripts/svg_to_pptx/animation_config.py`:
- Around line 191-201: The _validate_scope_effects helper currently skips
malformed transition/animation values when they are not dicts, so add explicit
type warnings for those fields before checking effects. Update
_validate_scope_effects in animation_config.py to detect non-dict
scope['transition'] and scope['animation'] values and append a warning that
includes the label and the unexpected type/value, while keeping the existing
unknown-effect validation for valid dicts.
In `@skills/office-pptx/scripts/svg_to_pptx/pptx_package/cli.py`:
- Around line 45-71: The on-click narration check in
_recorded_narration_on_click_slides is duplicating the same effective animation
resolution logic that already exists in builder._slide_animation_settings, so
the two paths can drift. Refactor the shared CLI/config/override precedence into
a single helper and have both _recorded_narration_on_click_slides and
_slide_animation_settings call it to resolve the effective effect and trigger
consistently. Keep the helper responsible for interpreting animation_config,
animation_cli_overrides, and the animation/animation_trigger inputs so
validation stays aligned with the builder behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f56c4125-4f92-42d6-a0cd-8ff40ce91c3d
📒 Files selected for processing (50)
.gitignorepackages/opencode/src/tool/skill.tspackages/opencode/test/tool/skill.test.tsskills/office-docx/SKILL.mdskills/office-docx/pyproject.tomlskills/office-docx/scripts/check_docx.pyskills/office-pdf/SKILL.mdskills/office-pdf/pyproject.tomlskills/office-pptx/LICENSEskills/office-pptx/SKILL.mdskills/office-pptx/VENDORED.mdskills/office-pptx/pyproject.tomlskills/office-pptx/scripts/check_pptx_assets.pyskills/office-pptx/scripts/check_text_budget.pyskills/office-pptx/scripts/console_encoding.pyskills/office-pptx/scripts/resource_paths.pyskills/office-pptx/scripts/svg_finalize/__init__.pyskills/office-pptx/scripts/svg_finalize/flatten_tspan.pyskills/office-pptx/scripts/svg_to_pptx.pyskills/office-pptx/scripts/svg_to_pptx/__init__.pyskills/office-pptx/scripts/svg_to_pptx/animation_config.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/__init__.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/context.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/converter.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/elements.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/paths.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/styles.pyskills/office-pptx/scripts/svg_to_pptx/drawingml/utils.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/__init__.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/chart_data.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/chart_style.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/chart_xml.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/chartex.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/marker_common.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/table.pyskills/office-pptx/scripts/svg_to_pptx/native_objects/workbook.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/__init__.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/builder.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/cli.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/dimensions.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/discovery.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/media.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/narration.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/notes.pyskills/office-pptx/scripts/svg_to_pptx/pptx_package/slide_xml.pyskills/office-pptx/scripts/svg_to_pptx/tspan_flattener.pyskills/office-pptx/scripts/svg_to_pptx/use_expander.pyskills/office-xlsx/SKILL.mdskills/office-xlsx/pyproject.tomlskills/office-xlsx/scripts/check_xlsx.py
- check_text_budget: anchor attr() on a word boundary so "x" no longer matches inside dx=/rx= and returns the wrong overflow position - builder: fix _REL_TARGET_RE ([^/] stopped at the first slash, so the internal-rels verification never matched real <Relationship .../> elements and silently passed); add a 10s timeout to the icacls ACL grant - media: on cache-write failure, deliver the rendered PNG from the temp file instead of discarding it - check_docx: count body paragraphs excluding Heading styles so a heading-only doc cannot satisfy --min-paragraphs - animation_config: scaffold default transition falls back to 'none' when pptx_animations is absent and TRANSITIONS is empty - office-docx/office-pdf SKILL.md: emit the exact documented missing-uv contract string and suppress the shell's own command-not-found noise
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #1273 productization, PR 3 of 4. Bundles the office document-generation skills into the product as a dark launch — the skills ship, are discoverable, and load at runtime, but no default routing/entry is changed and no
officeclicode is removed (the switch is PR 4).Four new skills under repo-root
skills/(ships as electronextraResources, scanned by the builtin skill loader, pattern*/SKILL.md):office-pptxskills/office-pptx/svg_to_pptx(SVG per slide → native editable PPTX)office-xlsxskills/office-xlsx/openpyxloffice-docxskills/office-docx/python-docxoffice-pdfskills/office-pdf/pdfplumber/pypdfThe PPT skill is productized from
codex/i1273-office-eval-baseline: same vendored converter and machine gates, SKILL.md reworded from eval language to a product flow, evalSVG_PPTX_SCRIPTenv var replaced by stable in-skill script paths. UpstreamLICENSEandVENDORED.mdprovenance preserved.Includes one tiny runtime change:
packages/opencode/src/tool/skill.tsnow prints the skill base directory as a plain filesystem path in addition to the existingfile://URL line (the URL line and its test assertion are unchanged). Bundled skills reference that path in shell commands; the URL form alone broke that contract.Dark launch — mechanism and known limitation
Skill selection is description-driven: the system prompt advertises every skill with a
descriptionand the model self-selects. Each new skill's description is prefixedPREVIEW / dark-launched … NOT the default … keep using officecli-*. Naming isoffice-*(notofficecli-*/morph-ppt*), so the officecli sync/prune script ignores these dirs and the skill-test scope-drift guard does not count them.Known limitation (explicit, also stated at the top of each SKILL.md): the preview status is enforced only by description text — there is no hard runtime switch, and the four skills appear in the /skills user page like any other skill. Evaluated hard-hide options: (a) a
previewfrontmatter flag filtered inSkill.available/Skill.fmtplus the app skills page — touches the skill Info schema, system prompt assembly, UI, and their tests; (b) droppingdescription— hides from the model prompt but makes the route undiscoverable and still shows in the UI. Both exceed this PR's scope; the hard on/off switch belongs to PR 4 with the routing change.Contract (aligned with PR 2)
Each skill assumes only that:
uvis already on the execution environment'sPATH.Every SKILL.md has an explicit uv-missing error contract: if
uv --versionfails, stop and report an environment error; never fall back to system pip /officecli/ an AGPL library. All Python in every skill runs throughuv run— including the stdlib-only gates (Windows packaging does not guarantee apython3binary;uvis the provisioned runtime).office-pdfbans PyMuPDF /fitz(AGPL) outright; allowed parsers arepdfplumber(MIT) andpypdf(BSD), generation uses the bundled Chromium only.SKILL_DIRcontract: SKILL.md instructs the model to use the new plain-filesystem-path line from the skill tool output (never thefile://URL; with a strip-prefix fallback if only the URL is visible).Key disciplines (machine-check over model judgement)
viewBox="0 0 1280 720"; px font floor (title ≥ 60px / body ≥ 24px, title ≥ 2× body); per-line character budget; speaker-note sidecars; native chart viadata-pptx-native+ JSON metadata; embedded images. Two gates that must printOK, both viauv run:check_text_budget.py(pre-convert; tolerates60px-style unit suffixes) andcheck_pptx_assets.py(pre-deliver; slide/notes/chart/media counts plus minimalartifact-summary.jsonschema:rendererstring + non-emptyslidesarray).=/+/-/@must be force-stored as text; links viacell.hyperlink, neverHYPERLINK()).check_xlsx.pygate counts charts fromxl/charts/chart*.xmlpackage parts (no reliance on openpyxl's private_chartsread-back) and FAILs on injection-style formulas (HYPERLINK/WEBSERVICE/IMPORT*/DDE patterns).check_docx.pygate.Testing evidence
check_pptx_assets.py(viauv run) verified FAIL on missing/invalidartifact-summary.jsonand OK on a valid deck. Unit-suffixedx="80px" font-size="60px"no longer tracebacks.=HYPERLINK(...)formula; OK when the same user string is force-stored as text (data_type="s"); FAIL on--require-formulawith hard-coded values. Chart round-trip verified for bar/line/pie/area/scatter on openpyxl 3.1.5 before switching to package-part counting.bun test test/tool/skill.test.ts test/skill/skill.test.ts→ 21 pass (includes new plain-path assertion and the test scanning the real reposkills/dir).bun test scripts/sync-officecli-skills.test.ts→ 25 pass.packages/opencodebun run typecheck(tsgo) → clean.opencode debug skilllists all fouroffice-*skills next to the untouchedofficecli-*skills with PREVIEW descriptions.Not in this PR
office-pdf's generation path documents the intended bundled-Chromium CDP mechanism and instructs the model to report unavailability rather than shell out to an unbundled browser. The parse path is fully usable today.Summary by CodeRabbit
New Features
Bug Fixes
Chores