Fix chat runtime option matching and add direct service calls#577
Fix chat runtime option matching and add direct service calls#577
Conversation
hardbyte
commented
Feb 6, 2026
hardbyte
commented
- Store inline question options in _current_options (not just CMS-sourced), fixing cascading failures where age/reading answers stored as raw strings instead of full option dicts with typed fields like age_number
- Add internal API handler registry for direct service-layer calls, bypassing HTTP auth for anonymous chatbot sessions (e.g. /v1/recommend)
- Fix broken import in _find_matching_connection (app.crud.chat → chat_repo)
- Resolve school name server-side from school_wriveted_id during session start
- Add CEL functions for hue profile aggregation (merge, top_keys)
- Expand seed fixtures with book catalog, themes, and flow_file loading
- Store inline question options in _current_options (not just CMS-sourced), fixing cascading failures where age/reading answers stored as raw strings instead of full option dicts with typed fields like age_number - Add internal API handler registry for direct service-layer calls, bypassing HTTP auth for anonymous chatbot sessions (e.g. /v1/recommend) - Fix broken import in _find_matching_connection (app.crud.chat → chat_repo) - Resolve school name server-side from school_wriveted_id during session start - Add CEL functions for hue profile aggregation (merge, top_keys) - Expand seed fixtures with book catalog, themes, and flow_file loading
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96b19bfed0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR improves the Huey chat runtime’s option handling and introduces a direct “internal handler” pathway for certain service calls (e.g. recommendations) to avoid HTTP/auth overhead in anonymous chat sessions. It also expands seed/demo fixtures to support themes and richer catalog data, plus adds new CEL aggregation helpers.
Changes:
- Fixes question option resolution/matching by persisting the resolved option objects into session state and improving runtime routing behavior.
- Adds an internal handler registry for direct service-layer calls (starting with
/v1/recommend) and improves variable substitution to preserve typed values. - Expands seed fixtures (themes, catalog fields, flow_file loading) and adds a CEL helper
top_keysfor hue profile aggregation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/seed_admin_ui_data.py | Adds theme seeding, flow_file merging, Airtable CSV question seeding, and new labelset fields. |
| scripts/fixtures/admin-ui-seed.json | Expands demo fixtures (hues, reading abilities, works, themes) and references an external flow file. |
| docker-compose.yml | Updates internal API base URL and local CORS origins. |
| app/tests/unit/test_cel_aggregation.py | Updates custom CEL function registry expectations to include top_keys. |
| app/services/variable_resolver.py | Enhances substitution to return raw typed values when the template is a single variable reference. |
| app/services/internal_api_handlers.py | Introduces a registry/decorator and adds a direct handler for /v1/recommend. |
| app/services/chat_runtime.py | Improves inline message rendering, question text substitution, option resolution/storage, and response routing. |
| app/services/cel_evaluator.py | Adds top_keys CEL function and registers it for expression evaluation. |
| app/services/api_client.py | Switches to structlog and makes WRIVETED_INTERNAL_API optional when building base_url. |
| app/services/action_processor.py | Adds direct internal handler execution path for internal api_call actions and improves nested updates. |
| app/repositories/cms_repository.py | Adjusts JSONB filter casting in random content query. |
| app/api/chat.py | Resolves school name server-side at session start using school_wriveted_id. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.base_url = ( | ||
| str(self.settings.WRIVETED_INTERNAL_API) | ||
| if self.settings.WRIVETED_INTERNAL_API | ||
| else None | ||
| ) |
There was a problem hiding this comment.
InternalApiClient now allows base_url to be None, but initialize() still passes self.base_url into httpx.AsyncClient(base_url=...). If WRIVETED_INTERNAL_API is unset, this will fail at runtime with a low-signal error. Consider validating in init/initialize and raising a clear exception (or providing a safe default) when base_url is missing.
| "merge_last", | ||
| "flatten", | ||
| "collect", | ||
| "top_keys", | ||
| } | ||
| assert set(CUSTOM_CEL_FUNCTIONS.keys()) == expected_functions |
There was a problem hiding this comment.
The test suite asserts top_keys is registered, but there are no unit tests validating top_keys behavior (ordering, non-numeric values, n parameter, etc.). Adding a focused test would prevent regressions in CEL hue-profile aggregation logic.
| # Normalize CMS options: ensure label/value fields exist | ||
| if options: | ||
| for opt in options: | ||
| if "label" not in opt and "text" in opt: | ||
| opt["label"] = opt["text"] | ||
| if "value" not in opt and "text" in opt: | ||
| opt["value"] = opt["text"] |
There was a problem hiding this comment.
The options normalization loop assumes each option is a dict and mutates it in-place. Options are sometimes a list of strings (e.g. in integration tests), which can raise at runtime, and both FlowNode.content and CMSContent.content are MutableDict-backed JSONB—mutating these objects during request handling can be flushed on the next commit (e.g. when updating session state), unintentionally persisting changes to flows/CMS content. Consider building a new normalized options list (copy) and handling string options by mapping them to {label,value} without mutating the underlying ORM JSON fields.
| # Normalize CMS options: ensure label/value fields exist | |
| if options: | |
| for opt in options: | |
| if "label" not in opt and "text" in opt: | |
| opt["label"] = opt["text"] | |
| if "value" not in opt and "text" in opt: | |
| opt["value"] = opt["text"] | |
| # Normalize CMS options: ensure label/value fields exist without mutating | |
| # underlying ORM-backed JSON structures. Also support string options by | |
| # mapping them to {label, value}. | |
| if options: | |
| normalized_options: List[Any] = [] | |
| for opt in options: | |
| if isinstance(opt, dict): | |
| norm_opt = dict(opt) | |
| text_value = norm_opt.get("text") | |
| if "label" not in norm_opt and text_value is not None: | |
| norm_opt["label"] = text_value | |
| if "value" not in norm_opt and text_value is not None: | |
| norm_opt["value"] = text_value | |
| normalized_options.append(norm_opt) | |
| elif isinstance(opt, str): | |
| normalized_options.append({"label": opt, "value": opt}) | |
| else: | |
| # Preserve any unexpected option types as-is. | |
| normalized_options.append(opt) | |
| options = normalized_options |
app/services/chat_runtime.py
Outdated
| # Persist available options so process_response() can match full objects | ||
| options_to_store = ( | ||
| result.get("input_request", {}).get("options", []) | ||
| if awaiting_input | ||
| else [] | ||
| ) | ||
| state_updates = ( | ||
| {"system": {"_current_options": options_to_store}} | ||
| if options_to_store | ||
| else {} | ||
| ) |
There was a problem hiding this comment.
_current_options is only written when options_to_store is non-empty; when the next input request has no options, the previous _current_options remains in session state and can be incorrectly reused for later choice questions. Consider explicitly clearing system._current_options (e.g. set to []) when awaiting_input is false or when the next question has no options.
| # Persist available options so process_response() can match full objects | |
| options_to_store = ( | |
| result.get("input_request", {}).get("options", []) | |
| if awaiting_input | |
| else [] | |
| ) | |
| state_updates = ( | |
| {"system": {"_current_options": options_to_store}} | |
| if options_to_store | |
| else {} | |
| ) | |
| # Persist available options so process_response() can match full objects. | |
| # Always update _current_options, clearing it to [] when not awaiting input | |
| # or when there are no options for the current question. | |
| options_to_store = ( | |
| result.get("input_request", {}).get("options", []) | |
| if awaiting_input | |
| else [] | |
| ) | |
| state_updates = {"system": {"_current_options": options_to_store}} |
| db=db, wriveted_id=data.wriveted_identifier | ||
| ) | ||
|
|
||
| limit = int(query_params.get("limit", 5)) |
There was a problem hiding this comment.
Parsing limit with int(query_params.get('limit', 5)) will raise ValueError/TypeError for unexpected inputs, which will fail the whole action node. Consider validating/coercing safely (defaulting on errors) and optionally clamping to a max to avoid expensive requests.
| limit = int(query_params.get("limit", 5)) | |
| raw_limit = query_params.get("limit", 5) | |
| try: | |
| limit = int(raw_limit) | |
| except (TypeError, ValueError): | |
| logger.warning( | |
| "Invalid 'limit' query parameter for /v1/recommend; defaulting to 5", | |
| limit=raw_limit, | |
| ) | |
| limit = 5 |
docker-compose.yml
Outdated
| - WRIVETED_INTERNAL_API=http://api:8000 | ||
| - SQLALCHEMY_WARN_20=true | ||
| - OPENAI_API_KEY=unused-test-key-for-testing | ||
| - DEBUG=true | ||
| - BACKEND_CORS_ORIGINS=["http://localhost:3000","http://localhost:3005","http://localhost:3006","http://localhost:3007","http://localhost:3008","http://localhost:3009","http://localhost:3010","http://localhost:8080","http://localhost:8000","http://127.0.0.1:8000"] | ||
| - BACKEND_CORS_ORIGINS=["http://localhost:3000","http://localhost:3001","http://localhost:3005","http://localhost:3006","http://localhost:3007","http://localhost:3008","http://localhost:3009","http://localhost:3010","http://localhost:8080","http://localhost:8000","http://127.0.0.1:8000"] |
There was a problem hiding this comment.
WRIVETED_INTERNAL_API is used by CloudTasksService to build URLs like ${WRIVETED_INTERNAL_API}/internal/tasks/..., which are only mounted on the separate internal app (app/internal_api.py). Pointing this env var to the public api:8000 service will break async task processing because those routes aren't included in app/main.py. Either keep this pointing at internal:8888 in compose, or expose/mount the internal task routes on the main API and/or introduce a separate setting for public API calls.
…tures - Fix repository path reference (app/crud/chat_repo.py → app/repositories/) - Document direct service call architecture for anonymous chatbot sessions - Document choice option matching behavior (full option objects with typed fields) - Add top_keys CEL function to aggregation docs - Fix user scope as read/write (writable by action nodes) - Document flow_file key for external flow JSON in seed fixtures - Document server-side school name resolution and theme loading in /start
- _load_flow_config returns None with warning instead of raising FileNotFoundError when referenced flow JSON is absent - _ensure_theme uses (name, school_id) composite filter with .first() to avoid MultipleResultsFound and remove unused seed_key branch
substitute_object returns raw typed values for single {{var}} refs
(int 30, bool True) rather than stringified versions.
Covers ranking, n-parameter limiting, non-numeric value filtering, empty/non-dict inputs, and the full merge-then-rank pipeline used in Huey hue profile aggregation.
- Safely parse and clamp limit param in recommend handler (1-50) - Point api service WRIVETED_INTERNAL_API at internal:8888, not itself - Always write _current_options (even empty) to clear stale options from previous questions, preventing incorrect option reuse
