⬆️(backend) upgrade docspec to v3.0.0 and adapt converter API#2220
⬆️(backend) upgrade docspec to v3.0.0 and adapt converter API#2220StephanMeijer wants to merge 1 commit intosuitenumerique:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughDocSpec runtime image updated to 3.0.0 in compose.yml and Helm value files. Backend converter changed: DOCX→YJS conversion forwards the incoming Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/backend/core/services/converter_services.py (1)
69-73: Prefer the shared BlockNote constant and lock this request shape down with a test.This media type is now duplicated here instead of reusing
mime_types.BLOCKNOTE, and the providedsrc/backend/core/tests/test_services_docspec_converter.py:1-30coverage only validates inputs, not the actual wire contract. Reusing the constant plus a patchedrequests.posttest would make future v2/v3 regressions much harder to reintroduce.Small cleanup
response = requests.post( url, headers={ "Content-Type": content_type, - "Accept": "application/vnd.docspec.blocknote+json", + "Accept": mime_types.BLOCKNOTE, }, data=data, timeout=settings.CONVERSION_API_TIMEOUT, verify=settings.CONVERSION_API_SECURE, )Example regression test
from unittest.mock import Mock, patch from core.services import mime_types from core.services.converter_services import DocSpecConverter `@patch`("core.services.converter_services.requests.post") def test_docspec_convert_uses_raw_body_and_explicit_headers(mock_post, settings): mock_post.return_value = Mock(ok=True, content=b"[]") converter = DocSpecConverter() converter.convert(b"docx-bytes", mime_types.DOCX, mime_types.BLOCKNOTE) mock_post.assert_called_once_with( settings.DOCSPEC_API_URL, headers={ "Content-Type": mime_types.DOCX, "Accept": mime_types.BLOCKNOTE, }, data=b"docx-bytes", timeout=settings.CONVERSION_API_TIMEOUT, verify=settings.CONVERSION_API_SECURE, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/core/services/converter_services.py` around lines 69 - 73, Replace the hardcoded Accept media type with the shared constant and add a unit test that patches requests.post to lock the wire contract: in core.services.converter_services update DocSpecConverter.convert to use mime_types.BLOCKNOTE (instead of the literal "application/vnd.docspec.blocknote+json") when building headers, and create a patched test (patching core.services.converter_services.requests.post) that asserts requests.post was called with settings.DOCSPEC_API_URL, headers using mime_types.DOCX and mime_types.BLOCKNOTE, data equal to the raw body, and the timeout/verify kwargs (settings.CONVERSION_API_TIMEOUT and settings.CONVERSION_API_SECURE).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compose.yml`:
- Line 247: Update the Helm defaults so the DocSpec image tag matches the
compose.yml change to 3.0.0: locate the docSpec.image.tag entries in
src/helm/impress/values.yaml and in the two environment overrides
src/helm/env.d/dev/values.impress.yaml.gotmpl and
src/helm/env.d/feature/values.impress.yaml.gotmpl (search for docSpec.image.tag
or tag: "2.6.3") and change their tag values from "2.6.3" to "3.0.0" so the Helm
charts use DocSpec v3 for backend wire format compatibility.
---
Nitpick comments:
In `@src/backend/core/services/converter_services.py`:
- Around line 69-73: Replace the hardcoded Accept media type with the shared
constant and add a unit test that patches requests.post to lock the wire
contract: in core.services.converter_services update DocSpecConverter.convert to
use mime_types.BLOCKNOTE (instead of the literal
"application/vnd.docspec.blocknote+json") when building headers, and create a
patched test (patching core.services.converter_services.requests.post) that
asserts requests.post was called with settings.DOCSPEC_API_URL, headers using
mime_types.DOCX and mime_types.BLOCKNOTE, data equal to the raw body, and the
timeout/verify kwargs (settings.CONVERSION_API_TIMEOUT and
settings.CONVERSION_API_SECURE).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4062ae5f-c69b-47ca-b325-3554c894e54f
📒 Files selected for processing (2)
compose.ymlsrc/backend/core/services/converter_services.py
There was a problem hiding this comment.
Pull request overview
Upgrades the DocSpec conversion service to v3.0.0 and updates the backend DocSpec converter client to use the new raw-body upload API with explicit Content-Type/Accept headers.
Changes:
- Updated
DocSpecConverterto POST the document as a raw request body instead of multipart form upload. - Adjusted DocSpec request headers to include explicit
Content-TypeandAccept. - Bumped the DocSpec Docker image tag in local
compose.ymlto3.0.0.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/backend/core/services/converter_services.py | Updates DocSpec request format/headers to match the v3 API. |
| compose.yml | Bumps the DocSpec service image tag to ghcr.io/docspecio/api:3.0.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
StephanMeijer
force-pushed
the
feature/upgrade-docspec-api
branch
from
4557a8d to
c3b1e31
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/helm/env.d/dev/values.impress.yaml.gotmpl (1)
182-182: Roll this image bump in the same release as backend converter changes.Because docspec v3 is incompatible with the v2 request format, deploying this value change independently can break conversions. Keep rollout atomic (chart + backend code together).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/helm/env.d/dev/values.impress.yaml.gotmpl` at line 182, The image tag bump to "3.0.0" in the Helm values (tag: "3.0.0") must be deployed atomically with the backend converter changes that update request handling for docspec v3; update your release/PR so the chart values change and the backend converter code changes are merged and released together, or add a deployment gating step (e.g., CI check or release note) that prevents rolling the Helm value without the corresponding converter commit, ensuring the v3 image and backend converter are released in the same atomic rollout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/helm/env.d/dev/values.impress.yaml.gotmpl`:
- Line 182: The image tag bump to "3.0.0" in the Helm values (tag: "3.0.0") must
be deployed atomically with the backend converter changes that update request
handling for docspec v3; update your release/PR so the chart values change and
the backend converter code changes are merged and released together, or add a
deployment gating step (e.g., CI check or release note) that prevents rolling
the Helm value without the corresponding converter commit, ensuring the v3 image
and backend converter are released in the same atomic rollout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7ea6d2b4-eaf8-403e-8e40-79f0e4d665f5
📒 Files selected for processing (7)
CHANGELOG.mdcompose.ymlsrc/backend/core/services/converter_services.pysrc/backend/core/tests/test_services_docspec_converter.pysrc/helm/env.d/dev/values.impress.yaml.gotmplsrc/helm/env.d/feature/values.impress.yaml.gotmplsrc/helm/impress/values.yaml
✅ Files skipped from review due to trivial changes (3)
- src/helm/env.d/feature/values.impress.yaml.gotmpl
- CHANGELOG.md
- src/helm/impress/values.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- src/backend/core/services/converter_services.py
- compose.yml
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
StephanMeijer
force-pushed
the
feature/upgrade-docspec-api
branch
from
c3b1e31 to
604694b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Bump docspec Docker image from 2.6.3 to 3.0.0 and update DocSpecConverter to match the new API: switch from multipart form upload to raw body with explicit Content-Type/Accept headers. Also update Helm chart values to match. The Docker image must be updated alongside the code changes, as the new request format is incompatible with v2.x.
StephanMeijer
force-pushed
the
feature/upgrade-docspec-api
branch
from
604694b to
e4f25ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 21-23: The CHANGELOG entry introduces a duplicate "### Changed"
heading which triggers markdownlint MD024; move the bullet "- ⬆️(backend)
upgrade docspec to v3.0.0 and adapt converter API `#2220`" into the existing "###
Changed" list under the "## [Unreleased]" section (remove the extra "###
Changed" heading and append the bullet to the current list) so there is only one
"### Changed" heading in the Unreleased section.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e923efaa-0ddf-4dc5-abec-751907246f21
📒 Files selected for processing (7)
CHANGELOG.mdcompose.ymlsrc/backend/core/services/converter_services.pysrc/backend/core/tests/test_services_docspec_converter.pysrc/helm/env.d/dev/values.impress.yaml.gotmplsrc/helm/env.d/feature/values.impress.yaml.gotmplsrc/helm/impress/values.yaml
✅ Files skipped from review due to trivial changes (1)
- src/helm/env.d/feature/values.impress.yaml.gotmpl
🚧 Files skipped from review as they are similar to previous changes (4)
- src/backend/core/tests/test_services_docspec_converter.py
- src/helm/impress/values.yaml
- compose.yml
- src/helm/env.d/dev/values.impress.yaml.gotmpl
Summary
2.6.3to3.0.0and adaptDocSpecConverterto the new API (raw body upload with explicitContent-Type/Acceptheaders instead of multipart form)Important
The Docker image (
ghcr.io/docspecio/api:3.0.0) must be updated alongside the code changes. The new request format is incompatible with v2.x — deploying only the code without updating the image (or vice versa) will break document conversion.External contributions
Please ensure the following items are checked before submitting your pull request:
General requirements
CI requirements
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title descriptionI have added a changelog entry under## [Unreleased]section (if noticeable change)AI requirements