Deleted starred pages #101
Conversation
1. Environment variable filtering in datastore.py:192-208: Modified save_feed_and_episodes() to filter out episodes where userUpdatedDate < (now - OVERCAST_LIMIT_DAYS) during insertion. 2. Cleanup method in datastore.py:461-478: Added cleanup_old_episodes() method that deletes old episodes from the database, but only if there are more than 100 episode rows. 3. CLI integration in cli.py:119: Added call to db.cleanup_old_episodes() at the end of the save command. How it works: - Set OVERCAST_LIMIT_DAYS=30 (or any number) to only keep episodes from the last 30 days - During save, episodes older than the limit won't be inserted - After save completes, if there are >100 episodes total, any remaining old episodes are deleted - If no environment variable is set, the behavior remains unchanged
Reviewer's GuideThis PR introduces an environment-variable–driven episode retention mechanism and cleanup routine, refactors Datastore by extracting URL normalization and query-building helpers, adds dedicated starred and deleted episode retrieval methods, and generalizes HTML page generation and the CLI to emit separate played, starred, and deleted pages with navigation links. Sequence diagram for CLI HTML command and page generationsequenceDiagram
actor User
participant CLI
participant Datastore
participant HTMLPageGenerator
User->>CLI: Run html command
CLI->>HTMLPageGenerator: generate_html_played(db_path, played_path)
HTMLPageGenerator->>Datastore: get_recently_played()
CLI->>HTMLPageGenerator: generate_html_starred(db_path, starred_path)
HTMLPageGenerator->>Datastore: get_starred_episodes()
CLI->>HTMLPageGenerator: generate_html_deleted(db_path, deleted_path)
HTMLPageGenerator->>Datastore: get_deleted_episodes()
CLI->>User: Print HTML file locations
Class diagram for updated Datastore methodsclassDiagram
class Datastore {
+save_feed_and_episodes(feed: dict, episodes: list[dict])
+get_recently_played() list[dict[str, str]]
+get_starred_episodes() list[dict[str, str]]
+get_deleted_episodes() list[dict[str, str]]
+cleanup_old_episodes() None
-_clean_enclosure_urls() None
-_clean_enclosure_urls_simple() None
-_get_base_fields() list[str]
-_build_episode_query(fields: list[str], where_clause: str, order_by: str) str
-_process_query_results(results: list[tuple], fields: list[str]) list[dict[str, str]]
}
Datastore <|-- HTMLPageGenerator
class HTMLPageGenerator {
+_generate_html_episodes(episodes: list[dict[str, str]], title: str, html_output_path: Path, date_field: str = "userUpdatedDate") None
+generate_html_played(db_path: str, html_output_path: Path) None
+generate_html_starred(db_path: str, html_output_path: Path) None
+generate_html_deleted(db_path: str, html_output_path: Path) None
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe changes introduce support for generating three separate HTML pages for recently played, starred, and deleted podcast episodes, with corresponding navigation links and refactored HTML generation logic. The datastore logic is modularized and enhanced to support episode filtering and cleanup based on environment variables. Additional configuration, coding guidelines, and development dependency updates are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Datastore
participant HTMLGen
User->>CLI: run 'html' command
CLI->>Datastore: get_recently_played()
Datastore-->>CLI: recently played episodes
CLI->>HTMLGen: generate_html_played()
HTMLGen-->>CLI: overcast-played.html
CLI->>Datastore: get_starred_episodes()
Datastore-->>CLI: starred episodes
CLI->>HTMLGen: generate_html_starred()
HTMLGen-->>CLI: overcast-starred.html
CLI->>Datastore: get_deleted_episodes()
Datastore-->>CLI: deleted episodes
CLI->>HTMLGen: generate_html_deleted()
HTMLGen-->>CLI: overcast-deleted.html
CLI->>User: Print file URLs for all three HTML files
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @hbmartin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly expands the overcast-to-sqlite tool's capabilities by introducing new HTML reports for starred and deleted podcast episodes, alongside enhanced data management features for episode retention and cleanup. The changes also include substantial refactoring to improve the codebase's structure and maintainability, as well as updates to development environment configurations.
Highlights
- Enhanced HTML Reporting: The tool now generates dedicated HTML pages for 'Starred Episodes' and 'Deleted Episodes', in addition to the existing 'Recently Played' page. A new navigation bar has been added to
index.htmlto easily switch between these views. - Episode Data Management: A new feature has been introduced to limit the retention of episodes in the database. Users can now set an
OVERCAST_LIMIT_DAYSenvironment variable, and the system will automatically clean up episodes older than this limit during data synchronization, provided the total episode count exceeds a default threshold (100 episodes). - Code Refactoring and Reusability: Significant refactoring has been applied to the
Datastoreand HTML generation logic. Common patterns for database queries and HTML rendering have been extracted into new private helper methods, improving code reusability, readability, and maintainability. - Development Environment Setup: New files have been added to support development, including a
.claude/settings.local.jsonfor AI assistant permissions and aCLAUDE.mddocument outlining Python coding best practices and style guidelines for contributors.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @hbmartin - I've reviewed your changes - here's some feedback:
Blocking issues:
- Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)
- Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)
General comments:
- In save_feed_and_episodes (and cleanup_old_episodes),
if env_limit := os.getenv(...) is not Noneassigns a boolean instead of the actual environment string; assignraw = os.getenv(...)and then checkraw is not Nonebefore converting it to int. - generate_html_deleted calls
_generate_html_episodeswith ashow_starred_iconargument, but the helper signature doesn’t accept that parameter; update the signature or remove the extra parameter. - There’s duplicated enclosure URL cleaning logic between
_clean_enclosure_urlsand_clean_enclosure_urls_simple; consider merging them into one method with a dedupe flag to reduce code repetition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In save_feed_and_episodes (and cleanup_old_episodes), `if env_limit := os.getenv(...) is not None` assigns a boolean instead of the actual environment string; assign `raw = os.getenv(...)` and then check `raw is not None` before converting it to int.
- generate_html_deleted calls `_generate_html_episodes` with a `show_starred_icon` argument, but the helper signature doesn’t accept that parameter; update the signature or remove the extra parameter.
- There’s duplicated enclosure URL cleaning logic between `_clean_enclosure_urls` and `_clean_enclosure_urls_simple`; consider merging them into one method with a dedupe flag to reduce code repetition.
## Individual Comments
### Comment 1
<location> `overcast_to_sqlite/datastore.py:201` </location>
<code_context>
) -> None:
"""Upsert feed and episodes into database."""
+ limit_days = None
+ try:
+ if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None:
+ limit_days = int(env_limit)
+ except ValueError:
+ pass
+
</code_context>
<issue_to_address>
The logic for reading OVERCAST_LIMIT_DAYS from the environment is incorrect.
The current line assigns a boolean to env_limit instead of the environment variable's value. Assign the result of os.getenv to env_limit first, then check if it is not None before converting to int.
</issue_to_address>
### Comment 2
<location> `overcast_to_sqlite/datastore.py:207` </location>
<code_context>
+ except ValueError:
+ pass
+
+ if limit_days:
+ cutoff_date = datetime.datetime.now(tz=datetime.UTC) - datetime.timedelta(
+ days=int(limit_days),
</code_context>
<issue_to_address>
The check 'if limit_days:' will skip filtering if limit_days is 0.
If the environment variable is 0, no filtering occurs. Use 'if limit_days is not None:' to handle zero correctly.
</issue_to_address>
### Comment 3
<location> `overcast_to_sqlite/html/page.py:61` </location>
<code_context>
+ for ep in episodes:
</code_context>
<issue_to_address>
The starred icon logic does not handle show_starred_icon parameter.
Please ensure show_starred_icon is checked before displaying the icon, so the parameter is used as intended.
</issue_to_address>
### Comment 4
<location> `CLAUDE.md:4` </location>
<code_context>
+## Python Practices
+- Always use or add type hints
+- Prefer @dataclasses where applicable
+- Always use f-string over string formatting or concatentation (except in logging strings)
+- Use async generators and comprehensions when they might provide benefits
+- Use underscores in large numeric literals
</code_context>
<issue_to_address>
Typo: 'concatentation' should be 'concatenation'.
Please correct the spelling to 'concatenation'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
- Always use f-string over string formatting or concatentation (except in logging strings)
=======
- Always use f-string over string formatting or concatenation (except in logging strings)
>>>>>>> REPLACE
</suggested_fix>
## Security Issues
### Issue 1
<location> `overcast_to_sqlite/datastore.py:578` </location>
<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `overcast_to_sqlite/datastore.py:589` </location>
<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| try: | ||
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | ||
| limit_days = int(env_limit) | ||
| except ValueError: |
There was a problem hiding this comment.
issue (bug_risk): The logic for reading OVERCAST_LIMIT_DAYS from the environment is incorrect.
The current line assigns a boolean to env_limit instead of the environment variable's value. Assign the result of os.getenv to env_limit first, then check if it is not None before converting to int.
| except ValueError: | ||
| pass | ||
|
|
||
| if limit_days: |
There was a problem hiding this comment.
suggestion (bug_risk): The check 'if limit_days:' will skip filtering if limit_days is 0.
If the environment variable is 0, no filtering occurs. Use 'if limit_days is not None:' to handle zero correctly.
| for ep in episodes: | ||
| ep["episode_title"] = html.escape(ep["episode_title"]) | ||
| ep[DESCRIPTION] = _fix_unclosed_html_tags( | ||
| _convert_urls_to_links(ep[DESCRIPTION]), | ||
| ) | ||
| user_date = ep["userUpdatedDate"].split("T")[0] | ||
| if last_user_updated_date != user_date: | ||
| user_date = ep[date_field].split("T")[0] if ep.get(date_field) else "" | ||
| if last_user_updated_date != user_date and user_date: | ||
| page_vars["episodes"] += ( | ||
| "<h1><script>document.write(" | ||
| f'new Date("{ep["userUpdatedDate"]}").toLocaleDateString()' | ||
| f'new Date("{ep[date_field]}").toLocaleDateString()' |
There was a problem hiding this comment.
issue (bug_risk): The starred icon logic does not handle show_starred_icon parameter.
Please ensure show_starred_icon is checked before displaying the icon, so the parameter is used as intended.
| ## Python Practices | ||
| - Always use or add type hints | ||
| - Prefer @dataclasses where applicable | ||
| - Always use f-string over string formatting or concatentation (except in logging strings) |
There was a problem hiding this comment.
issue (typo): Typo: 'concatentation' should be 'concatenation'.
Please correct the spelling to 'concatenation'.
| - Always use f-string over string formatting or concatentation (except in logging strings) | |
| - Always use f-string over string formatting or concatenation (except in logging strings) |
| if not limit_days: | ||
| return | ||
|
|
||
| episode_count = self.db.execute(f"SELECT COUNT(*) FROM {EPISODES}").fetchone()[ |
There was a problem hiding this comment.
security (python.sqlalchemy.security.sqlalchemy-execute-raw-query): Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
Source: opengrep
| self.db.execute( | ||
| f"DELETE FROM {EPISODES} WHERE {USER_UPDATED_DATE} < ?", | ||
| [cutoff_iso], | ||
| ) |
There was a problem hiding this comment.
security (python.sqlalchemy.security.sqlalchemy-execute-raw-query): Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
Source: opengrep
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 31b6db3 in 2 minutes and 52 seconds. Click for details.
- Reviewed
1364lines of code in8files - Skipped
1files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .claude/settings.local.json:8
- Draft comment:
Consider adding a newline at the end of the file for POSIX compliance. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While POSIX compliance for newlines is a real thing, this seems like a minor stylistic issue that would typically be handled by editor settings or linting rules. The file is valid JSON either way. The comment doesn't indicate any actual problems this would cause. The lack of a trailing newline could potentially cause issues with some Unix tools or git diffs. Some development environments enforce this as a standard. While true, this is a minor stylistic issue that would be better handled by automated tooling or editor settings rather than manual code review comments. It doesn't affect functionality. Delete this comment as it's too minor of an issue to warrant a code review comment. This kind of formatting should be handled by automated tools.
2. overcast_to_sqlite/datastore.py:570
- Draft comment:
Repeat of the walrus operator issue in cleanup_old_episodes. Wrap the assignment in parentheses to correctly capture the environment variable. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The current codeif env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None:is actually correct Python syntax. The walrus operator has lower precedence than theisoperator, so this correctly assigns the value and then checks if it's None. Adding parentheses would change the meaning - it would first check if the getenv result is None, then assign that boolean to env_limit. Could I be wrong about operator precedence in Python? Should I double-check the Python documentation about walrus operator precedence? I am confident about this. The Python documentation and PEP 572 explicitly state that the walrus operator has lower precedence than comparison operators likeis. The current code is correct and adding parentheses would change its meaning in an undesired way. The comment should be deleted because it suggests an incorrect change. The current code's operator precedence is correct and adding parentheses would actually break the intended functionality.
3. overcast_to_sqlite/datastore.py:497
- Draft comment:
Consider using explicit column aliases instead of parsing field names via split() in _process_query_results for more robust result mapping. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_TmzSwGihKkUqB5Ti
You can customize 
| episodes, | ||
| "Deleted Episodes", | ||
| html_output_path, | ||
| show_starred_icon=False, |
There was a problem hiding this comment.
The function call to _generate_html_episodes passes an unsupported keyword argument 'show_starred_icon'. Remove or update this parameter.
| show_starred_icon=False, |
| """Upsert feed and episodes into database.""" | ||
| limit_days = None | ||
| try: | ||
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: |
There was a problem hiding this comment.
The walrus operator usage is incorrect. Use parentheses: if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None:
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | |
| if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None: |
| ## Python Practices | ||
| - Always use or add type hints | ||
| - Prefer @dataclasses where applicable | ||
| - Always use f-string over string formatting or concatentation (except in logging strings) |
There was a problem hiding this comment.
Typo found: In "Always use f-string over string formatting or concatentation (except in logging strings)", "concatentation" should be spelled "concatenation".
| - Always use f-string over string formatting or concatentation (except in logging strings) | |
| - Always use f-string over string formatting or concatenation (except in logging strings) |
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable enhancements, including new HTML pages for starred and deleted episodes, episode age limiting, and significant refactoring of the HTML generation and database query logic.
The refactoring improves the code's structure and maintainability. However, I've identified a few critical issues that need to be addressed:
- A recurring bug in
datastore.pywith the use of the walrus operator will prevent the new episode age limiting feature from working as intended. - The refactored
_generate_html_episodesfunction inpage.pyhas several issues that will lead to runtime errors (TypeErrorandKeyError) and incorrect behavior.
I've also included a couple of medium-severity suggestions to align the code with the project's Python style guide regarding the use of named arguments. Once these critical issues are resolved, the PR will be in great shape.
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | ||
| limit_days = int(env_limit) |
There was a problem hiding this comment.
There's a bug in how the environment variable is being read. The expression os.getenv(...) is not None evaluates to a boolean (True or False), which is then assigned to env_limit. int(env_limit) will then result in 1 or 0, not the actual day limit from the environment variable.
To fix this, the walrus assignment needs to be enclosed in parentheses.
Note that this same logic is duplicated in cleanup_old_episodes (lines 570-571) and should also be fixed there. To improve maintainability, consider extracting this logic into a private helper method.
if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None:
limit_days = int(env_limit)| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | ||
| limit_days = int(env_limit) |
There was a problem hiding this comment.
This is the same bug as in save_feed_and_episodes. The walrus operator assignment is incorrect, causing limit_days to be 1 instead of the value from the environment variable.
Please apply the same fix here by wrapping the assignment in parentheses. As mentioned in the other comment, extracting this duplicated logic into a shared private method would be a good refactoring to avoid this kind of issue.
if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None:
limit_days = int(env_limit)| def _generate_html_episodes( | ||
| episodes: list[dict[str, str]], | ||
| title: str, | ||
| html_output_path: Path, | ||
| date_field: str = "userUpdatedDate", | ||
| ) -> None: | ||
| """Generate HTML for any list of episodes.""" | ||
| this_dir = Path(__file__).parent | ||
| page_vars = { | ||
| "title": "Recently Played", | ||
| "title": title, | ||
| "style": Path(this_dir / "mvp.css").read_text(), | ||
| "script": Path(this_dir / "search.js").read_text(), | ||
| "episodes": "", | ||
| } | ||
| page_template = (this_dir / "index.html").read_text() | ||
| episode_template = (this_dir / "episode.html").read_text() | ||
| last_user_updated_date = None | ||
|
|
||
| for ep in episodes: | ||
| ep["episode_title"] = html.escape(ep["episode_title"]) | ||
| ep[DESCRIPTION] = _fix_unclosed_html_tags( | ||
| _convert_urls_to_links(ep[DESCRIPTION]), | ||
| ) | ||
| user_date = ep["userUpdatedDate"].split("T")[0] | ||
| if last_user_updated_date != user_date: | ||
| user_date = ep[date_field].split("T")[0] if ep.get(date_field) else "" | ||
| if last_user_updated_date != user_date and user_date: | ||
| page_vars["episodes"] += ( | ||
| "<h1><script>document.write(" | ||
| f'new Date("{ep["userUpdatedDate"]}").toLocaleDateString()' | ||
| f'new Date("{ep[date_field]}").toLocaleDateString()' | ||
| ")</script></h1><hr />" | ||
| ) | ||
| last_user_updated_date = user_date | ||
| if ep["starred"] == "1": | ||
|
|
||
| if ep.get("starred") == "1": | ||
| ep["starred"] = "⭐ " | ||
| else: | ||
| ep["starred"] = "" | ||
|
|
||
| try: | ||
| page_vars["episodes"] += episode_template.format_map(ep) | ||
| except KeyError as e: | ||
| print(f"Error formatting episode: KeyError {e}") | ||
| print(ep) | ||
| html_output_path.write_text(page_template.format_map(page_vars)) |
There was a problem hiding this comment.
This refactored helper function has a few issues that will cause it to fail at runtime:
TypeError: The function is called fromgenerate_html_deletedwith ashow_starred_iconargument, but the function signature doesn't accept it.KeyError: For any non-starred episode,ep["starred"]is never initialized. This will cause aKeyErrorwhen theepisode.htmltemplate is formatted, as it expects a{starred}key. Anelseblock is needed to setep["starred"] = ""for these cases.- Incorrect comparison: The check
ep.get("starred") == "1"is likely incorrect. The value from the database for a boolean/integer column is typically an integer (0or1), not a string. The comparison should be against an integer.
Here is a single suggestion that fixes all three issues by updating the function signature and logic.
def _generate_html_episodes(
episodes: list[dict[str, str]],
title: str,
html_output_path: Path,
date_field: str = "userUpdatedDate",
show_starred_icon: bool = True,
) -> None:
"""Generate HTML for any list of episodes."""
this_dir = Path(__file__).parent
page_vars = {
"title": title,
"style": Path(this_dir / "mvp.css").read_text(),
"script": Path(this_dir / "search.js").read_text(),
"episodes": "",
}
page_template = (this_dir / "index.html").read_text()
episode_template = (this_dir / "episode.html").read_text()
last_user_updated_date = None
for ep in episodes:
ep["episode_title"] = html.escape(ep["episode_title"])
ep[DESCRIPTION] = _fix_unclosed_html_tags(
_convert_urls_to_links(ep[DESCRIPTION]),
)
user_date = ep[date_field].split("T")[0] if ep.get(date_field) else ""
if last_user_updated_date != user_date and user_date:
page_vars["episodes"] += (
"<h1><script>document.write("
f'new Date("{ep[date_field]}").toLocaleDateString()'
")</script></h1><hr />"
)
last_user_updated_date = user_date
if show_starred_icon and ep.get("starred") == 1:
ep["starred"] = "⭐ "
else:
ep["starred"] = ""
try:
page_vars["episodes"] += episode_template.format_map(ep)
except KeyError as e:
print(f"Error formatting episode: KeyError {e}")
print(ep)
html_output_path.write_text(page_template.format_map(page_vars))| ) | ||
|
|
||
| results = self.db.execute(query).fetchall() | ||
| return self._process_query_results(results, fields) |
There was a problem hiding this comment.
According to the project's Python style guide, you should prefer to use named arguments when calling a method with more than one argument.1 This call uses positional arguments.
This also applies to the other calls to _process_query_results in this file (lines 542 and 561).
return self._process_query_results(results=results, fields=fields)Style Guide References
Footnotes
-
CLAUDE.md, line 8 ↩
| def generate_html_played(db_path: str, html_output_path: Path) -> None: | ||
| db = Datastore(db_path) | ||
| episodes = db.get_recently_played() | ||
| _generate_html_episodes(episodes, "Recently Played", html_output_path) |
There was a problem hiding this comment.
According to the project's Python style guide, you should prefer to use named arguments when calling a method with more than one argument.1 This call uses positional arguments.
This also applies to the other calls to _generate_html_episodes in this file.
Style Guide References
| _generate_html_episodes(episodes, "Recently Played", html_output_path) | |
| _generate_html_episodes(episodes=episodes, title="Recently Played", html_output_path=html_output_path) |
Footnotes
-
CLAUDE.md, line 8 ↩
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
overcast_to_sqlite/datastore.py (4)
467-467: Fix line length violation.Line exceeds the 88 character limit.
Apply this diff to fix:
-f"coalesce({EPISODES_EXTENDED}.description, 'No description') as description", +f"coalesce({EPISODES_EXTENDED}.description, " +f"'No description') as description",
511-514: Use iterable unpacking instead of concatenation.Replace list concatenation with unpacking for better performance and readability.
Apply this diff:
-fields = base_fields + [ - f"{USER_UPDATED_DATE}", - f"CASE WHEN {USER_REC_DATE} IS NOT NULL THEN 1 ELSE 0 END AS starred", -] +fields = [ + *base_fields, + f"{USER_UPDATED_DATE}", + f"CASE WHEN {USER_REC_DATE} IS NOT NULL THEN 1 ELSE 0 END AS starred", +]
530-533: Use iterable unpacking instead of concatenation.Apply this diff:
-fields = base_fields + [ - f"{USER_REC_DATE} as userRecDate", - "1 as starred", -] +fields = [ + *base_fields, + f"{USER_REC_DATE} as userRecDate", + "1 as starred", +]
549-552: Use iterable unpacking instead of concatenation.Apply this diff:
-fields = base_fields + [ - f"{USER_UPDATED_DATE}", - "0 as starred", -] +fields = [ + *base_fields, + f"{USER_UPDATED_DATE}", + "0 as starred", +]overcast_to_sqlite/html/index.html (1)
18-22: Consider moving inline styles to the stylesheet.For better maintainability and consistency, consider moving the inline styles to the CSS section.
Move these styles to the
<style>section:nav { text-align: center; padding: 1rem; border-bottom: 1px solid var(--border); margin-bottom: 1rem; } nav a { margin: 0 1rem; }Then simplify the HTML:
-<nav style="text-align: center; padding: 1rem; border-bottom: 1px solid var(--border); margin-bottom: 1rem;"> - <a href="overcast-played.html" style="margin: 0 1rem;">Recently Played</a> - <a href="overcast-starred.html" style="margin: 0 1rem;">Starred Episodes</a> - <a href="overcast-deleted.html" style="margin: 0 1rem;">Deleted Episodes</a> +<nav> + <a href="overcast-played.html">Recently Played</a> + <a href="overcast-starred.html">Starred Episodes</a> + <a href="overcast-deleted.html">Deleted Episodes</a> </nav>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.claude/settings.local.json(1 hunks).gitignore(1 hunks)CLAUDE.md(1 hunks)overcast_to_sqlite/cli.py(3 hunks)overcast_to_sqlite/datastore.py(9 hunks)overcast_to_sqlite/html/index.html(1 hunks)overcast_to_sqlite/html/page.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
overcast_to_sqlite/cli.py (2)
overcast_to_sqlite/html/page.py (3)
generate_html_deleted(103-111)generate_html_played(86-89)generate_html_starred(92-100)overcast_to_sqlite/datastore.py (1)
cleanup_old_episodes(563-593)
overcast_to_sqlite/html/page.py (2)
overcast_to_sqlite/cli.py (1)
html(303-328)overcast_to_sqlite/datastore.py (3)
get_recently_played(506-523)get_starred_episodes(525-542)get_deleted_episodes(544-561)
🪛 LanguageTool
CLAUDE.md
[grammar] ~4-~4: Ensure spelling is correct
Context: ... use f-string over string formatting or concatentation (except in logging strings) - Use async...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 GitHub Actions: Lint
overcast_to_sqlite/datastore.py
[error] 216-216: Ruff FURB162: Unnecessary timezone replacement with zero offset. Remove .replace() call.
[error] 467-467: Ruff E501: Line too long (90 > 88).
[error] 511-514: Ruff RUF005: Consider iterable unpacking instead of concatenation. Replace with iterable unpacking.
[error] 530-533: Ruff RUF005: Consider iterable unpacking instead of concatenation. Replace with iterable unpacking.
[error] 549-552: Ruff RUF005: Consider iterable unpacking instead of concatenation. Replace with iterable unpacking.
🔇 Additional comments (9)
pyproject.toml (1)
51-53: LGTM!The development dependency updates are appropriate and help keep the tooling current with the latest bug fixes and improvements.
.gitignore (1)
5-5: LGTM!Good generalization of the ignore pattern to accommodate multiple generated HTML files.
overcast_to_sqlite/datastore.py (2)
267-267: Good fix for SQL ambiguity.Making the GROUP BY clause explicit with the table name prevents potential ambiguity issues.
407-428: Verify safety of duplicate URL deduplicationOur search didn’t find any tests or additional documentation covering the deduplication step in
_clean_enclosure_urls, so there’s a risk of unintended data loss if distinct episodes differ only by query parameters.• Location: overcast_to_sqlite/datastore.py (lines 407–428)
• No tests found for duplicate‐URL handling
• No comments explaining why multiple query‐string variants might legitimately coexistPlease manually confirm that:
- No valid episodes rely on unique query parameters for differentiation, or
- New test cases are added to cover any edge cases in
_clean_enclosure_urls.overcast_to_sqlite/cli.py (2)
11-15: LGTM! Clean import organization.The new HTML generation function imports are properly organized and alphabetically sorted.
123-123: LGTM! Proper integration of cleanup logic.The cleanup call is appropriately placed after feed processing and has built-in safety checks in the datastore implementation.
overcast_to_sqlite/html/page.py (3)
43-84: LGTM! Well-structured helper function with good error handling.The refactored helper function properly consolidates the HTML generation logic with:
- Safe field access using
.get()methods- Proper HTML escaping and tag fixing
- Flexible date field handling
- Good error handling for template formatting
86-89: LGTM! Clean refactor using the new helper function.The function is properly simplified while maintaining the same functionality.
92-100: LGTM! Correct implementation for starred episodes.The function properly uses
"userRecDate"as the date field, which is appropriate for starred episodes.
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(find:*)" | ||
| ], | ||
| "deny": [] | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Local-only config should not live in VCS
settings.local.json implies host-specific overrides. Committing it will (1) leak personal tooling preferences, (2) force-merge noisy changes from every developer, and (3) risk exposing permissive command scopes in CI.
Move this file to your local workspace and add a tracked template (e.g. settings.example.json) instead.
+# .gitignore
+/.claude/settings.local.json🏁 Script executed:
#!/bin/bash
# Check if settings.local.json is tracked in the repo
echo "Tracked status for .claude/settings.local.json:"
git ls-files .claude/settings.local.json || echo "Not tracked"
# Check if .claude/settings.local.json is ignored in .gitignore
echo "Ignore entries for .claude/settings.local.json in .gitignore:"
grep -R "\.claude/settings.local.json" -n .gitignore || echo "No ignore entry found"Length of output: 416
Remove local-only config from version control
.claude/settings.local.json is currently tracked and not ignored. Local overrides shouldn’t live in VCS—they can leak personal tooling prefs, generate noisy merge conflicts, and expose overly broad permissions in CI. Please:
• Untrack the file and add it to .gitignore
# .gitignore
+/.claude/settings.local.jsongit rm --cached .claude/settings.local.json
git commit -m "Remove local settings from VCS and ignore file"• Add a tracked template for defaults, e.g. .claude/settings.example.json, with placeholder permissions
• Update docs (e.g., README) to instruct devs to copy the template to settings.local.json and customize locally
🤖 Prompt for AI Agents
In .claude/settings.local.json lines 1 to 8, the local configuration file is
currently tracked in version control, which risks leaking personal settings and
causing merge conflicts. To fix this, untrack the file using 'git rm --cached
.claude/settings.local.json' and commit the change, then add the file to
.gitignore to prevent future tracking. Create a new tracked template file named
.claude/settings.example.json with placeholder permissions for defaults.
Finally, update the project documentation to instruct developers to copy the
example file to settings.local.json and customize their local settings
accordingly.
| "allow": [ | ||
| "Bash(find:*)" | ||
| ], |
There was a problem hiding this comment.
"Bash(find:*)" is overly broad—tighten the permission
Allowing every possible find invocation lets prompt users traverse the entire repo or disk and chain arbitrary flags (-exec, -delete, etc.). That’s a sizable escalation surface.
Explicitly enumerate the safe sub-commands or restrict flags, e.g.:
- "Bash(find:*)"
+ "Bash(find . -type f -name '*.md')"or place a complementary deny rule blocking -exec, -delete, -execdir, etc.
🤖 Prompt for AI Agents
In .claude/settings.local.json around lines 3 to 5, the permission
"Bash(find:*)" is too broad and allows unsafe usage of the find command with any
flags. To fix this, replace the wildcard with an explicit list of safe find
sub-commands or add deny rules to block dangerous flags like -exec, -delete, and
-execdir. This will restrict the allowed find invocations to a safer subset and
reduce security risks.
| ## Python Practices | ||
| - Always use or add type hints | ||
| - Prefer @dataclasses where applicable | ||
| - Always use f-string over string formatting or concatentation (except in logging strings) |
There was a problem hiding this comment.
Fix typo in coding guidelines.
There's a spelling error: "concatentation" should be "concatenation".
Apply this diff to fix the typo:
-- Always use f-string over string formatting or concatentation (except in logging strings)
+- Always use f-string over string formatting or concatenation (except in logging strings)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Always use f-string over string formatting or concatentation (except in logging strings) | |
| - Always use f-string over string formatting or concatenation (except in logging strings) |
🧰 Tools
🪛 LanguageTool
[grammar] ~4-~4: Ensure spelling is correct
Context: ... use f-string over string formatting or concatentation (except in logging strings) - Use async...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In CLAUDE.md at line 4, correct the spelling mistake by changing
"concatentation" to "concatenation" in the coding guidelines to fix the typo.
| """Generate HTML pages for recently played, starred, and deleted episodes.""" | ||
| if output_path: | ||
| if Path(output_path).is_dir(): | ||
| html_output_path = Path(output_path) / "overcast-played.html" | ||
| else: | ||
| html_output_path = Path(output_path) | ||
| output_dir = ( | ||
| Path(output_path) | ||
| if Path(output_path).is_dir() | ||
| else Path(output_path).parent | ||
| ) | ||
| else: | ||
| html_output_path = Path(db_path).parent / "overcast-played.html" | ||
| generate_html_played(db_path, html_output_path) | ||
| print(f"📝Saved HTML to: file://{html_output_path.absolute()}") | ||
| output_dir = Path(db_path).parent | ||
|
|
||
| played_path = output_dir / "overcast-played.html" | ||
| starred_path = output_dir / "overcast-starred.html" | ||
| deleted_path = output_dir / "overcast-deleted.html" | ||
|
|
||
| generate_html_played(db_path, played_path) | ||
| generate_html_starred(db_path, starred_path) | ||
| generate_html_deleted(db_path, deleted_path) | ||
|
|
||
| print("📝Saved HTML files to:") | ||
| print(f" Recently Played: file://{played_path.absolute()}") | ||
| print(f" Starred Episodes: file://{starred_path.absolute()}") | ||
| print(f" Deleted Episodes: file://{deleted_path.absolute()}") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve path handling logic for better robustness.
The path handling logic has potential issues:
Path(output_path).is_dir()returnsFalseif the path doesn't exist, even if it's intended to be a directory- The fallback to
.parentassumes non-directories are files, which may not always be correct
Consider this more robust approach:
- if output_path:
- output_dir = (
- Path(output_path)
- if Path(output_path).is_dir()
- else Path(output_path).parent
- )
- else:
- output_dir = Path(db_path).parent
+ if output_path:
+ output_path_obj = Path(output_path)
+ if output_path_obj.exists() and output_path_obj.is_dir():
+ output_dir = output_path_obj
+ elif output_path_obj.suffix: # Has file extension, treat as file
+ output_dir = output_path_obj.parent
+ else: # No extension, treat as directory and create if needed
+ output_dir = output_path_obj
+ output_dir.mkdir(parents=True, exist_ok=True)
+ else:
+ output_dir = Path(db_path).parentThis handles non-existent paths more reliably and creates directories as needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """Generate HTML pages for recently played, starred, and deleted episodes.""" | |
| if output_path: | |
| if Path(output_path).is_dir(): | |
| html_output_path = Path(output_path) / "overcast-played.html" | |
| else: | |
| html_output_path = Path(output_path) | |
| output_dir = ( | |
| Path(output_path) | |
| if Path(output_path).is_dir() | |
| else Path(output_path).parent | |
| ) | |
| else: | |
| html_output_path = Path(db_path).parent / "overcast-played.html" | |
| generate_html_played(db_path, html_output_path) | |
| print(f"📝Saved HTML to: file://{html_output_path.absolute()}") | |
| output_dir = Path(db_path).parent | |
| played_path = output_dir / "overcast-played.html" | |
| starred_path = output_dir / "overcast-starred.html" | |
| deleted_path = output_dir / "overcast-deleted.html" | |
| generate_html_played(db_path, played_path) | |
| generate_html_starred(db_path, starred_path) | |
| generate_html_deleted(db_path, deleted_path) | |
| print("📝Saved HTML files to:") | |
| print(f" Recently Played: file://{played_path.absolute()}") | |
| print(f" Starred Episodes: file://{starred_path.absolute()}") | |
| print(f" Deleted Episodes: file://{deleted_path.absolute()}") | |
| """Generate HTML pages for recently played, starred, and deleted episodes.""" | |
| if output_path: | |
| output_path_obj = Path(output_path) | |
| if output_path_obj.exists() and output_path_obj.is_dir(): | |
| output_dir = output_path_obj | |
| elif output_path_obj.suffix: # Has file extension, treat as file | |
| output_dir = output_path_obj.parent | |
| else: # No extension, treat as directory and create if needed | |
| output_dir = output_path_obj | |
| output_dir.mkdir(parents=True, exist_ok=True) | |
| else: | |
| output_dir = Path(db_path).parent | |
| played_path = output_dir / "overcast-played.html" | |
| starred_path = output_dir / "overcast-starred.html" | |
| deleted_path = output_dir / "overcast-deleted.html" | |
| generate_html_played(db_path, played_path) | |
| generate_html_starred(db_path, starred_path) | |
| generate_html_deleted(db_path, deleted_path) | |
| print("📝Saved HTML files to:") | |
| print(f" Recently Played: file://{played_path.absolute()}") | |
| print(f" Starred Episodes: file://{starred_path.absolute()}") | |
| print(f" Deleted Episodes: file://{deleted_path.absolute()}") |
🤖 Prompt for AI Agents
In overcast_to_sqlite/cli.py around lines 307 to 328, the current path handling
logic incorrectly assumes that if output_path does not exist or is not a
directory, it must be a file and uses its parent directory, which can cause
errors. To fix this, check if output_path exists and is a directory; if not,
treat output_path as a directory path and create it if necessary before
proceeding. This ensures the output directory is correctly identified or
created, improving robustness.
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | ||
| limit_days = int(env_limit) |
There was a problem hiding this comment.
Fix incorrect walrus operator usage.
The current code compares the assignment result with is not None rather than the assigned value. This will always evaluate to True when os.getenv() returns any value.
Apply this diff to fix the logic:
-if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None:
+if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | |
| limit_days = int(env_limit) | |
| if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None: | |
| limit_days = int(env_limit) |
🤖 Prompt for AI Agents
In overcast_to_sqlite/datastore.py around lines 202 to 203, the walrus operator
is used incorrectly by combining assignment and a comparison with 'is not None',
causing the condition to always be true when a value is returned. Fix this by
first assigning the result of os.getenv("OVERCAST_LIMIT_DAYS") to a variable
using the walrus operator, then checking if that variable is not None before
converting it to int and assigning to limit_days.
| and datetime.datetime.fromisoformat( | ||
| episode[USER_UPDATED_DATE].replace("Z", "+00:00"), | ||
| ) | ||
| >= cutoff_date |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify timezone handling.
The manual replacement of 'Z' with '+00:00' is unnecessary when using fromisoformat in Python 3.11+, which supports the 'Z' suffix directly.
Apply this diff to simplify:
-datetime.datetime.fromisoformat(
- episode[USER_UPDATED_DATE].replace("Z", "+00:00"),
-)
+datetime.datetime.fromisoformat(episode[USER_UPDATED_DATE])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| and datetime.datetime.fromisoformat( | |
| episode[USER_UPDATED_DATE].replace("Z", "+00:00"), | |
| ) | |
| >= cutoff_date | |
| and datetime.datetime.fromisoformat(episode[USER_UPDATED_DATE]) | |
| >= cutoff_date |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 216-216: Ruff FURB162: Unnecessary timezone replacement with zero offset. Remove .replace() call.
🤖 Prompt for AI Agents
In overcast_to_sqlite/datastore.py around lines 215 to 218, the code manually
replaces the 'Z' suffix with '+00:00' before calling
datetime.datetime.fromisoformat, which is unnecessary in Python 3.11 and later.
Remove the .replace("Z", "+00:00") call and pass the ISO string with 'Z'
directly to fromisoformat to simplify the timezone handling.
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | ||
| limit_days = int(env_limit) |
There was a problem hiding this comment.
Fix incorrect walrus operator usage.
Same issue as earlier - the parentheses are needed around the assignment.
Apply this diff:
-if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None:
+if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if env_limit := os.getenv("OVERCAST_LIMIT_DAYS") is not None: | |
| limit_days = int(env_limit) | |
| if (env_limit := os.getenv("OVERCAST_LIMIT_DAYS")) is not None: | |
| limit_days = int(env_limit) |
🤖 Prompt for AI Agents
In overcast_to_sqlite/datastore.py around lines 570 to 571, the walrus operator
is used incorrectly without parentheses, causing the assignment to not work as
intended. Add parentheses around the assignment expression so that env_limit is
assigned the result of os.getenv("OVERCAST_LIMIT_DAYS") before checking if it is
not None. This will correctly assign and check the environment variable in one
step.
| def generate_html_deleted(db_path: str, html_output_path: Path) -> None: | ||
| db = Datastore(db_path) | ||
| episodes = db.get_deleted_episodes() | ||
| _generate_html_episodes( | ||
| episodes, | ||
| "Deleted Episodes", | ||
| html_output_path, | ||
| show_starred_icon=False, | ||
| ) |
There was a problem hiding this comment.
Fix the unused parameter causing TypeError.
The show_starred_icon=False parameter is passed to _generate_html_episodes, but this parameter is not defined in the helper function's signature, causing a TypeError at runtime.
Either remove the unused parameter or add it to the helper function:
Option 1: Remove the parameter (simpler)
- _generate_html_episodes(
- episodes,
- "Deleted Episodes",
- html_output_path,
- show_starred_icon=False,
- )
+ _generate_html_episodes(
+ episodes,
+ "Deleted Episodes",
+ html_output_path,
+ )Option 2: Add parameter support to the helper function
def _generate_html_episodes(
episodes: list[dict[str, str]],
title: str,
html_output_path: Path,
date_field: str = "userUpdatedDate",
+ show_starred_icon: bool = True,
) -> None:Then modify the starred icon logic on Line 75:
- if ep.get("starred") == "1":
+ if show_starred_icon and ep.get("starred") == "1":📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def generate_html_deleted(db_path: str, html_output_path: Path) -> None: | |
| db = Datastore(db_path) | |
| episodes = db.get_deleted_episodes() | |
| _generate_html_episodes( | |
| episodes, | |
| "Deleted Episodes", | |
| html_output_path, | |
| show_starred_icon=False, | |
| ) | |
| def generate_html_deleted(db_path: str, html_output_path: Path) -> None: | |
| db = Datastore(db_path) | |
| episodes = db.get_deleted_episodes() | |
| _generate_html_episodes( | |
| episodes, | |
| "Deleted Episodes", | |
| html_output_path, | |
| ) |
🤖 Prompt for AI Agents
In overcast_to_sqlite/html/page.py around lines 103 to 111, the call to
_generate_html_episodes includes the parameter show_starred_icon=False, but this
parameter is not defined in the helper function's signature, causing a
TypeError. To fix this, either remove the show_starred_icon parameter from the
call if it is not needed, or add show_starred_icon as a parameter to the
_generate_html_episodes function definition and update its logic accordingly,
especially around line 75 where starred icon handling occurs.
PR Type
Enhancement
Description
Add starred and deleted episode HTML pages
Implement episode age limiting with environment variable
Refactor HTML generation with shared template function
Add navigation between different episode views
Diagram Walkthrough
File Walkthrough
cli.py
Expand HTML command for multiple page typesovercast_to_sqlite/cli.py
datastore.py
Add episode filtering and new query methodsovercast_to_sqlite/datastore.py
OVERCAST_LIMIT_DAYSenvironmentvariable
cleanup_old_episodes()method for database maintenanceget_starred_episodes()andget_deleted_episodes()query methodsget_feeds_to_extend()page.py
Refactor HTML generation with shared templateovercast_to_sqlite/html/page.py
_generate_html_episodes()helper
generate_html_starred()andgenerate_html_deleted()functionsindex.html
Add navigation between episode pagesovercast_to_sqlite/html/index.html
settings.local.json
Add Claude AI configuration.claude/settings.local.json
CLAUDE.md
Add Python coding standards documentationCLAUDE.md
pyproject.toml
Update development dependenciespyproject.toml
types-python-dateutil
Important
Add functionality to generate HTML pages for starred and deleted episodes, with supporting database queries and functions.
html()incli.pynow generates HTML for starred and deleted episodes usinggenerate_html_starred()andgenerate_html_deleted().get_starred_episodes()andget_deleted_episodes()indatastore.pyto retrieve starred and deleted episodes._generate_html_episodes()inpage.py.get_starred_episodes()andget_deleted_episodes()indatastore.pyto query starred and deleted episodes._clean_enclosure_urls_simple()added todatastore.pyfor cleaning URLs without removing duplicates.uv.lockto reflect new package versions and sources.This description was created by
for 31b6db3. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores