feat(taosnet): license-eligibility classifier - #1723
Conversation
First self-contained slice of the taOSnet Phase-2 client work. Decides whether a model's weights may be redistributed over the swarm, so the catalog-publish CLI can set each variant's license_allows_redistribution flag (which the download client already requires before touching the swarm). Conservative by default: restrictive markers (non-commercial, research, gated, S-Lab) force False; an explicit allow-list covers permissive + RAIL + Gemma + Llama-community licences; everything else defaults False for human review. Covers every licence string currently in app-catalog/models.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThis PR adds a new ChangesLicense Eligibility Feature
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant classify_manifest
participant license_allows_redistribution
participant normalize_license
Caller->>classify_manifest: classify_manifest(manifest)
classify_manifest->>license_allows_redistribution: license_allows_redistribution(license)
license_allows_redistribution->>normalize_license: normalize_license(raw)
normalize_license-->>license_allows_redistribution: normalized string
license_allows_redistribution-->>classify_manifest: True/False
classify_manifest-->>Caller: bool result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| collapse internal whitespace, so catalog licence strings compare stably.""" | ||
| text = raw.strip().strip('"').strip("'").lower() | ||
| # Drop a single trailing parenthetical note, e.g. "openrail++ (commercial use allowed)". | ||
| if text.endswith(")") and "(" in text: |
There was a problem hiding this comment.
WARNING: normalize_license only strips a single trailing parenthetical, so a licence with multiple notes such as "OpenRAIL++ (commercial use allowed) (v2)" would be left as "openrail++ (commercial use allowed)" after normalisation and would silently fall through to the conservative-False branch instead of matching the allow-list entry. Consider looping or using a regex (e.g. re.sub(r"\s*\([^)]*\)\s*$", "", text) applied repeatedly) to strip any number of trailing parentheticals.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "by-nc", # CC BY-NC / CC-BY-NC-SA | ||
| "-nc-", # sai-nc-community, stable-cascade-nc-community | ||
| "nc-community", | ||
| "research", # Qwen Research License |
There was a problem hiding this comment.
WARNING: The "research" and "gated" restrictive markers are bare substrings, so any future permissive licence whose name contains the word "research" (e.g. an "Apache Research Use License" or "Open Research Licence") or "gated" will be silently demoted to non-redistributable. This is an unbounded forward-compatibility risk given the conservative-default policy. Consider anchoring these (e.g. "research-only", " research license") or switching to a token- or regex-based check so the substring cannot appear inside an otherwise-permissive identifier.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| assert manifests, "no model manifests found; wrong path?" | ||
| for path in manifests: | ||
| data = yaml.safe_load(path.read_text()) | ||
| result = classify_manifest(data) |
There was a problem hiding this comment.
WARNING: The integration test test_every_catalog_manifest_classifies_without_error asserts isinstance(result, bool), but classify_manifest is statically typed to return bool, so this assertion is tautological and cannot fail regardless of the classification. A regression where a restricted licence (or every licence) starts returning True would still pass this test, defeating the PR's stated goal of "a new/unknown licence string surfaces for review rather than being silently redistributed". Strengthen it by, for example, collecting every manifest whose normalised licence is not in PERMISSIVE_LICENSES and asserting that set is a known/expected review queue (or at least printing the list so CI surfaces unknown licences explicitly).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return normalized in PERMISSIVE_LICENSES | ||
|
|
||
|
|
||
| def classify_manifest(manifest: dict) -> bool: |
There was a problem hiding this comment.
SUGGESTION: classify_manifest accepts manifest: dict with no value-type annotation and will raise AttributeError if a caller passes None, a list, or any non-mapping (e.g. a YAML scalar). Tighten the signature to Mapping[str, Any] | dict[str, Any] and either guard with isinstance(manifest, dict) or document the precondition; this matters because the catalog-publish CLI will be loading arbitrary manifest files where a malformed entry should produce a clear error, not a stack trace.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| def test_every_catalog_manifest_classifies_without_error(): | ||
| """Every model manifest must classify to a bool. If this fails on a NEW | ||
| licence string, add it to the allow-list or confirm it should stay False.""" | ||
| catalog = Path(__file__).resolve().parents[2] / "app-catalog" / "models" |
There was a problem hiding this comment.
SUGGESTION: The catalog discovery path uses Path(__file__).resolve().parents[2] / "app-catalog" / "models", which silently breaks if this test file is moved, the tests/taosnet directory is restructured, or the repo is vendored. Compute the path once, skip the integration test with pytest.skip when the directory is absent, and consider exposing the catalog root via pyproject.toml / a conftest fixture so the assumption is explicit and documented in one place.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 30.1K · Output: 5.9K · Cached: 102.6K |
![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 (3)
tests/taosnet/test_license_eligibility.py (1)
16-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for an annotated permissive license.
None of the current cases test a permissive license with a restrictive-marker annotation (e.g.
"MIT (non-commercial use)"). This is exactly the scenario that hides the marker-check ordering bug flagged inlicense_eligibility.py(the parenthetical is stripped before the marker scan runs), so it's worth a dedicatedNOT_REDISTRIBUTABLEcase here once that's fixed.🤖 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 `@tests/taosnet/test_license_eligibility.py` around lines 16 - 53, Add a regression test in test_license_eligibility for an annotated permissive license such as “MIT (non-commercial use)” or similar, and assert it belongs in NOT_REDISTRIBUTABLE. Update the license eligibility coverage around REDISTRIBUTABLE/NOT_REDISTRIBUTABLE so this case exercises the marker-check ordering in the eligibility logic and fails if the parenthetical annotation is stripped before marker detection.tinyagentos/taosnet/license_eligibility.py (2)
88-90: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against non-dict manifest input.
classify_manifestassumesmanifestis a dict. If a manifest file is empty,yaml.safe_loadreturnsNone, andmanifest.get(...)raisesAttributeErrorinstead of the conservativeFalsedefault this module otherwise guarantees everywhere else.🛡️ Proposed fix
def classify_manifest(manifest: dict) -> bool: """Convenience wrapper: read ``manifest['license']`` and classify it.""" - return license_allows_redistribution(manifest.get("license")) + return license_allows_redistribution((manifest or {}).get("license"))🤖 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 `@tinyagentos/taosnet/license_eligibility.py` around lines 88 - 90, `classify_manifest` currently assumes `manifest` is always a dict, so `manifest.get("license")` can crash when `yaml.safe_load` returns `None` for an empty file. Update `classify_manifest` in `license_eligibility.py` to safely handle non-dict input by checking the type (or using a guarded fallback) before accessing `.get`, and return `False` when the manifest is missing or invalid, matching the conservative behavior of `license_allows_redistribution`.
27-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM overall, with minor markers redundancy.
"nc-community"(Line 32) is already subsumed by"-nc-"(Line 31) for all current catalog entries — harmless duplication, not worth changing now.🤖 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 `@tinyagentos/taosnet/license_eligibility.py` around lines 27 - 38, The restrictive license marker list in _RESTRICTIVE_MARKERS has redundant overlap because "nc-community" is already covered by the existing "-nc-" substring check used in license_eligibility.py. Remove the duplicate marker from the tuple and keep the remaining entries in _RESTRICTIVE_MARKERS unchanged so the intent stays clear and the list does not contain unnecessary repetition.
🤖 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 `@tinyagentos/taosnet/license_eligibility.py`:
- Around line 64-85: The restrictive-marker check in
license_allows_redistribution is happening after normalize_license, which can
remove trailing parenthetical annotations and let restricted licenses slip
through. Update license_allows_redistribution to scan the quote-stripped,
lower-cased raw text for _RESTRICTIVE_MARKERS before calling normalize_license,
then use normalize_license only for the PERMISSIVE_LICENSES allow-list lookup.
Keep the fix localized to normalize_license and license_allows_redistribution so
annotated licenses like “Apache-2.0 (non-commercial use only)” are rejected
correctly.
---
Nitpick comments:
In `@tests/taosnet/test_license_eligibility.py`:
- Around line 16-53: Add a regression test in test_license_eligibility for an
annotated permissive license such as “MIT (non-commercial use)” or similar, and
assert it belongs in NOT_REDISTRIBUTABLE. Update the license eligibility
coverage around REDISTRIBUTABLE/NOT_REDISTRIBUTABLE so this case exercises the
marker-check ordering in the eligibility logic and fails if the parenthetical
annotation is stripped before marker detection.
In `@tinyagentos/taosnet/license_eligibility.py`:
- Around line 88-90: `classify_manifest` currently assumes `manifest` is always
a dict, so `manifest.get("license")` can crash when `yaml.safe_load` returns
`None` for an empty file. Update `classify_manifest` in `license_eligibility.py`
to safely handle non-dict input by checking the type (or using a guarded
fallback) before accessing `.get`, and return `False` when the manifest is
missing or invalid, matching the conservative behavior of
`license_allows_redistribution`.
- Around line 27-38: The restrictive license marker list in _RESTRICTIVE_MARKERS
has redundant overlap because "nc-community" is already covered by the existing
"-nc-" substring check used in license_eligibility.py. Remove the duplicate
marker from the tuple and keep the remaining entries in _RESTRICTIVE_MARKERS
unchanged so the intent stays clear and the list does not contain unnecessary
repetition.
🪄 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: edb5c6dd-2330-4931-a7f0-5e996d6798f5
📒 Files selected for processing (3)
tests/taosnet/test_license_eligibility.pytinyagentos/taosnet/__init__.pytinyagentos/taosnet/license_eligibility.py
| def normalize_license(raw: str) -> str: | ||
| """Lower-case, strip surrounding quotes, drop a trailing ``(...)`` note, and | ||
| collapse internal whitespace, so catalog licence strings compare stably.""" | ||
| text = raw.strip().strip('"').strip("'").lower() | ||
| # Drop a single trailing parenthetical note, e.g. "openrail++ (commercial use allowed)". | ||
| if text.endswith(")") and "(" in text: | ||
| text = text[: text.rindex("(")].strip() | ||
| return " ".join(text.split()) | ||
|
|
||
|
|
||
| def license_allows_redistribution(raw: str | None) -> bool: | ||
| """True only when the licence is known to permit redistributing the weights. | ||
|
|
||
| Conservative: an unknown, empty, or restrictively-marked licence returns | ||
| False. See module docstring for the policy. | ||
| """ | ||
| if not raw or not raw.strip(): | ||
| return False | ||
| normalized = normalize_license(raw) | ||
| if any(marker in normalized for marker in _RESTRICTIVE_MARKERS): | ||
| return False | ||
| return normalized in PERMISSIVE_LICENSES |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Restrictive markers are checked too late
license_allows_redistribution() normalizes first, which strips trailing (...) annotations before the restrictive-marker scan. That lets annotated permissive licenses like Apache-2.0 (non-commercial use only) collapse to apache-2.0 and pass the allow-list. Check markers on the quote-stripped/lower-cased text first, then apply normalize_license() only for the allow-list lookup.
🤖 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 `@tinyagentos/taosnet/license_eligibility.py` around lines 64 - 85, The
restrictive-marker check in license_allows_redistribution is happening after
normalize_license, which can remove trailing parenthetical annotations and let
restricted licenses slip through. Update license_allows_redistribution to scan
the quote-stripped, lower-cased raw text for _RESTRICTIVE_MARKERS before calling
normalize_license, then use normalize_license only for the PERMISSIVE_LICENSES
allow-list lookup. Keep the fix localized to normalize_license and
license_allows_redistribution so annotated licenses like “Apache-2.0
(non-commercial use only)” are rejected correctly.
1 participant
First self-contained slice of the taOSnet (model torrent mesh) Phase-2 client work. No dependency on the taos.my server endpoints.
tinyagentos/taosnet/license_eligibility.py: decideslicense_allows_redistributionfor a model manifest. Conservative by default (restrictive markers force False; explicit allow-list for permissive + RAIL + Gemma + Llama-community; unknown -> False for human review). The catalog-publish CLI (next slice) uses this to set the manifest flag thatshould_use_torrentalready gates on.40 tests, incl. an integration test that classifies every current
app-catalog/modelsmanifest without error (so a new/unknown licence string surfaces for review rather than being silently redistributed).Part of the taOSnet initiative (design: docs/design/model-torrent-mesh.md; taos.my contract owned by @taOS-website-dev).
Summary by CodeRabbit
New Features
Tests