Add gamification features and update gitignore#4
Conversation
…rning-engine Add gamification features (Pomodoro, TODO, Flashcards, Cheatsheets, Practice
Summary of ChangesHello, 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 application's capabilities by introducing a suite of new features designed to enhance the learning experience. It integrates gamification elements, study tools like flashcards and a Pomodoro timer, and content generation for practice problems and cheatsheets. These additions aim to make the platform more interactive, motivating, and personalized for users, while also improving the robustness of existing AI-powered functionalities. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| Uses Gemini to extract key points from notes. | ||
| """ | ||
| from pathlib import Path | ||
| from typing import List, Optional, Dict |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, we should remove the unused symbol List from the typing import while keeping the used symbols Optional and Dict. This eliminates the unnecessary dependency and satisfies the static analysis rule without affecting functionality.
Concretely, in core/cheatsheet.py, update the import line at the top of the file that currently reads from typing import List, Optional, Dict so that it imports only Optional and Dict. No other code changes are required, and no new methods or imports are needed.
| @@ -3,7 +3,7 @@ | ||
| Uses Gemini to extract key points from notes. | ||
| """ | ||
| from pathlib import Path | ||
| from typing import List, Optional, Dict | ||
| from typing import Optional, Dict | ||
| import json | ||
| import os | ||
| from dotenv import load_dotenv |
| Pure Python logic - no LLM calls needed. | ||
| """ | ||
| from pathlib import Path | ||
| from typing import List, Optional, Dict |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, we should remove the unused imported names List and Optional from the typing import, leaving only Dict, which is used as a return type annotation. This reduces unnecessary dependencies and clarifies which types are actually relied upon.
Concretely, in core/gamification.py, at the top of the file where from typing import List, Optional, Dict appears, we should edit that line to import only Dict. No other changes are needed, since there is no evidence of List or Optional usage in the provided snippet. The resulting line should be from typing import Dict. No new methods, definitions, or additional imports are required.
| @@ -3,7 +3,7 @@ | ||
| Pure Python logic - no LLM calls needed. | ||
| """ | ||
| from pathlib import Path | ||
| from typing import List, Optional, Dict | ||
| from typing import Dict | ||
| from datetime import datetime, date | ||
| import json | ||
| from core.models import UserProgress, Achievement, PomodoroSession, TodoItem |
| from datetime import datetime, timedelta | ||
| import json | ||
| import time | ||
| import os |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused import, you remove the import statement for the module that is not referenced anywhere in the file. This cleans up the namespace and improves readability without altering behavior, provided the module is indeed unused.
In this case, the best fix is to delete the line import os in core/pomodoro.py. No other changes are required because all the visible functionality uses pathlib.Path, json, datetime, time, and PomodoroSession, none of which depend on os being imported here. Specifically, in core/pomodoro.py, remove line 10 that reads import os. No new methods, imports, or definitions are needed.
| @@ -7,7 +7,6 @@ | ||
| from datetime import datetime, timedelta | ||
| import json | ||
| import time | ||
| import os | ||
| from core.models import PomodoroSession | ||
|
|
||
|
|
| from dotenv import load_dotenv | ||
| import google.genai as genai | ||
| from core.models import PracticeProblem | ||
| from core.utils import get_subject_dir |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused import, you remove the import statement (or the specific unused symbol) so that the module no longer depends on code it doesn't use. This improves readability and avoids unnecessary coupling.
In this file (core/practice.py), the single best fix is to delete the from core.utils import get_subject_dir line at the top of the file. No other code references get_subject_dir, and the class manages its paths using Path("data/practice_problems"), so removing the import will not change any existing behavior. Concretely, edit the import section near the top of core/practice.py and remove line 12 that imports get_subject_dir. No additional methods, definitions, or imports are required.
| @@ -9,7 +9,6 @@ | ||
| from dotenv import load_dotenv | ||
| import google.genai as genai | ||
| from core.models import PracticeProblem | ||
| from core.utils import get_subject_dir | ||
|
|
||
|
|
||
| class PracticeGenerator: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality, including gamification, flashcards, practice problems, and more. However, a security audit identified critical prompt injection vulnerabilities, especially where LLMs with external tool access (like Google Search) are involved, and arbitrary file write issues due to a lack of path validation in data export methods. Additionally, the code requires improvements in error handling, input validation, and robustness, specifically addressing bare except blocks, fuzzy string matching, and a bug in the Anki export feature related to multiline content.
| with open(output_file, 'w', encoding='utf-8') as f: | ||
| for card in deck.cards: | ||
| tags = " ".join(card.tags) | ||
| # Escape semicolons in content | ||
| front = card.front.replace(';', '\\;') | ||
| back = card.back.replace(';', '\\;') | ||
| f.write(f"{front};{back};{tags}\n") |
There was a problem hiding this comment.
The current implementation for Anki export might produce a corrupted CSV file if the card's front or back content contains newlines. The Anki CSV import format handles multiline fields correctly if they are enclosed in double quotes. Using Python's built-in csv module is the recommended way to handle this robustly. You will need to import csv at the top of the file.
| with open(output_file, 'w', encoding='utf-8') as f: | |
| for card in deck.cards: | |
| tags = " ".join(card.tags) | |
| # Escape semicolons in content | |
| front = card.front.replace(';', '\\;') | |
| back = card.back.replace(';', '\\;') | |
| f.write(f"{front};{back};{tags}\n") | |
| with open(output_file, 'w', encoding='utf-8', newline='') as f: | |
| writer = csv.writer(f, delimiter=';') | |
| for card in deck.cards: | |
| tags = " ".join(card.tags) | |
| writer.writerow([card.front, card.back, tags]) |
| try: | ||
| response = self.llm.invoke(prompt, tools=[types.Tool(google_search=types.GoogleSearch())] | ||
| ) | ||
| response = self.llm.invoke(prompt, tools=[types.Tool(google_search=types.GoogleSearch())]) |
There was a problem hiding this comment.
The LLM is invoked with the google_search tool enabled while using a prompt that incorporates untrusted data (extracted from PDF files). This combination is dangerous as a prompt injection attack (e.g., via a malicious PDF containing hidden instructions) could trick the LLM into performing unauthorized web searches or interacting with malicious external sites via the tool. Ensure that the prompt is properly sanitized using delimiters and consider if the google_search tool is strictly necessary for this specific function.
| f.write("---\n") | ||
|
|
||
| # Rate limit throttle: sleep 15s to respect 5 RPM | ||
| time.sleep(15) |
There was a problem hiding this comment.
The time.sleep(15) is hardcoded to handle API rate limiting. While the comment explains the reasoning (5 RPM), hardcoding magic numbers makes the code harder to maintain and adapt. It would be better to make this value configurable, for example, through an environment variable (e.g., API_REQUEST_DELAY_SECONDS). This would allow for easier adjustments if the API rate limits change or for different environments.
| time.sleep(15) | |
| # Rate limit throttle: sleep 15s to respect 5 RPM | |
| api_delay = int(os.getenv("API_REQUEST_DELAY_SECONDS", "15")) | |
| time.sleep(api_delay) |
|
|
||
| def get_notes_by_subject(self, subject: str) -> List[CustomNote]: | ||
| """Get all notes for a subject.""" | ||
| return [n for n in self.notes if n.subject and subject.lower() in n.subject.lower()] |
There was a problem hiding this comment.
The get_notes_by_subject method uses substring matching (in), which can be imprecise and return unexpected results if names are substrings of each other (e.g., "Math" and "Discrete Math"). For more predictable filtering, you should use an exact, case-insensitive comparison.
| return [n for n in self.notes if n.subject and subject.lower() in n.subject.lower()] | |
| return [n for n in self.notes if n.subject and subject.lower() == n.subject.lower()] |
|
|
||
| def get_notes_by_topic(self, topic: str) -> List[CustomNote]: | ||
| """Get all notes for a topic.""" | ||
| return [n for n in self.notes if n.topic and topic.lower() in n.topic.lower()] |
There was a problem hiding this comment.
Similar to get_notes_by_subject, get_notes_by_topic uses substring matching which can be imprecise. Consider changing to an exact, case-insensitive match for more predictable behavior.
| return [n for n in self.notes if n.topic and topic.lower() in n.topic.lower()] | |
| return [n for n in self.notes if n.topic and topic.lower() == n.topic.lower()] |
| prompt = f"""Analyze this educational content about "{topic_name}" and extract key information for a quick revision cheatsheet. | ||
|
|
||
| Content: | ||
| {content} |
There was a problem hiding this comment.
The _extract_key_points method constructs an LLM prompt by directly embedding the topic_name and content variables. Since content is read from note files that could be manipulated by a user, this creates a risk of prompt injection. An attacker could craft a malicious note file to manipulate the LLM's behavior or output format. It is recommended to use delimiters (e.g., triple quotes or XML-style tags) or structured input to separate user content from instructions.
| """ | ||
| types_str = ", ".join(problem_types) | ||
|
|
||
| prompt = f"""Generate {count} practice problems for the topic "{topic}" in {subject}. |
There was a problem hiding this comment.
The _generate_single_batch method constructs an LLM prompt by directly embedding the user-supplied topic and subject strings. This is vulnerable to prompt injection, allowing an attacker to manipulate the LLM's instructions by providing specially crafted topic or subject names. Consider sanitizing these inputs or using a more secure prompt construction method that clearly separates user-provided data from system instructions.
| console = Console() | ||
|
|
||
| # Persistence Manager | ||
| from core.persistence import get_persistence_manager |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix an unused import, remove the specific import statement for the symbol that is never referenced. This keeps dependencies explicit and the module easier to read, without changing program behavior.
Best fix here: in cli.py, delete line 50 (from core.persistence import get_persistence_manager) and keep the surrounding comments intact. No additional methods, imports, or definitions are needed.
| @@ -47,7 +47,6 @@ | ||
| console = Console() | ||
|
|
||
| # Persistence Manager | ||
| from core.persistence import get_persistence_manager | ||
|
|
||
| # Global state for current subject | ||
| CURRENT_SUBJECT_FILE = "data/.current_subject" |
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| subject = f.read().strip() | ||
| except: |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this safely without changing intended functionality, replace the bare except: in pomodoro() with except Exception:.
This keeps the fallback behavior (subject = "General") for normal runtime errors when reading data/.current_subject, while allowing KeyboardInterrupt and SystemExit to propagate correctly.
Change location:
- File:
cli.py - Region:
pomodoro()around lines 1567–1571 - Edit:
except:→except Exception: - No new imports, methods, or dependencies are required.
| @@ -1567,7 +1567,7 @@ | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| subject = f.read().strip() | ||
| except: | ||
| except Exception: | ||
| subject = "General" | ||
|
|
||
| console.print(f"[bold green]🍅 Starting {duration}m focus session for '{subject}'...[/bold green]") |
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| subject = f.read().strip() | ||
| except: pass |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use targeted exception handling instead of except: pass.
Best fix without changing intended behavior:
- In
cli.py, intodo_addaround lines 1631–1634, replace the bare emptyexceptwith:except FileNotFoundError:→ keepsubject = None(expected case).except OSError as e:→ log a warning and keepsubject = Noneso command still works.
- This preserves current functionality (task can still be added without subject), but avoids silently swallowing all exceptions and provides diagnosability.
No new imports are needed because logging is already imported.
| @@ -1631,7 +1631,11 @@ | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| subject = f.read().strip() | ||
| except: pass | ||
| except FileNotFoundError: | ||
| subject = None | ||
| except OSError as e: | ||
| logging.warning(f"Unable to read current subject context: {e}") | ||
| subject = None | ||
|
|
||
| due_date = None | ||
| if due: |
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| subject = f.read().strip() | ||
| except: pass |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use explicit exception handling instead of bare except:.
Best fix here (without changing intended functionality) is to replace except: pass with except Exception: pass in the todo_add subject-loading block in cli.py around lines 1631–1634. This keeps the “ignore normal failures when reading context file” behavior while allowing KeyboardInterrupt and SystemExit to propagate correctly.
No new imports, methods, or definitions are required.
| @@ -1631,7 +1631,8 @@ | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| subject = f.read().strip() | ||
| except: pass | ||
| except Exception: | ||
| pass | ||
|
|
||
| due_date = None | ||
| if due: |
| if due: | ||
| try: | ||
| due_date = datetime.strptime(due, "%Y-%m-%d") | ||
| except: |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use a specific exception instead of bare except: in the date parsing block.
Best fix without changing functionality:
- In
cli.py, intodo_addaround lines 1638–1642, replaceexcept:withexcept ValueError:. datetime.strptimeraisesValueErrorfor invalid date formats/values, so this preserves current behavior (print message + return) for invalid input while allowingKeyboardInterruptandSystemExitto propagate correctly.- No new imports or helper methods are required.
| @@ -1637,7 +1637,7 @@ | ||
| if due: | ||
| try: | ||
| due_date = datetime.strptime(due, "%Y-%m-%d") | ||
| except: | ||
| except ValueError: | ||
| console.print("[red]Invalid date format. Use YYYY-MM-DD[/red]") | ||
| return | ||
|
|
| # Extract timestamp from filename pattern: *_YYYYMMDD_HHMMSS.ext | ||
| ts_part = backup.stem.split('_')[-2:] | ||
| timestamp = f"{ts_part[0]} {ts_part[1][:2]}:{ts_part[1][2:4]}:{ts_part[1][4:]}" | ||
| except: |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Replace the bare except: in backup_list with a specific exception class that matches parsing failures, while preserving the existing fallback behavior.
Best fix here: change except: to except (IndexError, ValueError): in the for loop in backup_list (around lines 1862–1867 in cli.py). This keeps behavior identical for invalid filename formats (still sets timestamp to "Unknown") but no longer catches BaseException, allowing KeyboardInterrupt and SystemExit to propagate correctly.
No new imports, methods, or dependencies are required.
| @@ -1863,7 +1863,7 @@ | ||
| # Extract timestamp from filename pattern: *_YYYYMMDD_HHMMSS.ext | ||
| ts_part = backup.stem.split('_')[-2:] | ||
| timestamp = f"{ts_part[0]} {ts_part[1][:2]}:{ts_part[1][2:4]}:{ts_part[1][4:]}" | ||
| except: | ||
| except (IndexError, ValueError): | ||
| timestamp = "Unknown" | ||
|
|
||
| table.add_row(str(i), timestamp, backup.name) |
| console.print("[bold cyan]AI Learning Engine CLI[/bold cyan]") | ||
| try: | ||
| app(["--help"], standalone_mode=False) | ||
| except SystemExit: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this without changing behavior, keep catching SystemExit in interactive_mode() startup help rendering, but replace the empty pass with an explanatory comment and a debug log call. This preserves the current UX (don’t exit interactive mode when help triggers SystemExit) while eliminating the “empty except” anti-pattern and making intent explicit.
In cli.py, within interactive_mode() around lines 1922–1927:
- Change
except SystemExit: passto:- include a short comment explaining why this is intentionally ignored;
- call
logging.debug(...)(already imported at top) to record the event.
- Leave the generic
except Exceptionbehavior unchanged unless desired by another rule; here we only address the reported line.
No new imports, methods, or dependencies are required.
| @@ -1922,7 +1922,8 @@ | ||
| try: | ||
| app(["--help"], standalone_mode=False) | ||
| except SystemExit: | ||
| pass | ||
| # Help rendering may raise SystemExit; ignore it to keep interactive mode running. | ||
| logging.debug("Ignoring SystemExit raised while displaying startup help in interactive mode.") | ||
| except Exception: | ||
| pass | ||
|
|
| app(["--help"], standalone_mode=False) | ||
| except SystemExit: | ||
| pass | ||
| except Exception: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Handle the broad exception without changing control flow: keep interactive mode running, but surface the failure.
Best fix: replace the empty except Exception: pass in interactive_mode() with an exception handler that logs and/or prints a warning. This preserves existing functionality (CLI continues even if help rendering fails) while avoiding silent failure.
Edit in cli.py, inside interactive_mode() near lines 1922–1927:
- Keep
except SystemExit:behavior as-is. - Change
except Exception:block to capture the exception (as e) and print a warning viaconsole.print(...)(and optionallylogging.debug(..., exc_info=True)sinceloggingis already imported).
No new imports or dependencies are required.
| @@ -1923,8 +1923,9 @@ | ||
| app(["--help"], standalone_mode=False) | ||
| except SystemExit: | ||
| pass | ||
| except Exception: | ||
| pass | ||
| except Exception as e: | ||
| console.print(f"[yellow]Warning: failed to display startup help: {e}[/yellow]") | ||
| logging.debug("Startup help display failed", exc_info=True) | ||
|
|
||
| console.print("\nType 'exit' to quit.\n") | ||
|
|
| # Invoke the app directly for speed | ||
| # standalone_mode=False prevents SystemExit on error/help | ||
| app(args, standalone_mode=False) | ||
| except SystemExit: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this without changing functionality, replace the empty except SystemExit: pass in interactive_mode() (around lines 1952–1953 in cli.py) with explicit handling that documents intent and safely continues the REPL loop.
Best single fix:
- Catch
SystemExit as e. - Add a brief comment explaining why it is intentionally suppressed in interactive mode.
- Optionally emit a low-noise debug log message with the exit code for traceability.
continuethe loop explicitly.
This preserves current behavior (do not exit the interactive shell when a command internally triggers SystemExit) while removing the “empty except” anti-pattern flagged by CodeQL.
| @@ -1949,8 +1949,10 @@ | ||
| # Invoke the app directly for speed | ||
| # standalone_mode=False prevents SystemExit on error/help | ||
| app(args, standalone_mode=False) | ||
| except SystemExit: | ||
| pass | ||
| except SystemExit as e: | ||
| # Keep interactive mode running if a command triggers SystemExit. | ||
| logging.debug("Suppressed SystemExit in interactive mode (code=%s)", e.code) | ||
| continue | ||
| except Exception as e: | ||
| console.print(f"[red]Error executing command: {e}[/red]") | ||
|
|
| from pathlib import Path | ||
| from typing import List, Optional, Dict | ||
| from datetime import datetime, date | ||
| import json |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused import, remove the import statement if the module is not actually used anywhere in the file. This eliminates the unnecessary dependency and makes the code slightly clearer.
Concretely, in core/gamification.py, the json module imported on line 8 is not used in any of the shown methods; all persistence is handled via .persistence. The best fix that does not change functionality is to delete the import json line and leave all other imports and code unchanged.
You only need to modify the import section at the top of core/gamification.py, removing line 8; no additional methods, imports, or definitions are required.
| @@ -5,7 +5,6 @@ | ||
| from pathlib import Path | ||
| from typing import List, Optional, Dict | ||
| from datetime import datetime, date | ||
| import json | ||
| from core.models import UserProgress, Achievement, PomodoroSession, TodoItem | ||
|
|
||
|
|
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| except: pass |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General fix: Replace the bare except: with an except Exception: clause (or more specific exceptions like FileNotFoundError), so that KeyboardInterrupt and SystemExit are not caught and can behave as intended, while still ignoring ordinary runtime errors when reading the file.
Best fix for this snippet: On line 15, change except: pass to except Exception: pass. This keeps the behavior of silently ignoring any standard error (e.g., file not found, permission issues) while ensuring that special control-flow exceptions are not swallowed. No additional imports or helper functions are necessary. The change is fully localized to the show_todo_page function in streamlit/ui/todos.py.
Concretely:
- Edit
streamlit/ui/todos.py. - In the
show_todo_pagefunction’stry/exceptaround opening"data/.current_subject", change theexcept:toexcept Exception:.
| @@ -12,7 +12,8 @@ | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| except: pass | ||
| except Exception: | ||
| pass | ||
|
|
||
| # Add Task Form | ||
| with st.expander("Add New Task", expanded=False): |
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| except: pass |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, the exception handling should be made explicit and meaningful: catch only the expected exceptions when trying to read the context file and handle them in a clear way. At minimum, we should (1) avoid a bare except:, (2) add an explanatory comment, and ideally (3) log the error or give a non-intrusive hint to the user.
The best way to fix this without changing existing functionality is:
- Replace
except: passwith a more specificexcept OSError:(covers file-not-found, permission issues, etc.) and possibly a genericexcept Exception as e:if we want to surface unexpected errors. - Inside the
exceptblock, leavecurrent_subjectasNone(the existing behavior) and optionally log the issue using Streamlit’s logging; the least invasive is to add a comment explaining that missing/unreadable context is treated as "no current subject". - This change is localized to the
try/exceptaround readingdata/.current_subjectinshow_todo_pageinstreamlit/ui/todos.py. No new imports are strictly required if we keep handling minimal (comment only), but adding a very lightweight, non-intrusive user message (st.infoorst.warning) is also possible. To be maximally conservative about UX change, we will just add a comment and narrow the exception type.
Concretely:
- In
streamlit/ui/todos.py, lines 12–15, replace theexcept: passwith:
except OSError:
# If the current subject file is missing or unreadable, fall back to no current subject.
current_subject = NoneThis preserves behavior (since current_subject was already None by default) while making the intent explicit and avoiding a bare, empty except.
| @@ -12,7 +12,9 @@ | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| except: pass | ||
| except OSError: | ||
| # If the current subject file is missing or unreadable, fall back to no current subject. | ||
| current_subject = None | ||
|
|
||
| # Add Task Form | ||
| with st.expander("Add New Task", expanded=False): |
| import pandas as pd | ||
| import altair as alt | ||
| from core.gamification import GamificationManager | ||
| from core.models import UserProgress, Achievement |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, remove the unused imports from the file so that only actually used modules are imported. This reduces unnecessary dependencies and satisfies the static analysis rule.
Concretely, in streamlit/ui/progress.py, line 6 (from core.models import UserProgress, Achievement) should be deleted, because neither UserProgress nor Achievement is referenced anywhere in the shown function. No other code changes, imports, or definitions are needed, as the rest of the file uses only streamlit, pandas, altair (potentially, though alt is also currently unused but not flagged here), and GamificationManager. This change preserves all existing behavior while resolving both alert variants (for UserProgress and Achievement).
| @@ -3,8 +3,8 @@ | ||
| import pandas as pd | ||
| import altair as alt | ||
| from core.gamification import GamificationManager | ||
| from core.models import UserProgress, Achievement | ||
|
|
||
|
|
||
| def show_progress_page(): | ||
| """ | ||
| Display the gamification progress dashboard. |
| import streamlit as st | ||
| from datetime import datetime | ||
| import pandas as pd | ||
| import altair as alt |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix an unused import, remove the import line or, if the imported name is actually needed, update the code to use it. The goal is to keep imports minimal and only for symbols that are referenced.
In this specific case, the best fix without changing existing functionality is to delete the unused altair import line. The code already renders the chart using st.bar_chart, and there is no reference to alt. Removing import altair as alt from streamlit/ui/progress.py will not affect the function’s behavior, since alt is never used. Concretely, in streamlit/ui/progress.py, remove line 4 (import altair as alt) and leave all other imports and code unchanged.
| @@ -1,7 +1,6 @@ | ||
| import streamlit as st | ||
| from datetime import datetime | ||
| import pandas as pd | ||
| import altair as alt | ||
| from core.gamification import GamificationManager | ||
| from core.models import UserProgress, Achievement | ||
|
|
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| except: pass |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the fix is to avoid a bare, empty except: block. Either (a) catch only the specific exception(s) that are expected and harmless (e.g., FileNotFoundError), and/or (b) add minimal handling such as logging or a comment explaining why the exception is safely ignored. This retains the intended fallback behavior while improving debuggability and code clarity.
For this code, the best fix with minimal behavioral change is:
- Keep the default
current_subject = "General"as-is. - Replace the bare
except: passwith:- A more specific exception type (e.g.,
FileNotFoundError,OSError,IOError). - A log message or at least an explanatory comment.
- A more specific exception type (e.g.,
- Since this is a Streamlit app, we should avoid showing errors to the user for a missing optional file, but we can still log to the terminal using the standard library
loggingmodule, which is a well-known dependency. - To implement this:
- Add
import loggingat the top ofstreamlit/ui/pomodoro.py. - Initialize a module-level logger, e.g.
logger = logging.getLogger(__name__). - Change the
exceptblock to catch expected file-related errors and log them withlogger.debugorlogger.warning. Usingdebugis safest if we want to avoid noisy logs in production, butwarningmay be more visible; I’ll usedebugto keep behavior minimally intrusive.
- Add
Concretely, in show_pomodoro_page we will modify lines 23–26 to:
try:
with open("data/.current_subject", "r") as f:
current_subject = f.read().strip()
except (FileNotFoundError, OSError) as e:
logger.debug("Could not read current subject file 'data/.current_subject': %s", e)And we’ll add the logging import and logger definition near the top of the file.
| @@ -2,7 +2,10 @@ | ||
| import time | ||
| from datetime import datetime, timedelta | ||
| from core.pomodoro import PomodoroTimer | ||
| import logging | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| def show_pomodoro_page(): | ||
| st.header("🍅 Pomodoro Timer") | ||
|
|
||
| @@ -23,7 +25,9 @@ | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| except: pass | ||
| except (FileNotFoundError, OSError) as e: | ||
| # Fallback to default subject if file is missing or unreadable | ||
| logger.debug("Could not read current subject file 'data/.current_subject': %s", e) | ||
|
|
||
| # Timer Display | ||
| status = "Ready" |
| @@ -0,0 +1,89 @@ | |||
| import streamlit as st | |||
| import time | |||
| from datetime import datetime, timedelta | |||
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused-import problem, you remove the unused symbol from the import statement (or delete the whole import if none of its symbols are used). This preserves functionality because imports only matter when the imported names are actually referenced.
In this case, datetime is used in datetime.now(), but timedelta is not used anywhere in streamlit/ui/pomodoro.py. The best fix is to edit the import on line 3 to import only datetime, removing timedelta from that line. No other code changes, methods, or definitions are required.
Specifically, in streamlit/ui/pomodoro.py, change:
from datetime import datetime, timedeltato:
from datetime import datetimeThis addresses the CodeQL warning without altering existing functionality.
| @@ -1,6 +1,6 @@ | ||
| import streamlit as st | ||
| import time | ||
| from datetime import datetime, timedelta | ||
| from datetime import datetime | ||
| from core.pomodoro import PomodoroTimer | ||
|
|
||
| def show_pomodoro_page(): |
| current_subject = f.read().strip() | ||
| if current_subject: | ||
| st.info(f"Context: {current_subject}") | ||
| except: pass |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, this problem is fixed by never using bare except: (which catches BaseException) and instead catching the specific exception types you expect (most often Exception, or more narrowly, concrete subclasses like OSError, IOError, FileNotFoundError, ValueError, etc.). This ensures that KeyboardInterrupt and SystemExit are not accidentally swallowed.
For this code, the try block only opens and reads a simple text file (data/.current_subject). The realistic failures here are file/OS-related exceptions such as FileNotFoundError, PermissionError, or more generally OSError. We can safely change except: to except OSError: (or except Exception: if we want a wider net) while keeping the behavior the same: ignore any file-related errors and proceed without a current subject.
Concretely:
- In
streamlit/ui/flashcards.py, around lines 15–21, replaceexcept: passwith a more specific handler such asexcept OSError: pass. - No new imports are required because
OSErroris a built-in. - No other parts of the file need to change; the functionality remains that failures to read
.current_subjectare silently ignored, butKeyboardInterruptandSystemExitwill now propagate correctly.
| @@ -18,7 +18,8 @@ | ||
| current_subject = f.read().strip() | ||
| if current_subject: | ||
| st.info(f"Context: {current_subject}") | ||
| except: pass | ||
| except OSError: | ||
| pass | ||
|
|
||
| if st.toggle("Show all subjects", value=False): | ||
| filtered_decks = decks |
| current_subject = f.read().strip() | ||
| if current_subject: | ||
| st.info(f"Context: {current_subject}") | ||
| except: pass |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix an empty except block, either (a) handle the specific, expected exceptions (e.g., FileNotFoundError, OSError) and take an appropriate fallback action, or (b) if truly intentional, add logging and/or a clear explanatory comment. Avoid a bare except so that unexpected exceptions still surface.
For this code, the best fix that preserves existing behavior is:
- Narrow the exception to I/O-related ones that can occur when reading the context file (
OSError,IOError,FileNotFoundError). - On failure, keep the current behavior (no context), but log the issue at a low level so developers can diagnose it if needed.
- Optionally use
st.warningor similar for user feedback, but to avoid changing UX, we’ll only log.
Since we cannot assume a project-wide logger, we can use Python’s standard logging module, which is a well-known library. We’ll add an import for logging at the top of streamlit/ui/flashcards.py and replace the bare except: pass with a more precise except (OSError, IOError) as e: that logs the error at debug level, plus a short comment clarifying that the context file is optional.
Concretely:
- Edit
streamlit/ui/flashcards.py:- Add
import loggingalongside the existing imports. - Replace lines 16–21:
with:
16: try: 17: with open("data/.current_subject", "r") as f: 18: current_subject = f.read().strip() 19: if current_subject: 20: st.info(f"Context: {current_subject}") 21: except: pass
# Best-effort read of optional current-subject context file. try: with open("data/.current_subject", "r") as f: current_subject = f.read().strip() if current_subject: st.info(f"Context: {current_subject}") except (OSError, IOError) as e: logging.getLogger(__name__).debug( "Could not read current subject context file: %s", e )
- Add
- No other functional changes are needed.
| @@ -1,5 +1,6 @@ | ||
| import streamlit as st | ||
| from core.flashcards import FlashcardManager | ||
| import logging | ||
|
|
||
| def show_flashcards_page(): | ||
| st.header("🎴 Flashcards") | ||
| @@ -13,12 +14,16 @@ | ||
|
|
||
| # Determine context | ||
| current_subject = None | ||
| try: | ||
| # Best-effort read of optional current-subject context file. | ||
| try: | ||
| with open("data/.current_subject", "r") as f: | ||
| current_subject = f.read().strip() | ||
| if current_subject: | ||
| st.info(f"Context: {current_subject}") | ||
| except: pass | ||
| except (OSError, IOError) as e: | ||
| logging.getLogger(__name__).debug( | ||
| "Could not read current subject context file: %s", e | ||
| ) | ||
|
|
||
| if st.toggle("Show all subjects", value=False): | ||
| filtered_decks = decks |
| import json | ||
| import shutil | ||
| from pathlib import Path | ||
| from typing import Any, Optional, Type |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, remove the unused Any from the typing import so that only the actually used names remain. This keeps the code cleaner and aligns with static-analysis expectations without affecting functionality.
Concretely, in core/persistence.py, on the import line from typing import Any, Optional, Type, delete Any and the accompanying comma, changing it to from typing import Optional, Type. No additional methods, imports, or definitions are required, and no other parts of the file need modification.
| @@ -5,7 +5,7 @@ | ||
| import json | ||
| import shutil | ||
| from pathlib import Path | ||
| from typing import Any, Optional, Type | ||
| from typing import Optional, Type | ||
| from datetime import datetime | ||
| from pydantic import BaseModel, ValidationError | ||
|
|
| Module for generating conceptual Mermaid diagrams using Gemini AI via LangChain. | ||
| Focuses on educational/conceptual understanding of topics. | ||
| """ | ||
| import json |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused import, the general approach is to delete the import statement (or the specific unused name in a multi-name import) so that the code no longer declares dependencies it does not use. This improves readability and keeps the import list aligned with actual usage.
For this specific case in core/diagram_generator.py, the best fix is to remove the import json line at the top of the file (line 5). No other edits are necessary: there are no visible references to json in the provided code, and removing the import will not change runtime behavior. Only the single line import json should be deleted; all other imports (logging, os, time, typing, Path, LangChain, etc.) remain unchanged.
| @@ -2,7 +2,6 @@ | ||
| Module for generating conceptual Mermaid diagrams using Gemini AI via LangChain. | ||
| Focuses on educational/conceptual understanding of topics. | ||
| """ | ||
| import json | ||
| import logging | ||
| import os | ||
| import time |
| import json | ||
| import logging | ||
| import os | ||
| import time |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused import, you remove the import statement for the module that is not referenced anywhere in the file. This reduces unnecessary dependencies and noise without changing runtime behavior, as unused imports from the standard library do not have side effects.
Specifically for core/diagram_generator.py, remove the line import time (line 8). No additional code changes are required, and no new imports, methods, or definitions are needed. The rest of the imports (json, logging, os, List, Optional, Path, LangChain-related imports, and local modules) remain unchanged.
| @@ -5,7 +5,6 @@ | ||
| import json | ||
| import logging | ||
| import os | ||
| import time | ||
| from typing import List, Optional | ||
| from pathlib import Path | ||
|
|
| import logging | ||
| import os | ||
| import time | ||
| from typing import List, Optional |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused-import issue, remove the imported symbol that is not referenced anywhere in the file, or adjust the import so that only actually used names are imported. This avoids unnecessary dependencies and improves readability without affecting runtime behavior.
In this file, the best minimal change is to modify the from typing import List, Optional line so that it imports only Optional, leaving the rest of the file unchanged. Specifically, in core/diagram_generator.py, locate line 9:
from typing import List, Optionaland change it to:
from typing import OptionalNo additional methods, imports, or definitions are needed.
| @@ -6,7 +6,7 @@ | ||
| import logging | ||
| import os | ||
| import time | ||
| from typing import List, Optional | ||
| from typing import Optional | ||
| from pathlib import Path | ||
|
|
||
| from langchain_google_genai import ChatGoogleGenerativeAI |
| import os | ||
| import time | ||
| from typing import List, Optional | ||
| from pathlib import Path |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix an unused import, you simply remove the import statement (or the unused name from a multi-name import) so that the code only imports what it actually uses. This eliminates unnecessary dependencies and silences the static analysis warning without affecting functionality.
In this specific case, the import from pathlib import Path on line 10 of core/diagram_generator.py should be removed entirely, assuming there is no hidden usage below the snippet. No additional methods, definitions, or other imports are needed. The rest of the imports (json, logging, os, time, List, Optional, LangChain/Gemini-related imports, and local models) remain unchanged. The only modification is deleting the single unused Path import line in this file.
| @@ -7,7 +7,6 @@ | ||
| import os | ||
| import time | ||
| from typing import List, Optional | ||
| from pathlib import Path | ||
|
|
||
| from langchain_google_genai import ChatGoogleGenerativeAI | ||
| from google.api_core.exceptions import ResourceExhausted |
…d-friendly docs Agent-Logs-Url: https://github.com/djmahe4/Study-AI-Agent/sessions/17de5520-0157-4d70-8411-895c8959c558 Co-authored-by: djmahe4 <137691824+djmahe4@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Agent-Logs-Url: https://github.com/djmahe4/Study-AI-Agent/sessions/5aa72fbc-8c76-4caf-90f4-e3c2df8d93b8 Co-authored-by: djmahe4 <137691824+djmahe4@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…errors Add semantic query classifier, agent skills framework, and input sanitization hardening
…m/djmahe4/Study-AI-Agent into copilot/design-ai-learning-engine
| """ | ||
|
|
||
| import sys | ||
| import re |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, unused imports should be removed to avoid unnecessary dependencies and improve readability. Here, the best fix is simply to delete the unused import re line from skills/concept-mastery/scripts/explain_topic.py.
Concretely, in skills/concept-mastery/scripts/explain_topic.py, remove line 14 containing import re, leaving the neighboring imports (import sys and from pathlib import Path) unchanged. No additional methods, imports, or definitions are needed, and this change does not affect existing behavior.
| @@ -11,7 +11,6 @@ | ||
| """ | ||
|
|
||
| import sys | ||
| import re | ||
| from pathlib import Path | ||
|
|
||
| # Add repo root so we can import core.input_validator |
| idx = args.index("-o") | ||
| try: | ||
| output_file = args[idx + 1] | ||
| except IndexError: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, to fix an empty except you should either (1) handle the error meaningfully (e.g., log it, show a message, adjust control flow) or (2) re-raise it, possibly after adding context. For CLI argument parsing, the best fix is to treat a missing argument as invalid usage: print a clear error message, show usage, and exit with a non-zero code.
For this specific code, the best minimal change that doesn’t alter intended functionality (beyond making errors explicit) is to replace the except IndexError: pass with logic that prints an error about the missing output filename, prints the usage line that’s already present in main(), and calls sys.exit(1). This uses only existing imports (sys is already imported at the top). All changes are confined to the try/except block around output_file = args[idx + 1] in skills/mindmap-generator/scripts/syllabus_to_mindmap.py (around lines 79–85). No new methods or imports are required.
| @@ -81,7 +81,9 @@ | ||
| try: | ||
| output_file = args[idx + 1] | ||
| except IndexError: | ||
| pass | ||
| print("Error: Missing output filename after '-o'.") | ||
| print("Usage: syllabus_to_mindmap.py <syllabus.txt> [-o output.mmd]") | ||
| sys.exit(1) | ||
|
|
||
| raw = Path(input_file).read_text(encoding="utf-8") | ||
| clean = sanitize_syllabus_text(raw) |
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Code Review SummaryStatus: 70 Active Issues | Recommendation: Address critical issues before merge OverviewTotal active issues: 70. Incremental review examined latest commits adding MCP server, enhanced PYQ extraction, skill generation, and YouTube subtitle extraction.
Critical Issues (Blockers)
Warning Issues
Unchanged Files (51 existing issues still apply)
Positive Changes
Note: CRITICAL issues are blocking runtime failures. Fix before merge. Reviewed by ling-2.6-1t-20260423:free · 4,554,512 tokens |
| from pydantic import BaseModel as PydanticModel | ||
| from google.genai.errors import APIError | ||
| import tenacity | ||
| from tenacity import retry_if_exception, wait_exponential, stop_after_attempt, wait_fixed |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix an unused import, remove the unused symbol from the import statement and keep the rest unchanged. This preserves functionality while resolving the static analysis warning.
In core/gemini_processor.py, update line 22 import from tenacity by deleting wait_fixed from the imported names. No other code changes are required, and no new methods, definitions, or dependencies are needed.
| @@ -19,7 +19,7 @@ | ||
| from pydantic import BaseModel as PydanticModel | ||
| from google.genai.errors import APIError | ||
| import tenacity | ||
| from tenacity import retry_if_exception, wait_exponential, stop_after_attempt, wait_fixed | ||
| from tenacity import retry_if_exception, wait_exponential, stop_after_attempt | ||
| from dotenv import load_dotenv | ||
|
|
||
| # Module-level logger – configuration is the responsibility of the application |
| match = re.search(r'"retryDelay":\s*"([\d\.]+)s"', str(error)) | ||
| if match: | ||
| return float(match.group(1)) | ||
| except Exception: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this without changing behavior, keep the “best-effort return None” flow but replace the empty except with explicit handling that records diagnostic context.
Best approach in core/gemini_processor.py (inside _extract_retry_delay):
- Keep the existing
trylogic. - Replace
except Exception: passwithexcept Exception as exc: - Log at debug level (or warning if preferred) that retry delay extraction failed, then return
None.
This preserves current functionality (fallback backoff still used) while avoiding silent exception suppression and satisfying CodeQL’s complaint about empty except blocks.
| @@ -162,8 +162,8 @@ | ||
| match = re.search(r'"retryDelay":\s*"([\d\.]+)s"', str(error)) | ||
| if match: | ||
| return float(match.group(1)) | ||
| except Exception: | ||
| pass | ||
| except Exception as exc: | ||
| logger.debug("Failed to extract retryDelay from API error: %s", exc) | ||
| return None | ||
|
|
||
| def _wait_strategy(self, retry_state: tenacity.RetryCallState) -> float: |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| # Local fallback: agentic skills via Ollama. | ||
| # Same structured educational output (Explanation + Python + Mermaid) | ||
| # is requested so the toggle only changes *which* model answers, not the format. | ||
| from .agents.free_llm_client import EDUCATIONAL_SYSTEM |
There was a problem hiding this comment.
CRITICAL: Import error - relative import .agents will fail when module is run directly. Change to:
| from .agents.free_llm_client import EDUCATIONAL_SYSTEM | |
| from agents.free_llm_client import EDUCATIONAL_SYSTEM |
| {"role": "user", "content": prompt}], | ||
| model="llama3" | ||
| ) | ||
|
|
There was a problem hiding this comment.
WARNING: Hardcoded model name llama3 should use config setting for flexibility.
|
|
||
| yield json.dumps({"type": "done"}) + "\n" | ||
|
|
||
| return StreamingResponse(event_generator(), media_type="text/event-stream") |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix this without changing functionality, keep returning a user-friendly fallback message from call_free_llm, but remove interpolation of the raw exception object.
Best approach: in mcp-server/agents/free_llm_client.py, replace except Exception as e block with a generic message (no {e}), and log the detailed exception server-side using Python’s standard logging (with logger.exception(...) so stack trace is retained in logs).
Changes needed:
- In
mcp-server/agents/free_llm_client.py:- Add
import logging. - Define module logger:
logger = logging.getLogger(__name__). - Replace the
except Exception as ereturn line with:logger.exception("Error calling free-llm-apis")return "⚠️ Error calling free-llm-apis. Please try again later."
- Add
No behavior change for successful calls; only error text is sanitized for clients.
| @@ -16,11 +16,13 @@ | ||
| import httpx | ||
| import json | ||
| import uuid | ||
| import logging | ||
| from typing import Optional | ||
| from config import settings | ||
|
|
||
| FREE_LLM_BASE = getattr(settings, "FREE_LLM_HOST", "http://localhost:3000") | ||
| MCP_ENDPOINT = f"{FREE_LLM_BASE}/mcp" | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # ─ Structured output prompt ─────────────────────────────────────────────────── | ||
| EDUCATIONAL_SYSTEM = """ | ||
| @@ -140,5 +137,6 @@ | ||
| "cd /home/kali/Desktop/awesome-free-llm-apis/mcp-server\n" | ||
| "node dist/src/server.js --sse\n```" | ||
| ) | ||
| except Exception as e: | ||
| return f"⚠️ Error calling free-llm-apis: {e}" | ||
| except Exception: | ||
| logger.exception("Error calling free-llm-apis") | ||
| return "⚠️ Error calling free-llm-apis. Please try again later." |
| for item in result.get("content", []): | ||
| if item.get("type") == "text": | ||
| result_text += item["text"] | ||
| except json.JSONDecodeError: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix this without changing functionality, replace the empty except json.JSONDecodeError: pass with a non-empty handler that explicitly continues (and ideally documents intent). This preserves current behavior (ignore malformed SSE chunks and keep parsing) while satisfying the static analysis concern and improving maintainability.
Best change in mcp-server/agents/free_llm_client.py:
- In the SSE parsing loop (around lines 119–127), replace:
except json.JSONDecodeError:pass
- With:
except json.JSONDecodeError:# Ignore non-JSON/partial SSE chunks and continue parsing subsequent events.continue
No new imports, methods, or dependencies are required.
| @@ -124,7 +124,8 @@ | ||
| if item.get("type") == "text": | ||
| result_text += item["text"] | ||
| except json.JSONDecodeError: | ||
| pass | ||
| # Ignore non-JSON/partial SSE chunks and continue parsing subsequent events. | ||
| continue | ||
| return result_text or "[No content returned from free-llm-apis]" | ||
| else: | ||
| obj = resp.json() |
| @@ -0,0 +1,51 @@ | |||
| import httpx | |||
| import json | |||
| from typing import List, Dict, Any, Optional, AsyncIterable | |||
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
Remove the unused Optional symbol from the typing import in mcp-server/agents/ollama_client.py while keeping all other imports unchanged.
Specifically:
- Edit line 3 import statement.
- Change:
from typing import List, Dict, Any, Optional, AsyncIterable
- To:
from typing import List, Dict, Any, AsyncIterable
No behavioral or functional code changes are needed.
| @@ -1,6 +1,6 @@ | ||
| import httpx | ||
| import json | ||
| from typing import List, Dict, Any, Optional, AsyncIterable | ||
| from typing import List, Dict, Any, AsyncIterable | ||
| from config import settings | ||
|
|
||
| class OllamaClient: |
| from typing import List, Dict, Any, Optional | ||
| from agents.ollama_client import OllamaClient | ||
| from agents.skill_loader import SkillLoader, SkillDefinition | ||
| import json |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
The correct fix is to remove the unused import json line from mcp-server/agents/orchestrator.py.
Best single change without altering behavior:
- In
mcp-server/agents/orchestrator.py, delete line 4 (import json). - No other code changes are needed.
- No new imports, methods, or definitions are required.
| @@ -1,7 +1,6 @@ | ||
| from typing import List, Dict, Any, Optional | ||
| from agents.ollama_client import OllamaClient | ||
| from agents.skill_loader import SkillLoader, SkillDefinition | ||
| import json | ||
|
|
||
| class ConversationMemory: | ||
| def __init__(self, limit: int = 20): |
| @@ -0,0 +1,54 @@ | |||
| import frontmatter | |||
| from pathlib import Path | |||
| from typing import List, Dict, Any, Optional | |||
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
The fix is to remove only the unused symbol from the import list while preserving all currently used imports and behavior.
In mcp-server/agents/skill_loader.py, update the typing import on line 3:
- Change
from typing import List, Dict, Any, Optional - To
from typing import List, Dict, Optional
This is the minimal and best fix because it resolves the static-analysis error without changing functionality, logic, or dependencies.
| @@ -1,6 +1,6 @@ | ||
| import frontmatter | ||
| from pathlib import Path | ||
| from typing import List, Dict, Any, Optional | ||
| from typing import List, Dict, Optional | ||
| from pydantic import BaseModel | ||
| from config import settings | ||
|
|
| @@ -0,0 +1,53 @@ | |||
| from sqlalchemy import Column, Integer, String, Text, DateTime, ForeignKey | |||
| from sqlalchemy.ext.declarative import declarative_base | |||
| from sqlalchemy.orm import sessionmaker, Relationship | |||
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
Remove the unused Relationship symbol from the SQLAlchemy ORM import list in mcp-server/memory.py.
Best fix without changing behavior:
- Edit the import line at the top of the file:
- Change
from sqlalchemy.orm import sessionmaker, Relationship - To
from sqlalchemy.orm import sessionmaker
- Change
- No other code changes are needed because
Relationshipis not referenced anywhere else in the shown code. - No new imports, methods, or definitions are required.
| @@ -1,6 +1,6 @@ | ||
| from sqlalchemy import Column, Integer, String, Text, DateTime, ForeignKey | ||
| from sqlalchemy.ext.declarative import declarative_base | ||
| from sqlalchemy.orm import sessionmaker, Relationship | ||
| from sqlalchemy.orm import sessionmaker | ||
| from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession | ||
| import datetime | ||
| from config import settings |
| @@ -0,0 +1,104 @@ | |||
| from fastapi import FastAPI, HTTPException, Request | |||
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
The best fix is to narrow the fastapi import to only what is actually used.
In mcp-server/server.py, update line 1 from:
from fastapi import FastAPI, HTTPException, Request
to:
from fastapi import FastAPI
This preserves existing functionality, removes unnecessary dependencies in the import surface, and resolves both alert variants (HTTPException unused and Request unused) with a single change. No additional methods, definitions, or package changes are required.
| @@ -1,4 +1,4 @@ | ||
| from fastapi import FastAPI, HTTPException, Request | ||
| from fastapi import FastAPI | ||
| from fastapi.responses import StreamingResponse | ||
| from pydantic import BaseModel | ||
| from typing import List, Optional |
| from fastapi import FastAPI, HTTPException, Request | ||
| from fastapi.responses import StreamingResponse | ||
| from pydantic import BaseModel | ||
| from typing import List, Optional |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, fix unused-import issues by removing only the unused symbols from import statements (or deleting the whole statement if nothing from it is needed), while leaving runtime behavior unchanged.
Best fix here: in mcp-server/server.py, update line 4 to remove List and Optional from typing imports. Since both are flagged as unused at the same line, remove the full typing import line. No other code changes are required.
| @@ -1,7 +1,6 @@ | ||
| from fastapi import FastAPI, HTTPException, Request | ||
| from fastapi.responses import StreamingResponse | ||
| from pydantic import BaseModel | ||
| from typing import List, Optional | ||
| import uvicorn | ||
| import json | ||
| import asyncio |
| # Call backend | ||
| try: | ||
| import requests | ||
| import json |
Check notice
Code scanning / CodeQL
Module is imported more than once Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix this, remove the duplicate import json from the inner try block and continue using the existing top-level import json (line 18).
Best single fix without changing functionality:
- File:
streamlit/app.py - Region: the backend call section around lines 830–833
- Change: delete only the inner
import json, keepimport requestslocal if desired. - No new methods, definitions, or external imports are needed.
| @@ -829,7 +829,6 @@ | ||
| # Call backend | ||
| try: | ||
| import requests | ||
| import json | ||
|
|
||
| # Check for backend server | ||
| endpoint = "chat/external" if use_frontier else "chat" |
| if not self.page.ele('.ad-showing', timeout=0.5): | ||
| break | ||
| time.sleep(1) | ||
| except: pass |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Replace the empty bare except in skip_youtube_ads() with an except Exception as e that records the failure using logging (or at least a warning print), while preserving current behavior (continue execution).
Best single fix in core/rag.py at the skip_youtube_ads function region:
- Change:
except: pass
- To:
except Exception as e:logging.warning("Ad-skipping routine failed; continuing without ad skip: %s", e)
This keeps the function non-blocking (same functional behavior: it does not raise), but removes the silent failure and satisfies the CodeQL finding.
| @@ -487,7 +487,8 @@ | ||
| if not self.page.ele('.ad-showing', timeout=0.5): | ||
| break | ||
| time.sleep(1) | ||
| except: pass | ||
| except Exception as e: | ||
| logging.warning("Ad-skipping routine failed; continuing without ad skip: %s", e) | ||
|
|
||
| skip_youtube_ads() | ||
|
|
| try: | ||
| video.hover() | ||
| self.page.actions.type('c') | ||
| except: pass |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use an explicit exception type instead of a bare except:.
Best fix (without changing functionality): replace except: pass with except Exception: pass in core/rag.py at the shown block around lines 580–583. This keeps the “ignore normal runtime errors” behavior for flaky browser actions while no longer swallowing interpreter-exit and keyboard-interrupt exceptions.
No new imports, methods, or definitions are required.
| @@ -580,7 +580,8 @@ | ||
| try: | ||
| video.hover() | ||
| self.page.actions.type('c') | ||
| except: pass | ||
| except Exception: | ||
| pass | ||
|
|
||
| self.page.listen.stop() | ||
| self.page.quit() |
| try: | ||
| video.hover() | ||
| self.page.actions.type('c') | ||
| except: pass |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The best fix is to replace the empty bare except: with a non-empty handler that preserves behavior (continue loop) while recording diagnostic context. Since logging is already imported in this file, we can log the exception and keep execution unchanged.
In core/rag.py, around lines 580–583 inside the while loop:
- Change
except: passtoexcept Exception as e: .... - Add a short explanatory comment and a
logging.debug(...)call (withexc_info=True) so transient UI interaction failures are visible in logs without interrupting flow.
No new imports or dependencies are required.
| @@ -580,7 +580,9 @@ | ||
| try: | ||
| video.hover() | ||
| self.page.actions.type('c') | ||
| except: pass | ||
| except Exception as e: | ||
| # Best-effort UI interaction: continue loop, but keep diagnostics. | ||
| logging.debug("Failed to hover/toggle CC during subtitle capture: %s", e, exc_info=True) | ||
|
|
||
| self.page.listen.stop() | ||
| self.page.quit() |
|
|
||
| if found_data: | ||
| # Save to data/cache | ||
| import json |
Check notice
Code scanning / CodeQL
Module is imported more than once Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The fix is to remove the duplicate local import (import json) inside fetch_captions and continue using the already-imported module-level json.
Best single fix (no functional change):
- Edit
core/rag.pyin theif found_data:block around lines 589–594. - Delete the line
import json(current line 590). - Keep
json.dump(...)unchanged; it will resolve to the top-level import from line 7.
No new methods, definitions, or external imports are needed.
| @@ -587,7 +587,6 @@ | ||
|
|
||
| if found_data: | ||
| # Save to data/cache | ||
| import json | ||
| temp_json = os.path.join("data", "cache", f"yt_subs_{video_id}.json") | ||
| with open(temp_json, "w", encoding="utf-8") as f: | ||
| if isinstance(found_data, dict): |
| if hasattr(self, 'page') and self.page: | ||
| try: | ||
| self.page.quit() | ||
| except: |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use an explicit exception type in the inner cleanup handler instead of a bare except:.
Best fix without changing functionality:
- In
core/rag.py, insideYouTubeSearcher.fetch_captions, change:except:toexcept Exception:
- Keep behavior identical (
pass) so cleanup remains best-effort, while allowingKeyboardInterruptandSystemExitto propagate correctly.
No new imports, methods, or definitions are required.
| @@ -608,7 +608,7 @@ | ||
| if hasattr(self, 'page') and self.page: | ||
| try: | ||
| self.page.quit() | ||
| except: | ||
| except Exception: | ||
| pass | ||
| return None | ||
|
|
| if hasattr(self, 'page') and self.page: | ||
| try: | ||
| self.page.quit() | ||
| except: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use a targeted exception handler and log the cleanup failure instead of silently ignoring it.
Best fix in this snippet (without changing behavior):
- In
core/rag.py, insideYouTubeSearcher.fetch_captions, replace the inner bareexcept:aroundself.page.quit()withexcept Exception as quit_error:and print/log a warning message. - Keep returning
Noneexactly as before so external behavior remains unchanged. - No new dependencies are required; existing
printstyle is already used in this method.
| @@ -608,8 +608,8 @@ | ||
| if hasattr(self, 'page') and self.page: | ||
| try: | ||
| self.page.quit() | ||
| except: | ||
| pass | ||
| except Exception as quit_error: | ||
| print(f"[WARN] Failed to quit browser page during error cleanup: {quit_error}") | ||
| return None | ||
|
|
||
| if json_path: |
| import re | ||
| import time | ||
| from pathlib import Path | ||
| from typing import Optional, Dict, Any, List, Literal |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Remove the unused Any symbol from the typing import in core/skill_generator.py while preserving all other imports and behavior.
Best fix (non-functional change only):
- In
core/skill_generator.py, update line 8:- From:
from typing import Optional, Dict, Any, List, Literal - To:
from typing import Optional, Dict, List, Literal
- From:
- No other code changes are required since no usage of
Anyis shown.
| @@ -5,7 +5,7 @@ | ||
| import re | ||
| import time | ||
| from pathlib import Path | ||
| from typing import Optional, Dict, Any, List, Literal | ||
| from typing import Optional, Dict, List, Literal | ||
| from pydantic import BaseModel, Field | ||
|
|
||
| import hashlib |
| if video: | ||
| video.click() | ||
| page.actions.type('c') | ||
| except: |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this safely without changing functionality, replace the bare except: with except Exception: in core/rag.py inside YouTubeSearcher._wait_for_subs, in the retry block around video.click() and page.actions.type('c').
Best single fix:
- Keep the current behavior of silently ignoring non-fatal automation errors.
- Narrow exception handling to
ExceptionsoKeyboardInterruptandSystemExitstill propagate.
No new imports, methods, or dependencies are required.
| @@ -318,7 +318,7 @@ | ||
| if video: | ||
| video.click() | ||
| page.actions.type('c') | ||
| except: | ||
| except Exception: | ||
| pass | ||
|
|
||
| # Wait for the listener to catch something |
| if video: | ||
| video.click() | ||
| page.actions.type('c') | ||
| except: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this without changing functionality, keep the retry flow intact but replace the empty bare except: with a specific exception capture and lightweight logging.
Best change in core/rag.py around lines 316–323:
- Replace:
except:pass
- With:
except Exception as e:- a debug (or warning) log message explaining that CC toggle failed on this retry and execution continues.
This preserves behavior (continue retry loop) while eliminating silent exception swallowing and providing diagnostics. No new imports are needed because logging is already imported.
| @@ -318,8 +318,8 @@ | ||
| if video: | ||
| video.click() | ||
| page.actions.type('c') | ||
| except: | ||
| pass | ||
| except Exception as e: | ||
| logging.debug(f"Failed to toggle CC on retry {retries}: {e}") | ||
|
|
||
| # Wait for the listener to catch something | ||
| res = page.listen.wait(timeout=5) |
| if not self.page.ele('.ad-showing', timeout=0.5): | ||
| break | ||
| time.sleep(1) | ||
| except: pass |
Check notice
Code scanning / CodeQL
Except block handles 'BaseException' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
Use an explicit except Exception instead of bare except: in core/rag.py inside skip_youtube_ads(). This keeps existing functionality (ignore normal runtime errors in ad-skipping logic) while no longer intercepting KeyboardInterrupt/SystemExit.
Exact change:
- File:
core/rag.py - Region:
skip_youtube_ads()in the shown snippet, line withexcept: pass(around line 490) - Replace with
except Exception: pass(or optionally log the error; to avoid behavior change, keep silent pass)
No imports or new dependencies are required.
| @@ -487,7 +487,8 @@ | ||
| if not self.page.ele('.ad-showing', timeout=0.5): | ||
| break | ||
| time.sleep(1) | ||
| except: pass | ||
| except Exception: | ||
| pass | ||
|
|
||
| skip_youtube_ads() | ||
|
|
| import json | ||
| import uuid | ||
| from typing import Optional | ||
| from config import settings |
There was a problem hiding this comment.
CRITICAL: Missing Content-Type: application/json header for JSON-RPC MCP requests. Add "Content-Type": "application/json" to headers dict.
| import json | ||
| import uuid | ||
| from typing import Optional | ||
| from config import settings |
There was a problem hiding this comment.
[CRITICAL] Missing Content-Type: application/json header for JSON-RPC MCP requests. Add "Content-Type": "application/json" to headers dict.
| ... | ||
| ``` | ||
|
|
||
| IMPORTANT: Always include all three sections, even for simple topics. |
There was a problem hiding this comment.
[WARNING] Accept: application/json, text/event-stream conflicts with StreamableHTTP transport. Use Accept: text/event-stream only.
| "model": model, | ||
| "prompt": text | ||
| } | ||
| response = await self.client.post("/api/embeddings", json=payload) |
There was a problem hiding this comment.
[WARNING] _stream_chat yields content before validating response status. If HTTP error occurs, JSON parsing fails silently. Check response.status_code before iterating.
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) | ||
|
|
||
| prompt = f""" |
There was a problem hiding this comment.
[CRITICAL] from langchain_huggingface.embeddings import HuggingFaceEmbeddings (line 34) is unconditional. If langchain-huggingface not installed, module import fails. Wrap the import in try/except to gracefully fallback.
|
|
||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) | ||
|
|
There was a problem hiding this comment.
[WARNING] Hardcoded Gemini model string "models/gemini-2.5-flash" (line 680/685) and two more below. Should use config. Also line 34 unconditional import of langchain_huggingface will crash if not installed.
|
|
||
| # 5. If practical, create scripts placeholder | ||
| if metadata.get("type") == "practical": | ||
| (target_dir / "scripts").mkdir(exist_ok=True) |
There was a problem hiding this comment.
[WARNING] target_dir.mkdir(parents=True, exist_ok=True) followed by write_text without error handling. Permission/disk errors fail silently. Consider try/except around file writes.
| if "mermaid" in md_file.name: | ||
| continue | ||
| # Check cache before calling generate (which also checks cache but we want to track it for stats) | ||
| content = md_file.read_text(encoding="utf-8") |
There was a problem hiding this comment.
[WARNING] md_file.read_text() called twice (here and inside generate_skill_from_markdown line 78). Redundant I/O and could fail between reads. Read once and pass content.
| self.llm = ChatGoogleGenerativeAI( | ||
| model="gemini-2.5-flash", | ||
| temperature=0.2, # Deterministic for extraction | ||
| model="gemini-3.1-flash-lite-preview", |
There was a problem hiding this comment.
[WARNING] Hardcoded Gemini model "gemini-3.1-flash-lite-preview" should use config setting.
|
|
||
| def __init__(self, gemini_processor: Optional[GeminiProcessor] = None): | ||
| # Use flash-lite for bulk skill generation: 1500 req/day vs 20 req/day for flash | ||
| self.processor = gemini_processor or GeminiProcessor(model_name="gemini-3.1-flash-lite-preview") #"gemini-2.5-flash-lite") |
There was a problem hiding this comment.
[WARNING] Hardcoded Gemini model "gemini-3.1-flash-lite-preview" should use config setting.
| self.llm = ChatGoogleGenerativeAI( | ||
| model="gemini-2.5-flash", | ||
| temperature=0.2, # Deterministic for extraction | ||
| model="gemini-3.1-flash-lite-preview", |
There was a problem hiding this comment.
[WARNING] Hardcoded Gemini model "gemini-3.1-flash-lite-preview" (line 28 of exam_analysis.py) should use config setting. Also self.bulk_llm uses same hardcoded model (line 34). DefaultModel in gemini_processor.py also hardcodes it. Better to centralize model names in config.
|
|
||
| def __init__(self, gemini_processor: Optional[GeminiProcessor] = None): | ||
| # Use flash-lite for bulk skill generation: 1500 req/day vs 20 req/day for flash | ||
| self.processor = gemini_processor or GeminiProcessor(model_name="gemini-3.1-flash-lite-preview") #"gemini-2.5-flash-lite") |
There was a problem hiding this comment.
[WARNING] Hardcoded Gemini model "gemini-3.1-flash-lite-preview" in skill_generator.py line 37 (and DEFAULT_MODEL in gemini_processor.py) should use config setting for consistency.
| @@ -13,6 +13,8 @@ | |||
| from icecream import ic | |||
| ic.disable() | |||
| from core.models import ExamPattern, AnalyzedQuestion, QuestionBank | |||
| # 3. Initialize Chat Model with Gemini 2.5 Flash | ||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=1.0) | ||
| # 3. Initialize Chat Model | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=1.0) |
|
|
||
| # 3. Generate Questions | ||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) |
| transcript = f.read() | ||
|
|
||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) |
| logger.warning(f"Google Generative AI Embeddings not available: {e}. Falling back to HuggingFace embeddings.") | ||
| # Choose one of the recommended models | ||
| # Check if we can use local embeddings (may be blocked by system policy) | ||
| from langchain_huggingface import HuggingFaceEmbeddings |
There was a problem hiding this comment.
CRITICAL: Unconditional import of - module import fails if not installed. Wrap in try/except with fallback or informative error.
| @@ -13,6 +13,8 @@ | |||
| from icecream import ic | |||
| ic.disable() | |||
| from core.models import ExamPattern, AnalyzedQuestion, QuestionBank | |||
There was a problem hiding this comment.
CRITICAL: Missing import time but time.sleep(3) is used on line 178. Add import time to fix NameError.
| logger.warning(f"Google Generative AI Embeddings not available: {e}. Falling back to HuggingFace embeddings.") | ||
| # Choose one of the recommended models | ||
| # Check if we can use local embeddings (may be blocked by system policy) | ||
| from langchain_huggingface import HuggingFaceEmbeddings |
There was a problem hiding this comment.
CRITICAL: Unconditional import of langchain_huggingface - module import fails if not installed. Wrap in try/except with fallback or informative error.
| # 3. Initialize Chat Model with Gemini 2.5 Flash | ||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=1.0) | ||
| # 3. Initialize Chat Model | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=1.0) |
There was a problem hiding this comment.
CRITICAL: Hardcoded Gemini model "models/gemini-2.5-flash". Should use configurable constant like GeminiProcessor.DEFAULT_MODEL or config value.
|
|
||
| # 3. Generate Questions | ||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) |
There was a problem hiding this comment.
CRITICAL: Hardcoded Gemini model in generate_quiz_from_video. Should use configurable constant.
| transcript = f.read() | ||
|
|
||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) |
There was a problem hiding this comment.
CRITICAL: Hardcoded Gemini model in analyze_video_structure. Should use configurable constant.
|
|
||
| headers = { | ||
| "Content-Type": "application/json", | ||
| "Accept": "application/json, text/event-stream", |
There was a problem hiding this comment.
WARNING: Conflicting Accept header with both application/json and text/event-stream may break StreamableHTTP transport. Use separate headers for init vs streaming calls.
| return response.json() | ||
|
|
||
| async def _stream_chat(self, payload: Dict[str, Any]) -> AsyncIterable[str]: | ||
| async with self.client.stream("POST", "/api/chat", json=payload) as response: |
There was a problem hiding this comment.
WARNING: _stream_chat yields content before validating HTTP response status. Non-200 responses cause silent JSON parse failures. Check status with response.raise_for_status() before iterating.
| if "mermaid" in md_file.name: | ||
| continue | ||
| # Check cache before calling generate (which also checks cache but we want to track it for stats) | ||
| content = md_file.read_text(encoding="utf-8") |
There was a problem hiding this comment.
WARNING: Redundant file read - md_file.read_text() called twice (once here, once in generate_skill_from_markdown). Cache the content or pass it as parameter.
| from icecream import ic | ||
| ic.disable() | ||
| from core.models import ExamPattern, AnalyzedQuestion, QuestionBank | ||
| from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception_type, before_sleep_log, wait_random_exponential |
| from icecream import ic | ||
| ic.disable() | ||
| from core.models import ExamPattern, AnalyzedQuestion, QuestionBank | ||
| from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception_type, before_sleep_log, wait_random_exponential |
There was a problem hiding this comment.
CRITICAL: Missing import time but time.sleep(3) is used on line 178. Add import time to fix NameError.
| # Choose one of the recommended models | ||
| # Check if we can use local embeddings (may be blocked by system policy) | ||
| from langchain_huggingface import HuggingFaceEmbeddings | ||
| embeddings = HuggingFaceEmbeddings( |
There was a problem hiding this comment.
CRITICAL: Unconditional import of langchain_huggingface - module import fails if not installed. Wrap in try/except with fallback or informative error.
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=1.0) | ||
| # 3. Initialize Chat Model | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=1.0) | ||
|
|
There was a problem hiding this comment.
CRITICAL: Hardcoded Gemini model "models/gemini-2.5-flash". Should use configurable constant like GeminiProcessor.DEFAULT_MODEL or config value.
| # 3. Generate Questions | ||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) | ||
|
|
There was a problem hiding this comment.
CRITICAL: Hardcoded Gemini model in generate_quiz_from_video. Should use configurable constant.
|
|
||
| llm = ChatGoogleGenerativeAI(model="gemini-2.5-flash", temperature=0.7) | ||
| llm = ChatGoogleGenerativeAI(model="models/gemini-2.5-flash", temperature=0.7) | ||
|
|
There was a problem hiding this comment.
CRITICAL: Hardcoded Gemini model in analyze_video_structure. Should use configurable constant.
| headers = { | ||
| "Content-Type": "application/json", | ||
| "Accept": "application/json, text/event-stream", | ||
| } |
There was a problem hiding this comment.
WARNING: Conflicting Accept header with both application/json and text/event-stream may break StreamableHTTP transport. Use separate headers for init vs streaming calls.
|
|
||
| async def _stream_chat(self, payload: Dict[str, Any]) -> AsyncIterable[str]: | ||
| async with self.client.stream("POST", "/api/chat", json=payload) as response: | ||
| async for line in response.aiter_lines(): |
There was a problem hiding this comment.
WARNING: _stream_chat yields content before validating HTTP response status. Non-200 responses cause silent JSON parse failures. Check status with response.raise_for_status() before iterating.
| continue | ||
| # Check cache before calling generate (which also checks cache but we want to track it for stats) | ||
| content = md_file.read_text(encoding="utf-8") | ||
| content_hash = self._calculate_hash(content) |
There was a problem hiding this comment.
WARNING: Redundant file read - md_file.read_text() called twice (once here, once in generate_skill_from_markdown). Cache the content or pass it as parameter.
This pull request introduces several significant enhancements and new features to the AI learning system, most notably agentic skills, robust input validation, and new modules for cheatsheet and diagram generation. The updates improve both the system's security and its educational capabilities, while also expanding programmatic and CLI interfaces for advanced users.
Key new features and improvements:
Agentic Skills and Semantic Query Classification
concept-mastery,study-planner,rag-optimizer) with a registry for programmatic discovery, invocation, and semantic query classification, allowing the system to intelligently route and process user queries based on intent.classify_query_semantics) that determines query type and aggregation strategy, improving retrieval and answer quality. [1] [2] [3]Security and Input Validation
bleachandmarkupsafefor all user inputs before LLM processing, enhancing security against injection attacks. [1] [2] [3] [4]Cheatsheet and Diagram Generation
core/cheatsheet.py, a new module for generating concise revision cheatsheets from markdown notes using Gemini, with intelligent chunking to avoid token limits and support for both module- and topic-level summaries.core/diagram_generator.py, enabling conceptual Mermaid diagram generation for topics via Gemini and LangChain, with robust error handling and caching for efficiency and reliability.Persistent LLM Caching
Ollama Local MCP server (In Devolopment)
Documentation and CLI Enhancements
README.mdwith detailed documentation for new agentic skills, semantic query classification, CLI demos, and a new section for young learners. [1] [2]These changes collectively make the system more secure, modular, and powerful for both end-users and developers.