feat: add intent/proposals v1 subjects and schemas#60
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Comment |
Reviewer's GuideIntroduces versioned QIKI intent and proposal subjects plus strict Pydantic v1 schemas for intents and proposals, along with tests covering validation, versioning, and JSON round-trips. Class diagram for QIKI intent and proposal schemas v1classDiagram
class _StrictModel {
<<pydantic.BaseModel>>
+ConfigDict model_config
}
class LangHint {
<<enum>>
+AUTO
+RU
+EN
}
class EnvironmentMode {
<<enum>>
+FACTORY
+MISSION
}
class SelectionV1 {
<<model>>
+Literal kind
+str id
}
class IntentV1 {
<<model>>
+Literal version
+str text
+LangHint lang_hint
+str screen
+SelectionV1 selection
+int ts
+EnvironmentMode environment_mode
+dict~str, Any~ snapshot_min
}
class ProposalV1 {
<<model>>
+str proposal_id
+str title
+str justification
+int priority
+float confidence
+list~Any~ proposed_actions
+list~Any~ _must_be_empty_in_stage_a(v)
}
class ProposalsBatchV1 {
<<model>>
+Literal version
+int ts
+list~ProposalV1~ proposals
+dict~str, Any~ metadata
}
_StrictModel <|-- SelectionV1
_StrictModel <|-- IntentV1
_StrictModel <|-- ProposalV1
_StrictModel <|-- ProposalsBatchV1
LangHint <.. IntentV1 : uses
EnvironmentMode <.. IntentV1 : uses
SelectionV1 --> IntentV1 : selection
ProposalV1 --> ProposalsBatchV1 : element of proposals
Flow diagram for QIKI intent and proposal NATS subjects v1flowchart LR
subgraph Versioned_subjects_v1
QIKI_INTENT_V1[QIKI_INTENT_V1\nqiki.intent.v1]
QIKI_PROPOSALS_V1[QIKI_PROPOSALS_V1\nqiki.proposals.v1]
end
QIKI_INTENTS[QIKI_INTENTS\nbackward-compat alias]
QIKI_INTENTS --> QIKI_INTENT_V1
IntentV1_model[IntentV1 schema]
ProposalsBatchV1_model[ProposalsBatchV1 schema]
IntentV1_model --> QIKI_INTENT_V1
ProposalsBatchV1_model --> QIKI_PROPOSALS_V1
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider adding a validator to SelectionV1 to enforce that id is present for all kinds except 'none' (and possibly absent for 'none'), so selection consistency is guaranteed at the schema level.
- The ts fields in IntentV1 and ProposalsBatchV1 are plain ints; if they are always milliseconds since epoch, consider encoding that in the name (e.g., ts_ms) or via a type alias to avoid ambiguity and accidental misuse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a validator to SelectionV1 to enforce that id is present for all kinds except 'none' (and possibly absent for 'none'), so selection consistency is guaranteed at the schema level.
- The ts fields in IntentV1 and ProposalsBatchV1 are plain ints; if they are always milliseconds since epoch, consider encoding that in the name (e.g., ts_ms) or via a type alias to avoid ambiguity and accidental misuse.
## Individual Comments
### Comment 1
<location> `tests/unit/test_orion_qiki_protocol_v1.py:8-14` </location>
<code_context>
+import pytest
+from pydantic import ValidationError
+
+from qiki.shared.models.orion_qiki_protocol import (
+ EnvironmentMode,
+ IntentV1,
+ LangHint,
+ ProposalV1,
+ ProposalsBatchV1,
+ SelectionV1,
+)
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a small test to assert the QIKI_INTENTS backward‑compat alias equals QIKI_INTENT_V1
Since this PR introduces `QIKI_INTENT_V1` while keeping `QIKI_INTENTS` as a backwards-compat alias, please add a small test (here or in `test_nats_subjects.py`) asserting `QIKI_INTENTS == QIKI_INTENT_V1` to guard against them drifting apart in future changes.
Suggested implementation:
```python
from qiki.shared.models.orion_qiki_protocol import (
EnvironmentMode,
IntentV1,
LangHint,
ProposalV1,
QIKI_INTENT_V1,
QIKI_INTENTS,
)
def test_qiki_intents_backward_compat_alias():
assert QIKI_INTENTS == QIKI_INTENT_V1
```
If this file already imports additional symbols (e.g. `ProposalsBatchV1`, `SelectionV1`) in the same tuple, you should merge `QIKI_INTENT_V1` and `QIKI_INTENTS` into that existing import list rather than creating duplicates. Place the `test_qiki_intents_backward_compat_alias` test alongside other simple constant/alias tests if there is an existing section for those, keeping naming consistent with the surrounding tests.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| from qiki.shared.models.orion_qiki_protocol import ( | ||
| EnvironmentMode, | ||
| IntentV1, | ||
| LangHint, | ||
| ProposalV1, | ||
| ProposalsBatchV1, | ||
| SelectionV1, |
There was a problem hiding this comment.
suggestion (testing): Add a small test to assert the QIKI_INTENTS backward‑compat alias equals QIKI_INTENT_V1
Since this PR introduces QIKI_INTENT_V1 while keeping QIKI_INTENTS as a backwards-compat alias, please add a small test (here or in test_nats_subjects.py) asserting QIKI_INTENTS == QIKI_INTENT_V1 to guard against them drifting apart in future changes.
Suggested implementation:
from qiki.shared.models.orion_qiki_protocol import (
EnvironmentMode,
IntentV1,
LangHint,
ProposalV1,
QIKI_INTENT_V1,
QIKI_INTENTS,
)
def test_qiki_intents_backward_compat_alias():
assert QIKI_INTENTS == QIKI_INTENT_V1If this file already imports additional symbols (e.g. ProposalsBatchV1, SelectionV1) in the same tuple, you should merge QIKI_INTENT_V1 and QIKI_INTENTS into that existing import list rather than creating duplicates. Place the test_qiki_intents_backward_compat_alias test alongside other simple constant/alias tests if there is an existing section for those, keeping naming consistent with the surrounding tests.
![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: 55efea123e
ℹ️ 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".
| QIKI_INTENT_V1 = "qiki.intent.v1" | ||
| QIKI_PROPOSALS_V1 = "qiki.proposals.v1" | ||
|
|
||
| # Backward-compat alias: prefer QIKI_INTENT_V1. | ||
| QIKI_INTENTS = QIKI_INTENT_V1 |
There was a problem hiding this comment.
By aliasing QIKI_INTENTS to qiki.intent.v1, existing publishers (e.g., the operator console still sends {text, source, ts_epoch}) will now emit legacy payloads on the v1 subject, which consumers validating IntentV1 will reject, and any legacy subscribers still listening to qiki.intents will stop receiving messages. Consider keeping QIKI_INTENTS as the old subject until the publisher is updated to emit the v1 schema (or keep a separate legacy constant).
Useful? React with 👍 / 👎.
1 participant
This PR introduces QIKI schemas subjects and proposals. Part of Stage 0 plan.
Summary by Sourcery
Introduce v1 QIKI intent and proposal protocol schemas and wire them into messaging subjects.
New Features:
Tests: