fix(slides): migrate SML namespace from HTTP to HTTPS - #2169
Open
ethan-zhx wants to merge 2 commits into
Open
Conversation
- Change canonical namespace to https://www.larkoffice.com/sml/2.0 in protocol schema, production code, docs, and tests - Keep HTTP and /sml/2.0 as legacy readback compat in validator - Fix sml_prefixed_tag check to cover all accepted SML namespaces - Add regression test for legacy HTTP namespace acceptance
github-actions
Bot
added
the
size/L
📝 WalkthroughWalkthroughThe Slides XML namespace now uses HTTPS as the canonical URI. Validators accept HTTPS, legacy HTTP, and readback namespaces. Test fixtures, end-to-end tests, schemas, examples, and validation guidance reflect the updated namespace. ChangesSlides SML namespace
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/lark-slides/scripts/xml_text_overlap_lint_test.py (1)
289-305: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExtend the regression test to cover prefixed legacy namespaces.
These lines cover only an unprefixed HTTP root. The production change in
skills/lark-slides/scripts/xml_text_overlap_lint.pyLine 686 also changes prefixed-tag validation to useACCEPTED_SML_NAMESPACES. Add cases for prefixed elements bound to the legacy HTTP URI and/sml/2.0, and assertsml_prefixed_tag. Otherwise, reverting the production change would still leave this test green.🤖 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/lark-slides/scripts/xml_text_overlap_lint_test.py` around lines 289 - 305, Extend test_lint_xml_accepts_legacy_http_namespace with prefixed XML cases bound to both legacy HTTP namespace URIs, including /sml/2.0, and assert the resulting sml_prefixed_tag behavior. Ensure the test would fail if prefixed-tag validation no longer uses ACCEPTED_SML_NAMESPACES, while preserving the existing unprefixed acceptance assertions.
🤖 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.
Nitpick comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint_test.py`:
- Around line 289-305: Extend test_lint_xml_accepts_legacy_http_namespace with
prefixed XML cases bound to both legacy HTTP namespace URIs, including /sml/2.0,
and assert the resulting sml_prefixed_tag behavior. Ensure the test would fail
if prefixed-tag validation no longer uses ACCEPTED_SML_NAMESPACES, while
preserving the existing unprefixed acceptance assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ee5e0cd-d976-4b7c-83a8-599df1f17afa
📒 Files selected for processing (20)
shortcuts/slides/slides_add_slide_test.goshortcuts/slides/slides_create.goshortcuts/slides/slides_create_test.goshortcuts/slides/slides_replace_pages_test.goshortcuts/slides/slides_screenshot_test.goskills/lark-slides/references/lark-slides-add-slide.mdskills/lark-slides/references/lark-slides-create.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-xml-presentations-get.mdskills/lark-slides/references/slides_chart_demo.xmlskills/lark-slides/references/slides_xml_schema_definition.xmlskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/scripts/sxsd_validator.pyskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.pytests/cli_e2e/slides/slides_create_workflow_test.gotests/cli_e2e/slides/slides_history_workflow_test.gotests/cli_e2e/slides/slides_slide_add_delete_dryrun_test.gotests/cli_e2e/slides/slides_slide_add_delete_workflow_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@55b6f58a914dd674a52c32eb90d53bdecc462a89🧩 Skill updatenpx skills add larksuite/cli#fix/slides_sml_namespace -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2169 +/- ##
==========================================
+ Coverage 75.65% 75.69% +0.03%
==========================================
Files 940 942 +2
Lines 99876 100079 +203
==========================================
+ Hits 75564 75750 +186
- Misses 18530 18537 +7
- Partials 5782 5792 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Migrate the SML 2.0 namespace from
http://www.larkoffice.com/sml/2.0tohttps://www.larkoffice.com/sml/2.0across the entire codebase: protocol schema, production code, Skill docs, and all tests.Changes
targetNamespaceandxmlns:sml→ HTTPSbuildPresentationXMLinslides_create.gonow emits HTTPSSML_NAMESPACEis now HTTPS; old HTTP URI kept asSML_LEGACY_HTTP_NAMESPACE;ACCEPTED_SML_NAMESPACESretains three-format compat (HTTPS, HTTP,/sml/2.0)SML_NAMESPACE→ HTTPS;validate_sml_tag_prefixesnow checks against all accepted namespaces (was previously HTTP-only, missing HTTPS-prefixed tags)slides_chart_demo.xmlchanged from/sml/2.0to HTTPStest_lint_xml_accepts_legacy_http_namespaceregression testDesign
/sml/2.0remain accepted by the lint validator for backward compatibility with existing contentVerification
python3 -m unittest xml_text_overlap_lint_test— 185 tests passSummary by CodeRabbit
Bug Fixes
Documentation
Tests