Add Limitless Pendant skill for querying lifelogs#40
Conversation
Python CLI tool using uv inline script dependencies for zero-setup execution. Features: - Query recent, today, or date-specific conversations - Semantic search across all lifelogs - Smart .env file loading from multiple locations - Beautiful rich terminal output - Helpful setup instructions when API key missing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Points LLMs to Context7 and developer portal for API lookups Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review: Limitless Pendant SkillSummaryThis PR adds a well-crafted skill for querying Limitless Pendant lifelogs. The implementation is clean, follows modern Python practices, and includes excellent UX with helpful setup instructions. Overall, this is high-quality code with just a few minor suggestions for improvement. ✅ StrengthsCode Quality
Documentation
Security & Best Practices
🔧 Issues & Suggestions1. Potential Bug: Integer Parsing Without ValidationLocation: case "recent":
limit = int(args[1]) if len(args) > 1 else 5
cmd_recent(limit)Issue: If user passes non-integer value like Suggestion: Add validation: case "recent":
limit = 5
if len(args) > 1:
try:
limit = int(args[1])
except ValueError:
error(f"Invalid limit: {args[1]}. Must be an integer.")
sys.exit(1)
cmd_recent(limit)2. Security: API Key Logging RiskLocation: error(f"API error: {e.response.status_code} - {e.response.text}")Issue: If the API returns an error that includes the API key in the response body, this would print it to stderr. This is unlikely but possible. Suggestion: Consider truncating or sanitizing error responses: error_text = e.response.text[:200] # Truncate to prevent key leakage
error(f"API error: {e.response.status_code} - {error_text}")3. Code Consistency: Mixed Error Handling PatternsLocations: Multiple functions Issue: Some functions check Current pattern: data = call_api(...)
if data:
format_lifelogs(data)Suggestion: This is actually fine since 4. Minor: Hardcoded Timezone DefaultLocation: TIMEZONE = os.environ.get("LIMITLESS_TIMEZONE", "America/Chicago")Issue: Hardcoded Suggestion: Consider using system timezone as default: import zoneinfo
TIMEZONE = os.environ.get("LIMITLESS_TIMEZONE") or datetime.now().astimezone().tzinfo.key or "America/Chicago"Or at minimum, document this clearly in the SKILL.md prerequisites (which you already do - so this is fine as-is). 5. Enhancement: Raw Params Parsing Could Be More RobustLocation: def cmd_raw(params: str) -> None:
param_dict = {}
if params:
for pair in params.split("&"):
if "=" in pair:
key, value = pair.split("=", 1)
param_dict[key] = valueIssue: This doesn't handle URL-encoded values or validate parameter names. Suggestion: Consider using from urllib.parse import parse_qs
def cmd_raw(params: str) -> None:
param_dict = {}
if params:
parsed = parse_qs(params, keep_blank_values=True)
param_dict = {k: v[0] if len(v) == 1 else v for k, v in parsed.items()}
data = call_api("/lifelogs", param_dict)
if data:
console.print_json(data=data)6. Documentation: Missing Color FieldLocation: Issue: According to CLAUDE.md conventions, skills should have a Suggestion: Add color to frontmatter (check existing research skills for the standard): ---
name: limitless
color: magenta # or whatever color research skills use
# prettier-ignore
description: "Use when recalling conversations..."🧪 Test CoveragePR mentions:
Recommendation: Since this requires hardware and API access, consider adding a mock test or at minimum document manual testing steps for future contributors. 📊 Performance Considerations
🎯 RecommendationsMust Fix (before merge):
Should Fix (nice to have):
Optional (future enhancement):
🎉 Overall AssessmentVerdict: ✅ Approve with minor fixes This is excellent work. The code is clean, well-documented, and follows the project's conventions. The user experience is thoughtful with helpful setup instructions. The only blocking issue is the integer parsing bug which could cause crashes. Once that's fixed, this is ready to merge. The skill fills a useful niche for Limitless Pendant users and demonstrates good patterns for future skill development (uv inline scripts, multi-location .env loading, rich terminal output). Great job! 🚀 |
Code Review - Limitless Pendant SkillOverall this is a well-implemented skill with clean Python code and good documentation. The uv inline script approach is modern and eliminates dependency setup friction. Here are my findings: ✅ StrengthsCode Quality:
Documentation:
Security:
User Experience:
🟡 Suggestions1. Security: API Key Exposure in JSON Parsing In try:
config = json.loads(env_path.read_text())
env_vars = config.get("env", {})
if "LIMITLESS_API_KEY" in env_vars:
return env_vars["LIMITLESS_API_KEY"]
except (json.JSONDecodeError, KeyError):
passConsider logging when JSON parsing fails so users know their config file is malformed. 2. Type Safety: Union Syntax Good use of 3. Error Handling: HTTP Response Details In else:
error(f"API error: {e.response.status_code} - {e.response.text}")For 429 (rate limit), could give more helpful guidance about the 180/min limit mentioned in docs. 4. Potential Bug: Date Validation
def cmd_date(date_str: str) -> None:
"""Get conversations for a specific date."""
try:
datetime.strptime(date_str, "%Y-%m-%d")
except ValueError:
error("Date must be in YYYY-MM-DD format")
return
# ... rest of function5. Test Coverage The PR checklist shows "Test with actual LIMITLESS_API_KEY" is unchecked. While the script executes with
6. Documentation: Context7 Reference The 🟢 Minor Polish1. Timezone Handling Default timezone is 2. Script Permissions The file has executable permissions (good!), but consider documenting in SKILL.md that users may need to 3. Help Text Formatting The help text at line 204 could include a note about the setup instructions being displayed automatically when the API key is missing. 📊 Metrics
🎯 VerdictRecommend: Approve with minor suggestions This is production-ready code with excellent UX. The suggestions above are optimizations, not blockers. The skill will work well as-is and follows all project conventions from CLAUDE.md. The only item I'd strongly recommend before merge is completing the test plan item "Test with actual LIMITLESS_API_KEY" to ensure the API integration works end-to-end. Review generated by Claude Code following CLAUDE.md conventions |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df5d2c412b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try: | ||
| config = json.loads(env_path.read_text()) | ||
| env_vars = config.get("env", {}) | ||
| if "LIMITLESS_API_KEY" in env_vars: | ||
| return env_vars["LIMITLESS_API_KEY"] |
There was a problem hiding this comment.
Because load_api_key returns immediately when clawdbot.json contains LIMITLESS_API_KEY, later sources (e.g., ./.env, ./.env.local, or an exported LIMITLESS_API_KEY) are never considered. This means a stale key in ~/.clawdbot/clawdbot.json will override a newer key set in the shell, leading to persistent 401s even after a user updates their environment. Consider loading the JSON into the environment and continuing the loop, or deferring the decision until all sources are processed so higher-precedence sources can override.
Useful? React with 👍 / 👎.
Fixes from bot feedback: - Fix Codex P2: Env loading now properly allows later sources to override earlier - Add date format validation with helpful error message - Improve 429 rate limit error message - Add warning when JSON config parsing fails Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| except (json.JSONDecodeError, KeyError) as e: | ||
| console.print(f"[yellow]Warning: Failed to parse {env_path}: {e}[/yellow]", file=sys.stderr) | ||
| else: | ||
| load_dotenv(env_path, override=True) |
There was a problem hiding this comment.
Environment variable priority inverted from documented behavior
Medium Severity
The docstring states "Environment variable (highest priority)" but using load_dotenv(env_path, override=True) causes .env files to overwrite any pre-existing environment variable. This inverts the documented priority - the environment variable actually has the lowest priority, and .env.local has the highest. Users following the documentation who set LIMITLESS_API_KEY in their shell will have it unexpectedly overwritten by values in .env files.
|
|
||
| match cmd: | ||
| case "recent": | ||
| limit = int(args[1]) if len(args) > 1 else 5 |
There was a problem hiding this comment.
Unhandled ValueError when parsing limit argument
Low Severity
The recent command parses its argument with int(args[1]) without validation. Running limitless recent abc causes an unhandled ValueError and Python traceback instead of a user-friendly error message. Other commands like date validate their input and call sys.exit(1) with a helpful message, but recent lacks this handling.
|
|
||
| def cmd_today() -> None: | ||
| """Get today's conversations.""" | ||
| today = datetime.now().strftime("%Y-%m-%d") |
There was a problem hiding this comment.
Today command uses machine timezone instead of configured timezone
Medium Severity
The cmd_today() function uses datetime.now() which returns the date in the machine's local timezone, but then passes LIMITLESS_TIMEZONE to the API. If the script runs on a server in UTC while LIMITLESS_TIMEZONE is set to a different timezone like America/Chicago, the computed date can be off by a day during timezone boundary hours. For example, at 11pm Chicago time (5am UTC), the UTC server calculates "tomorrow's" date, causing the wrong day's conversations to be retrieved.
| env_vars = config.get("env", {}) | ||
| if "LIMITLESS_API_KEY" in env_vars: | ||
| os.environ["LIMITLESS_API_KEY"] = env_vars["LIMITLESS_API_KEY"] | ||
| except (json.JSONDecodeError, KeyError) as e: |
There was a problem hiding this comment.
JSON config with null env crashes with TypeError
Low Severity
When parsing clawdbot.json, if the file contains {"env": null}, config.get("env", {}) returns None (since the key exists) rather than the default empty dict. The subsequent "LIMITLESS_API_KEY" in env_vars check raises an unhandled TypeError. The except clause only catches JSONDecodeError and KeyError, so the script crashes instead of gracefully warning and continuing.
| return None | ||
| except httpx.RequestError as e: | ||
| error(f"Request failed: {e}") | ||
| return None |
There was a problem hiding this comment.
Script exits with success code despite API errors
Medium Severity
When call_api encounters HTTP errors (401, 429, 5xx) or network failures, it prints an error message but returns None instead of exiting. Command functions check if data: and simply skip processing, causing the script to exit with code 0 (success). This contradicts the behavior for missing API key which correctly calls sys.exit(1). Shell scripts using set -e, &&, or checking $? will incorrectly believe the command succeeded despite the error being reported to stderr.
Additional Locations (1)
1 participant
Summary
Adds a new skill for querying Limitless Pendant lifelogs - conversations, meetings, and ambient recordings captured by the wearable AI device.
Features:
Test plan
helpcommand🤖 Generated with Claude Code