Orb P0 S2a: tinyagentos/agent_avatars.py (move avatar slug + dir out of routes/auth.py, add avatar_hash) - #3517
Conversation
RED-FIRST proof:
```text
==================================== ERRORS ====================================
_________________ ERROR collecting tests/test_agent_avatars.py _________________
ImportError while importing test module '/tmp/exec-tsk-aa5qck/tests/test_agent_avatars.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback (most recent call last):
File "/home/jay/.local/share/uv/python/cpython-3.14.7-linux-x86_64-gnu/lib/python3.14/importlib/__init__.py", line 88, in _gcd_import
module = _bootstrap._gcd_import(name, package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_agent_avatars.py:9: in <module>
from tinyagentos.agent_avatars import avatar_hash, avatar_source_path
E ModuleNotFoundError: No module named 'tinyagentos.agent_avatars'
!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
1 error in 0.32s
```
Green after fix:
```text
==================================== 4 passed in 0.24s ====================================
```
Demo mode and lock-widgets tests also green:
```text
==================================== 17 passed in 22.48s ====================================
...
........................................................................ [ 60%]
............................................... [100%]
119 passed in 43.90s
```
Docs-Reviewed: module extraction only, no route or user-facing behavior change, docs/agent-coordination.md not affected
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughAvatar directory and slug logic moved from the auth route into a dedicated module. The module adds helpers that return an installed avatar path and its truncated SHA-256 hash. Tests cover lookup, hashing, and slug-helper agreement. ChangesAvatar helper module
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is evident in the supplied change context. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The existing avatar access checks and directory configuration are preserved. The new helpers currently have only test callers, so this change does not introduce a new externally reachable file operation or materially broaden filesystem access. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (2 skipped: 1 unsupported, 1 too large.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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}
| import os | ||
| from pathlib import Path | ||
|
|
||
| LOCK_AVATAR_DIR = os.environ.get("TAOS_LOCK_AVATAR_DIR", "/var/lib/taos/lock-avatars") |
There was a problem hiding this comment.
WARNING: Missing docstring for LOCK_AVATAR_DIR
The #: Sphinx-style docstring that explained its purpose and overridability was dropped when the definitions were moved from auth.py. This constant is environment-overridable and its documentation should travel with it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| path = avatar_source_path(name) | ||
| if path is None: | ||
| return None | ||
| return hashlib.sha256(path.read_bytes()).hexdigest()[:16] |
There was a problem hiding this comment.
WARNING: Unhandled OSError in avatar_hash
path.read_bytes() can raise OSError (e.g., FileNotFoundError) if the file is deleted between avatar_source_path's is_file() check and the read. The original lock_avatar route in auth.py handled this with try/except OSError.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash:free · Input: 46.4K · Output: 15.1K · Cached: 389.8K |
|
Lead review at head against card tsk-aa5qck: APPROVE. Both definitions moved verbatim, auth.py re-imports them (one slug rule), avatar_source_path/avatar_hash match the card, red shown (ModuleNotFoundError) then green, lock-widgets + demo suites green. |
1 participant
CARD TITLE (intent, not commit subject): Orb P0 S2a: tinyagentos/agent_avatars.py (move avatar slug + dir out of routes/auth.py, add avatar_hash)
Autonomous build of board card tsk-aa5qck.
tsk-aa5qck: extract avatar slug + dir into agent_avatars module
RED-FIRST proof:
Green after fix:
Demo mode and lock-widgets tests also green:
Docs-Reviewed: module extraction only, no route or user-facing behavior change, docs/agent-coordination.md not affected
Files:
changelog.d/tsk-aa5qck-agent-avatars-module.md | 3 ++
tests/test_agent_avatars.py | 45 ++++++++++++++++++++++++++
tinyagentos/agent_avatars.py | 38 ++++++++++++++++++++++
tinyagentos/routes/auth.py | 25 +-------------
4 files changed, 87 insertions(+), 24 deletions(-)
In-house review (pre-PR)
in-house-review: PASS (openrouter/inclusionai/ling-3.1-flash)
reviewer output
VERDICT: PASS
from pathlib import Pathis unused (tmp_path already is a Path);import pytest(line 7) is also unused.#:doc-comment describing LOCK_AVATAR_DIR's purpose/overridability was dropped in the move instead of being carried into agent_avatars.py (the two definitions themselves were moved intact per the card).lock_widgetsand the@router.get("/lock-avatar/{slug}")decorator after the removal (cosmetic only).Verified against card: both definitions moved unchanged with docstring intact; import swapped in at top-level imports of auth.py so all existing uses (incl.
from tinyagentos.routes.auth import _avatar_slug) still resolve via re-export, leaving exactly one slug rule;avatar_source_pathreads the module globalLOCK_AVATAR_DIRat call time (monkeypatchable as tests do) and returns None when no file;avatar_hashreturns first 16 hex chars of sha256 or None; module deps are only os/hashlib/pathlib; no route added; all four required test cases present and capable of failing (RED ModuleNotFoundError on origin/dev, then green); changelog file has one bullet under### Changed; no em dashes or AI attribution in the diff.In-house-Review: PASS model=openrouter/inclusionai/ling-3.1-flash head=ad0f4522e needs-lead=0
Summary by CodeRabbit
Lead note: _avatar_slug moved verbatim to tinyagentos/agent_avatars.py and is re-imported into routes/auth.py (one slug rule, not a removal).
Removes-Intentionally: tinyagentos/routes/auth.py:_avatar_slug