Skip to content

Conversation

@sunxilin
Copy link
Contributor

@sunxilin sunxilin commented Dec 2, 2025

No description provided.

@sunxilin sunxilin self-assigned this Dec 2, 2025
@claude
Copy link

claude bot commented Dec 2, 2025

PR Review: Remove version lock on ten_ai_base

Summary

This PR changes the ten_ai_base dependency version constraint from =0.7.22 (exact version) to 0.7 (minor version range) across 80 manifest files. This is a dependency management improvement that relaxes version constraints to allow more flexibility.


✅ Code Quality & Best Practices

Positive aspects:

  • Consistency: The change is applied uniformly across all 80 files, maintaining consistency throughout the codebase
  • Semantic versioning: Using 0.7 follows semantic versioning principles, allowing patch updates (0.7.x) while staying within the same minor version
  • Reduced maintenance: This eliminates the need to update every manifest when releasing patch versions of ten_ai_base

Observations:

  • Two extensions also bumped their own versions:
    • aliyun_asr_bigmodel_python: 0.2.40.2.5
    • minimax_tts_websocket_python: Significant manifest changes (added many new properties)

🔍 Potential Issues & Concerns

1. Version Semantics Clarification Needed

The change from =0.7.22 to 0.7 needs clarification on the TEN Framework's version resolution semantics:

  • Does "version": "0.7" mean:
    • >=0.7.0 <0.8.0 (any 0.7.x version)?
    • >=0.7.22 (minimum 0.7.22, but allows newer)?
    • Exactly 0.7.0 (which would be a downgrade)?

Recommendation: Add a comment in the PR description explaining what 0.7 resolves to in the TEN package manager (tman). If this allows 0.7.0 - 0.7.21, that could introduce compatibility issues since extensions were previously locked to 0.7.22.

2. Breaking Change Risk

High concern: If 0.7 can resolve to versions older than 0.7.22, extensions may fail if they depend on features/fixes introduced in 0.7.22.

Recommendation:

  • If tman supports it, use >=0.7.22 <0.8.0 or ^0.7.22 to ensure minimum version requirements
  • Add integration tests to verify extensions work with the full range of 0.7.x versions
  • Document the minimum supported ten_ai_base version in CLAUDE.md or project docs

3. Missing Test Coverage

The PR contains no test changes or additions:

  • No verification that extensions work with different 0.7.x versions
  • No CI/CD changes to test against multiple versions
  • Integration tests (ASR guarder, TTS guarder) aren't updated

Recommendation: Add automated testing that validates extensions against:

  • The current ten_ai_base version in the repo
  • Oldest supported 0.7.x version (if allowing downgrades)
  • Latest 0.7.x version available

⚡ Performance Considerations

Neutral impact: Version resolution happens at build/install time via tman, not runtime. No performance concerns.


🔒 Security Concerns

Low risk: This change doesn't introduce security vulnerabilities. However:

  • If older 0.7.x versions contain security fixes that were patched in 0.7.22, allowing downgrades could reintroduce those vulnerabilities
  • Recommendation: Document minimum secure version and consider using >=0.7.22 constraint

📋 Additional Observations

  1. Extension version bumps: The aliyun_asr_bigmodel_python version bump from 0.2.4 to 0.2.5 suggests this change may be considered a minor update to extensions. Consider whether all extensions should bump patch versions.

  2. Minimax changes: The minimax_tts_websocket_python/manifest.json shows 62 additions and 22 deletions. These appear to be new property definitions added to the API schema (new TTS parameters like stability, style, speaker_boost, etc.). This seems unrelated to the version lock removal and might warrant a separate PR.

  3. Commit message: The commit message "fix: remove the version lock on ten_ai_base" uses "fix:" prefix. This suggests it's treating the exact version as a bug. Consider if "chore:" or "refactor:" would be more appropriate since this is dependency management rather than a bug fix.


🎯 Recommendations Summary

Before merging:

  1. ✅ Clarify version resolution semantics in PR description
  2. ✅ Verify minimum version requirement (suggest >=0.7.22 <0.8.0 if supported)
  3. ✅ Add integration test coverage for version compatibility
  4. ✅ Document minimum ten_ai_base version requirements
  5. ⚠️ Consider separating the minimax changes into a separate PR
  6. ✅ Update commit message prefix if this isn't truly a "fix"

Overall assessment: The intent is good - loosening version constraints improves maintainability. However, the PR needs more context about version semantics and testing to ensure it doesn't inadvertently introduce compatibility issues.


🤖 Generated with Claude Code

@claude
Copy link

claude bot commented Dec 2, 2025

Follow-up Review: Additional Analysis

Since my initial review, I've analyzed the changes more thoroughly. Here are additional findings:

✅ Positive Observations

  1. CI/CD is Running: The PR has triggered extensive build tests across all example applications (demo, voice-assistant, voice-assistant-realtime, etc.). This provides confidence that the version change doesn't break existing functionality.

  2. Linting Passed: The lint and commitlint checks have passed, confirming code quality standards are maintained.

  3. Consistent Application: All 80 manifest files have been updated uniformly, showing thorough attention to detail.

🔍 Key Technical Points

Version Constraint Semantics

Based on the TEN Framework's manifest structure and the CLAUDE.md documentation, the version change from =0.7.22 to 0.7 appears to follow a minor version range pattern. This likely means:

  • Before: =0.7.22 (exact match only)
  • After: 0.7 (likely >=0.7.0 <0.8.0 or similar)

Critical Question: Does the TEN package manager (tman) resolve 0.7 to:

  • The latest available 0.7.x version? (Best practice)
  • Any 0.7.x version including older ones? (Risky)
  • A specific version like 0.7.0? (Would be a downgrade)

📊 Testing Status

Ongoing CI Checks:

  • ✅ Commitlint: PASSED
  • ✅ Lint: PASSED
  • ✅ Check PR Status: PASSED
  • ⏳ Build tests for 15+ examples: IN_PROGRESS
  • ⏳ CI for voice-assistant examples: IN_PROGRESS

Recommendation: Wait for all build tests to complete before merging. If any builds fail, it would indicate that the version relaxation exposes compatibility issues.

🎯 Outstanding Questions

  1. Version Resolution: Please document in the PR description how tman resolves 0.7 - does it pick the latest 0.7.x, or can it select older versions?

  2. Minimum Version: If extensions depend on features from 0.7.22, should the constraint be >=0.7.22 instead of 0.7?

  3. Minimax Changes: The minimax_tts_websocket_python manifest has 62 additions unrelated to version changes. These new properties (stability, style, speaker_boost, etc.) should ideally be in a separate PR for cleaner history.

  4. Extension Version Bump: Why did aliyun_asr_bigmodel_python get bumped to 0.2.5? Should other extensions also bump their versions when dependency constraints change?

📋 Pre-Merge Checklist

Before approving this PR, I recommend:

  • Clarify tman version resolution behavior
  • Verify all build tests pass (currently in progress)
  • Document the rationale for removing the exact version lock
  • Consider if >=0.7.22 would be more appropriate to prevent downgrades
  • Optionally: Split minimax changes into a separate PR

💡 Alternative Approach

If the goal is to allow patch updates while maintaining a minimum version, consider:

{
  "type": "system",
  "name": "ten_ai_base",
  "version": ">=0.7.22 <0.8.0"
}

This would be more explicit about intentions and prevent accidental downgrades.


Overall: The change is well-executed mechanically, but needs clarification on version semantics and justification for the approach. Once build tests complete successfully and version behavior is documented, this should be safe to merge.


🤖 Generated with Claude Code

@sunxilin sunxilin marked this pull request as draft December 4, 2025 12:39
@claude
Copy link

claude bot commented Dec 4, 2025

Pull Request Review: Remove version lock on ten_ai_base

Overview

This PR changes the dependency version constraint for ten_ai_base from an exact version lock (=0.7.22) to a minor version range (0.7) across 80 manifest files. This is a systematic refactoring that affects examples, extensions, and the main agents manifest.


✅ Code Quality & Best Practices

Strengths:

  • Consistent application: The change is applied uniformly across all manifest files (79 files changed from =0.7.22 to 0.7, plus 1 extension version bump)
  • Follows semantic versioning: Using 0.7 allows patch version updates while maintaining minor version compatibility
  • Reduces maintenance burden: No longer need to update every manifest when patch versions of ten_ai_base are released
  • Aligns with existing patterns: Other dependencies like ten_runtime_python already use this versioning style (0.11 without exact locks)

Observations:

  • The aliyun_asr_bigmodel_python extension was also bumped from 0.2.4 to 0.2.5, which appears to be a separate change bundled with this PR
  • All changes are to manifest.json files only - no code changes required

🐛 Potential Issues

Minor concerns:

  1. Bundled version bump: The aliyun_asr_bigmodel_python extension version bump (0.2.40.2.5) is included in this PR but not mentioned in the commit message. Consider:

    • Adding a note to explain why this extension was bumped
    • OR separating this into its own commit/PR for clarity
  2. Backward compatibility risk: Changing from exact version (=0.7.22) to range (0.7) means:

    • Users could get any 0.7.x version, including future patch releases
    • If ten_ai_base 0.7.23+ introduces breaking changes (despite being a patch), this could cause issues
    • Recommendation: Ensure ten_ai_base follows strict semantic versioning where patches never break compatibility
  3. Missing context: The PR description is empty. Consider adding:

    • Rationale for removing the version lock
    • Testing performed to verify compatibility
    • Any known compatibility ranges

🔒 Security Concerns

No security issues identified:

  • Changes are purely to dependency version constraints
  • No code execution paths modified
  • No credentials or sensitive data exposed

⚡ Performance Considerations

No performance impact:

  • Manifest files are read at build/install time only
  • Version resolution happens during tman install
  • No runtime performance changes

🧪 Test Coverage

Testing recommendations:

  1. Verify dependency resolution: Ensure tman install correctly resolves ten_ai_base version 0.7.x in various scenarios:

    # Test in a few representative examples
    cd ai_agents/agents/examples/voice-assistant/tenapp
    tman install
    # Verify correct version installed
  2. Integration testing: Run at least one example end-to-end to confirm no breakage:

    task test-extension EXTENSION=agents/ten_packages/extension/openai_llm2_python
  3. Cross-version compatibility: Test with current 0.7.22 and any newer 0.7.x versions if available


📋 Additional Recommendations

  1. Documentation: Update any documentation that references the version lock pattern for ten_ai_base

  2. Changelog: Add entry noting the dependency constraint change

  3. Communication: If this is a breaking change for users who explicitly relied on =0.7.22, consider:

    • Release notes mentioning the change
    • Migration guide if needed
  4. Future consistency: Establish a policy for when to use exact versions (=x.y.z) vs ranges (x.y) to avoid inconsistent patterns in the future


📝 Summary

Approval Status: ✅ Approve with minor suggestions

This is a clean, systematic refactoring that improves maintainability. The changes are low-risk and follow existing patterns in the codebase. The main suggestions are:

  • Clarify the aliyun_asr_bigmodel_python version bump
  • Add PR description with context
  • Verify testing has been performed

The change makes sense and aligns with the goal of reducing maintenance overhead while maintaining compatibility guarantees through semantic versioning.


Files reviewed: 80/80 ✓
Pattern consistency: ✓
Breaking changes: None (assuming semantic versioning compliance)
Recommendation: Merge after addressing description/documentation suggestions

@sunxilin sunxilin marked this pull request as ready for review December 9, 2025 02:37
@claude
Copy link

claude bot commented Dec 9, 2025

Pull Request Review: Remove version lock on ten_ai_base

Summary

This PR removes exact version pinning (=0.7.22) from the ten_ai_base dependency across 80 manifest files, replacing it with a more flexible minor version constraint (0.7). This is a maintenance improvement that affects dependency management across the entire AI agents ecosystem.

✅ Code Quality and Best Practices

Strengths:

  • Consistency: The change is applied uniformly across all 80+ affected manifest files
  • Semantic versioning: Using "0.7" follows semantic versioning principles, allowing patch updates while maintaining API compatibility within the 0.7.x minor version
  • Reduced maintenance burden: Eliminates the need to update every manifest when releasing patch versions of ten_ai_base

Observations:

  • The extension aliyun_asr_bigmodel_python also had its own version bumped from 0.2.4 to 0.2.5, which appears intentional and appropriate
  • One file (lightrag_graph_memory_python/manifest.json) shows a larger diff (84 lines) - this appears to be formatting/structural changes beyond just the version update

⚠️ Potential Issues

1. Version Constraint Semantics
The change from "=0.7.22" to "0.7" needs clarification:

  • Does "0.7" mean >=0.7.0 <0.8.0 (recommended for semver minor version)?
  • Or does it mean ~0.7.0 (only patch updates)?
  • Or does it mean exactly 0.7.0?

Recommendation: Document the version constraint syntax used by the TEN package manager (tman) to ensure this change has the intended effect. If "0.7" is interpreted as exactly 0.7.0, this would be more restrictive than the current =0.7.22.

2. Missing Test Coverage

  • No test changes are included in this PR
  • There's no evidence of CI runs or validation that existing applications still build/run correctly with the relaxed version constraint
  • The repository has integration test infrastructure (asr_guarder, tts_guarder) that should verify extensions still work

Recommendation:

  • Run the existing test suite to validate the change
  • Consider adding a CI check that validates version constraints are properly formatted
  • Test at least one example application (e.g., voice-assistant) to ensure tman install resolves dependencies correctly

3. Incomplete Change Coverage
Looking at the patch, I notice that only ten_ai_base dependencies are updated, but extensions still pin to specific versions of ten_runtime_python:

{
  "type": "system",
  "name": "ten_runtime_python",
  "version": "0.11"  // Still exact version
}

Question: Should ten_runtime_python dependencies also be relaxed for consistency? Or is there a reason to keep runtime versions strictly pinned?

4. Dependency Resolution Concerns
With 60+ extensions now accepting any 0.7.x version of ten_ai_base:

  • What happens if different extensions in the same application graph require incompatible patch versions?
  • Does tman perform dependency resolution to find a compatible version across all extensions?
  • Is there a lock file mechanism (like manifest-lock.json) to ensure reproducible builds?

🔒 Security Considerations

Low Risk: This is a dependency management change with minimal security impact. However:

  • Allowing flexible patch versions means applications could automatically pull in newer ten_ai_base versions
  • Ensure there's a process to vet and test patch releases of ten_ai_base before they're published
  • Consider whether applications should have a lock file to pin exact resolved versions in production

⚡ Performance Considerations

No Direct Impact: This is a metadata change and won't affect runtime performance. The actual version of ten_ai_base used will determine performance characteristics.

🧪 Test Coverage

Missing:

  • No test changes included in the PR
  • No evidence of validation runs
  • The PR description is empty, providing no context about testing performed

Recommendations:

  1. Run task test to validate all extensions still build
  2. Test at least one example application end-to-end:
    cd ai_agents/agents/examples/voice-assistant
    task install  # Verify dependency resolution works
    task test     # Run tests if available
  3. Verify tman install resolves to an appropriate version of ten_ai_base
  4. Check that manifest-lock.json files (if they exist and are committed) reflect expected versions

📋 Additional Questions

  1. Changelog/Release Notes: Will this change be documented in release notes? Applications upgrading may need to know about the versioning strategy change.

  2. Migration Path: For applications with existing manifest-lock.json files, will they need to re-run tman install to pick up the new constraint?

  3. Breaking Changes: The PR title says "fix:" but this seems more like "chore:" or "refactor:" - is this fixing a specific bug? If so, what issue does it address?

🎯 Verdict

Conditional Approval ✅ with recommendations:

The change itself is sound and follows best practices for dependency management. However, before merging:

  1. Required: Validate that the version constraint "0.7" is interpreted as intended by tman
  2. Required: Run the existing test suite to ensure no breakage
  3. Recommended: Add a brief PR description explaining the motivation and expected behavior
  4. Recommended: Consider whether ten_runtime_python versions should also be relaxed

This is good dependency hygiene that will reduce maintenance burden. Just needs validation before merge.


Review generated with Claude Code

@plutoless plutoless merged commit 2ec2b2c into main Dec 9, 2025
10 of 32 checks passed
@plutoless plutoless deleted the chore/version_lock branch December 9, 2025 02:46
cyfyifanchen added a commit that referenced this pull request Dec 10, 2025
* fix: remove the version lock on ten_ai_base (#1818)

* fix: remove the version lock on ten_ai_base

* fix: revert

---------

Co-authored-by: Ethan Zhang <[email protected]>

* feat: add a python system package `pytest_ten` supporting AAA-style tests (#1838)

* feat: add a python test wrapper supporting AAA-style tests

* fix: use relative import

* feat: move test_wrapper from ten_runtime into core_system package

* test: add example tests using pytest_ten

* chore: refine code

* feat: update workflow to upload pytest_ten system package

* fix: refine code

* fix: test case on windows

---------

Co-authored-by: xxxxl_sun <[email protected]>
Co-authored-by: sunxilin <[email protected]>

* fix: elevenlabs asr language (#1842)

Co-authored-by: liaochenliang <[email protected]>

* feat: support tman install app (#1845)

Co-authored-by: Hu Yueh-Wei <[email protected]>

* refactor(property): rename max_tokens to max_completion_tokens for clarity (#1848)

Update property.json files to use more descriptive parameter name max_completion_tokens instead of max_tokens. Also includes minor text and voice setting adjustments.

* feat: add gradium tts (#1843)

* feat: add gradium tts

* style(gradium_tts_python): improve code formatting and line wrapping

* refactor: simplify error logging and config handling

* fix: update licence when uploading to ppa (#1851)

* feat: winget (#1844)

* feat: winget (dry run when push)

* fix: remove potential code injection

* fix: variable format

* fix: build command args

* chore: test production run, trigger PR to  winget-pkgs

* fix: higher rate limit

* Revert "chore: test production run, trigger PR to  winget-pkgs"

This reverts commit 4d5d6e0.

* chore: modify PackageIdentifier

* chore: refine codes

* fix: trigger

* chore: refine codes

* chore: refine codes

* chore: refine template

* fix: license type

* fix: refine codes

---------

Co-authored-by: Nie Zhihe <[email protected]>
Co-authored-by: Hu Yueh-Wei <[email protected]>

* fix: update minimax api endpoint from .com to .io (#1852)

* refactor: simplify build command by removing redundant clean task (#1849)

Co-authored-by: Ethan Zhang <[email protected]>

* fix(voice-assistant-live2d): update voice settings and api endpoint

* fix: update minimax websocket url from minimaxi.io to minimax.io (#1854)

---------

Co-authored-by: xxxxl_sun <[email protected]>
Co-authored-by: Ethan Zhang <[email protected]>
Co-authored-by: Jun <[email protected]>
Co-authored-by: sunxilin <[email protected]>
Co-authored-by: liaochenliang <[email protected]>
Co-authored-by: liaochenliang <[email protected]>
Co-authored-by: Hu Yueh-Wei <[email protected]>
Co-authored-by: Nie Zhihe <[email protected]>
Co-authored-by: Nie Zhihe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants