[sdk/python] Add FlameSessionContext for custom session IDs (RFE350)#351
Conversation
Introduces FlameSessionContext dataclass that allows users to specify custom session IDs for Runner.service(). The context can be attached to execution objects (classes, instances, or functions) via the _flame_session_context attribute. Features: - FlameSessionContext with session_id and application_name fields - Validation for session_id (type, length, non-empty) - Detection logic in RunnerService.__init__() - Backward compatible - auto-generates session ID if not specified Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Design document for FlameSessionContext feature that enables custom session IDs in Runner.service(). Includes motivation, function specification, implementation details, and use cases for all supported execution object types. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
E2E tests for FlameSessionContext feature covering: - Custom session ID with class, instance, and function - Session ID auto-generation when not specified - Backward compatibility without context - Invalid context type handling (ignored with warning) - Validation tests (empty, too long, invalid types) - Dynamic class creation with context Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Summary of ChangesHello @k82cn, 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 enhances the Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request introduces FlameSessionContext to allow custom session IDs, which is a great feature for improving traceability and testing. The implementation is solid, with a comprehensive design document and extensive end-to-end tests covering various use cases. The changes are backward compatible and well-integrated into the existing Runner.service() flow. I've added a couple of minor suggestions to improve validation consistency in the new FlameSessionContext and fix a small typo in the design document. Overall, this is an excellent contribution.
| ## 5. References | ||
|
|
||
| ### Related Documents | ||
| - [RFE280 - Simplify the Python API of Flame](../FRE280-runner/RFE280-runner.md) |
There was a problem hiding this comment.
There's a minor typo in the link to the related document. It seems FRE280 should be RFE280 to align with the naming convention of other RFEs.
| - [RFE280 - Simplify the Python API of Flame](../FRE280-runner/RFE280-runner.md) | |
| - [RFE280 - Simplify the Python API of Flame](../RFE280-runner/RFE280-runner.md) |
| if self.application_name is not None and not isinstance(self.application_name, str): | ||
| raise ValueError(f"application_name must be a string, got {type(self.application_name)}") |
There was a problem hiding this comment.
The validation for application_name only checks its type. For consistency with session_id and to prevent potentially undesirable values, it would be beneficial to also validate that application_name is not an empty string and enforce a maximum length. This would make the context object more robust.
if self.application_name is not None:
if not isinstance(self.application_name, str):
raise ValueError(f"application_name must be a string, got {type(self.application_name)}")
if len(self.application_name) == 0:
raise ValueError("application_name cannot be empty string")
if len(self.application_name) > 128:
raise ValueError(f"application_name too long ({len(self.application_name)} chars, max 128)")Rename FlameSessionContext to SessionContext and _flame_session_context to _session_context for consistency and simplicity. No functional changes - pure rename. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Summary
This PR introduces FlameSessionContext, a new feature that allows users to specify custom session IDs when creating services via Runner.service(). This enables better traceability, deterministic testing, and external system integration.
Changes
Testing