Abigail/conclusions level filter#851
Open
ajspig wants to merge 6 commits into
Open
Conversation
The `level` of a conclusion (explicit / deductive / inductive /
contradiction) was filterable server-side but stripped from the
`Conclusion` response and not surfaced in either SDK. This adds it
end-to-end so callers can list explicit-only ("not dreamed on")
conclusions without dropping to raw HTTP.
- api: add `level` to the Conclusion response schema
- python sdk: `ConclusionLevel` type, `level` on Conclusion/response,
`level=` kwarg on ConclusionScope.list() and the async variant
- ts sdk: `ConclusionLevel` type, `level` on Conclusion/response,
`level` option on list()
- tests: assert level is exposed; add level-filter list test
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l= kwarg
Match the documented SDK convention (peers/sessions/messages all take a
generic `filters` dict passed through to the same dynamic server-side
filter logic) instead of a one-off `level=` kwarg. `level` filtering now
works as `list(filters={"level": "explicit"})` alongside any other
supported filter/operator.
The `level` field on the Conclusion response (added in the previous
commit) is kept — it's still not otherwise returned by the API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The branch's level-filter work exposed `filters=` on `list()` but left
`query()` (semantic search) hardcoding `{observer, observed}`, so callers
could filter the list endpoint by reasoning level but not semantic search —
asymmetric in both SDKs.
- Python: add keyword-only `filters` to `ConclusionScope.query` and
`ConclusionScopeAio.query`, merged over the scope's observer/observed.
- TypeScript: add optional `filters` arg to `ConclusionScope.query`,
mirroring the existing `list()` change.
The server `/conclusions/query` endpoint already honors filters in the body
(verified against production), so this is purely SDK surface parity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The using-filters page covered workspaces/peers/sessions/messages but not conclusions. Add a "Filtering Conclusions" section showing level-based filtering on both list() and query(), including the common "explicit only" (exclude dream-derived) case and the in[deductive,inductive] inverse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughConclusion responses now include a typed ChangesConclusion levels and scoped filters
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sdks/python/src/honcho/conclusions.py (1)
202-209: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueCaller
filterscan silently override scope keys.
merged_filters.update(filters)applies caller-providedfilterslast, so a caller passingobserver_id/observed_id/session_idinsidefilterswill overwrite the scope-injected values rather than supplementing them. This mirrors the TypeScript SDK, so it may be intentional, but if scope identity should be authoritative consider seeding caller filters first and re-applying scope keys afterward.♻️ Optional: make scope keys authoritative
- merged_filters: dict[str, Any] = { - "observer_id": self.observer, - "observed_id": self.observed, - } - if resolved_session_id: - merged_filters["session_id"] = resolved_session_id - if filters: - merged_filters.update(filters) + merged_filters: dict[str, Any] = dict(filters) if filters else {} + merged_filters["observer_id"] = self.observer + merged_filters["observed_id"] = self.observed + if resolved_session_id: + merged_filters["session_id"] = resolved_session_id🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/src/honcho/conclusions.py` around lines 202 - 209, The merged filter construction in `Conclusions` currently lets caller-supplied `filters` override scope identity keys like `observer_id`, `observed_id`, and `session_id`. If scope keys should be authoritative, change the merge order in the method building `merged_filters` so caller filters are applied first and then re-apply the scope values afterward, ensuring `self.observer`, `self.observed`, and the resolved session id cannot be overwritten.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sdks/python/src/honcho/conclusions.py`:
- Around line 202-209: The merged filter construction in `Conclusions` currently
lets caller-supplied `filters` override scope identity keys like `observer_id`,
`observed_id`, and `session_id`. If scope keys should be authoritative, change
the merge order in the method building `merged_filters` so caller filters are
applied first and then re-apply the scope values afterward, ensuring
`self.observer`, `self.observed`, and the resolved session id cannot be
overwritten.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed5ed6ea-df1b-45ca-b4a0-8be5485e7bb9
📒 Files selected for processing (9)
docs/v3/documentation/features/advanced/using-filters.mdxsdks/python/src/honcho/aio.pysdks/python/src/honcho/api_types.pysdks/python/src/honcho/conclusions.pysdks/typescript/src/conclusions.tssdks/typescript/src/index.tssdks/typescript/src/types/api.tssrc/schemas/api.pytests/routes/test_conclusions.py
Replace the merged_filters + if-block pattern in list()/query() (py sync, aio, ts) with a single dict spread that layers the caller's filters over the scope's observer/observed (and session). No behavior change — same merge order (caller wins) — just less code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
The generic filters= argument on ConclusionScope.list()/query() spread user-supplied filters last, so a stray observer/observed/session key silently overrode the scope and returned data from a different peer pair. Add a fail-loud guard in both the Python and TypeScript SDKs that rejects scope-managed filter keys with a clear error, directing callers to peer.conclusions / conclusions_of(target) and the session= parameter. session_id remains a valid filter on query() (which has no dedicated session parameter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
level(explicit,deductive,inductive,contradiction).filters(including filtering bylevel), with scope-managed fields protected from being overridden.levelfiltering and providing examples (including querying another peer’s conclusions).leveland added coverage forlevelfiltering and reserved filter-key validation.