fix: pass SessionModel to get_suggestions instead of raw dict#125
Conversation
Fixes RichardAtCT#119 - QuickActionManager.get_suggestions() was being called with session_data=dict but the method signature expects session=SessionModel. Construct a minimal SessionModel from the available handler context (user_id and current_dir) to satisfy the interface correctly.
There was a problem hiding this comment.
Pull request overview
Fixes the /actions command crash by aligning the QuickActionManager.get_suggestions() call site with its actual signature (expects a SessionModel, not a session_data dict).
Changes:
- Replace
session_data=...call withsession=SessionModel(...)when requesting quick action suggestions. - Add required imports (
SessionModel,datetime/timezone) to support session construction.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/bot/handlers/command.py
Outdated
| actions = await quick_action_manager.get_suggestions( | ||
| session_data={"working_directory": str(current_dir), "user_id": user_id} | ||
| session=SessionModel( | ||
| session_id="", | ||
| user_id=user_id, | ||
| project_path=str(current_dir), | ||
| created_at=datetime.now(timezone.utc), | ||
| last_used=datetime.now(timezone.utc), |
There was a problem hiding this comment.
QuickActionManager._analyze_context() accesses session.context, but SessionModel (from src/storage/models.py) doesn't define a context field. As constructed here, this will raise AttributeError inside get_suggestions() and it will fall back to returning [], so /actions will always show “No Actions Available”. Consider attaching a context attribute (e.g., {} or recent message context) to the SessionModel instance before calling get_suggestions, or adjust the quick actions feature to use getattr(session, "context", None) instead of session.context.
src/bot/handlers/command.py
Outdated
| created_at=datetime.now(timezone.utc), | ||
| last_used=datetime.now(timezone.utc), | ||
| ) |
There was a problem hiding this comment.
created_at and last_used are computed with two separate datetime.now(...) calls. It’s clearer (and avoids subtle ordering differences) to compute now once and reuse it for both fields.
src/bot/handlers/command.py
Outdated
|
|
||
|
|
There was a problem hiding this comment.
There’s trailing whitespace on this blank line, which can cause noisy diffs and may fail strict linters. Remove the extra spaces so the line is empty.
Friday's Review 🖤Fix is correct and well-targeted. The root cause — passing a non-existent ObservationsTrailing whitespace (line ~1115): The diff shows a line with only spaces (
now = datetime.now(timezone.utc)
session=SessionModel(
session_id="",
user_id=user_id,
project_path=str(current_dir),
created_at=now,
last_used=now,
)
SummarySolid fix. The two — Friday, AI assistant to @RichardAtCT |
RichardAtCT
left a comment
There was a problem hiding this comment.
LGTM — fixes the SessionModel constructor argument for get_suggestions. Closes #119.
Description
Fixes the
/actionscommand crash reported in #119.QuickActionManager.get_suggestions()expects aSessionModelobject butwas being called with
session_data=dict— a keyword argument that doesn'texist in the method signature, causing a TypeError every time a user ran
/actions.Fix
Construct a minimal
SessionModelfrom available handler context(user_id and current_dir) and pass it correctly as
session=.Related Issue
Fixes #119
Type of Change
Testing
Checklist