feat: add artifact and status contract helpers#39
Merged
Conversation
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
==========================================
- Coverage 91.22% 90.97% -0.26%
==========================================
Files 81 83 +2
Lines 3464 3644 +180
==========================================
+ Hits 3160 3315 +155
- Misses 304 329 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
MapleEve
force-pushed
the
feat/0.8.3-artifact-status-contracts
branch
from
b554e62 to
8bac193
Compare
There was a problem hiding this comment.
Pull request overview
Adds stable “contract helper” layers for artifact manifests, persisted job status payloads, and optional schema-version handling, with optional Rust-backed validation/normalization when RUST_KERNEL_MODE=required. This aims to keep public payload shapes backward compatible while preventing non-public fields (notably paths) from leaking through normalization.
Changes:
- Introduces Python contract helpers for artifact manifests (
empty_*/normalize_*), persisted status payloads, and optional schema versions. - Adds Rust-side contract helpers (PyO3 exports) plus Python kernel-bridge wrappers/validators for artifact manifest + status payload contracts.
- Updates tests/docs/CI smoke to cover the new contract behaviors and Rust bridge paths.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_pipeline_runner.py | Adds coverage ensuring artifact manifests flow through the Rust contract path when required. |
| tests/unit/test_kernel_bridge.py | Adds tests for new Rust bridge contract helpers and bumps expected core version. |
| tests/unit/test_artifact_status_schema_contracts.py | New contract tests for artifact manifest/status/schema helpers and legacy normalization. |
| doc/configuration.zh.md | Documents artifact manifest helper contract under RUST_KERNEL_MODE. |
| doc/configuration.en.md | Documents artifact manifest helper contract under RUST_KERNEL_MODE. |
| doc/changelog.zh.md | Changelog entries for new helper contracts + Rust validation coverage. |
| doc/changelog.en.md | Changelog entries for new helper contracts + Rust validation coverage. |
| crates/voscript_core/src/lib.rs | Exposes new contracts module and PyO3 functions for contract helpers. |
| crates/voscript_core/src/contracts.rs | New Rust contract utilities for public-safe manifest/status fields and status normalization. |
| crates/voscript_core/Cargo.toml | Bumps voscript_core version to 0.8.3. |
| Cargo.lock | Updates lockfile for voscript_core 0.8.3. |
| app/providers/kernel_bridge/runtime.py | Adds Rust bridge calls + response validators for artifact manifest and status payload contracts. |
| app/providers/kernel_bridge/init.py | Re-exports new kernel-bridge helper contracts. |
| app/providers/artifacts/default.py | Runs artifact manifests through Rust contract when Rust provider paths are required. |
| app/pipeline/contracts/status.py | New stable helper for building/normalizing persisted status.json. |
| app/pipeline/contracts/schema.py | New optional-first schema version helper. |
| app/pipeline/contracts/artifacts.py | Adds public-safe manifest normalization + helpers and exports categories. |
| app/pipeline/contracts/init.py | Exposes new contract helpers at the contracts package boundary. |
| app/infra/job_persistence.py | Switches persisted status writing/recovery reads to contract helpers. |
| app/api/routers/transcriptions.py | Normalizes persisted status payload reads before returning job status. |
| .github/workflows/rust-foundation-heavy.yml | Extends Rust-required smoke to cover new contract helper calls. |
| .env.example | Documents artifact manifest helper contracts in selected Rust paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+31
| def _public_safe_filename(value: Any) -> str: | ||
| text = _public_safe_text(value, "filename") | ||
| # Filenames in the manifest are artifact names only, never host-local paths. | ||
| if _UNSAFE_FILENAME_RE.search(text) or PurePosixPath(text).name != text: | ||
| raise ValueError("artifact manifest filename must not include a path or URL") | ||
| return text |
Comment on lines
+110
to
+121
| return cls( | ||
| name=_public_safe_text(payload.get("name"), "name"), | ||
| filename=_public_safe_filename(payload.get("filename")), | ||
| role=_public_safe_text(payload.get("role"), "role"), | ||
| media_type=_public_safe_text(payload.get("media_type"), "media_type"), | ||
| required_for_result=bool(payload.get("required_for_result", False)), | ||
| speaker_label=( | ||
| _public_safe_text(payload.get("speaker_label"), "speaker_label") | ||
| if payload.get("speaker_label") is not None | ||
| else None | ||
| ), | ||
| ) |
Comment on lines
+265
to
+272
| if ( | ||
| "/" in result["filename"] | ||
| or "\\" in result["filename"] | ||
| or "://" in result["filename"] | ||
| ): | ||
| raise RustKernelBridgeError( | ||
| "Rust artifact_manifest_contract filename must not expose a path" | ||
| ) |
Comment on lines
+307
to
+311
| if "filename" in result and not isinstance(result["filename"], str): | ||
| raise RustKernelBridgeError( | ||
| "Rust status_payload_contract filename must be string" | ||
| ) | ||
| return result |
MapleEve
force-pushed
the
feat/0.8.3-artifact-status-contracts
branch
from
8bac193 to
6d1a2dd
Compare
|
❤️ Great PR @MapleEve ❤️ The growth of the project is inseparable from user feedback and contribution. Thanks for your contribution! |
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
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
Validation
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/unit/test_artifact_status_schema_contracts.py tests/unit/test_kernel_bridge.py tests/unit/test_pipeline_runner.py -q --tb=shortPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/unit/ tests/test_security.py tests/test_voiceprint_db.py tests/test_job_service.py -q --tb=shortruff format --check app/ tests/unit/test_artifact_status_schema_contracts.py tests/unit/test_kernel_bridge.py tests/unit/test_pipeline_runner.pyruff check app/ tests/unit/test_artifact_status_schema_contracts.py tests/unit/test_kernel_bridge.py tests/unit/test_pipeline_runner.py --ignore E501cargo test --manifest-path crates/voscript_core/Cargo.tomlcargo fmt --manifest-path crates/voscript_core/Cargo.toml -- --checkcargo clippy --manifest-path crates/voscript_core/Cargo.toml --features python-bindings --all-targets -- -D warningspublic_release_scan.py --root <repo-root>git diff --check