refactor: replace /derived endpoint with parent_id list filter
The conclusions/list parent_id filter covers reverse traversal, so the
dedicated GET /conclusions/{id}/derived route is redundant. SDK derived()
helpers (py sync/async, TS) now call list with a parent_id filter — an
unknown parent yields an empty page instead of 404. Adds an id tiebreak
to filtered list ordering so same-batch pagination stays deterministic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
20fddc08f4
commit
538e9af880
|
|
@ -1662,10 +1662,14 @@ class ConclusionScopeAio:
|
|||
size: Number of results per page. Default: 50.
|
||||
reverse: If True, reverses the default newest-first ordering.
|
||||
|
||||
Sugar for ``list`` with a ``parent_id`` filter; an unknown
|
||||
``conclusion_id`` yields an empty page rather than an error.
|
||||
|
||||
Returns:
|
||||
Paginated response containing Conclusion objects
|
||||
"""
|
||||
await self._scope._honcho._ensure_workspace_async()
|
||||
body: dict[str, Any] = {"filters": {"parent_id": conclusion_id}}
|
||||
|
||||
def build_query(page_num: int) -> dict[str, Any]:
|
||||
query: dict[str, Any] = {"page": page_num, "size": size}
|
||||
|
|
@ -1673,8 +1677,9 @@ class ConclusionScopeAio:
|
|||
query["reverse"] = "true"
|
||||
return query
|
||||
|
||||
data = await self._scope._honcho._async_http_client.get(
|
||||
routes.conclusion_derived(self._scope.workspace_id, conclusion_id),
|
||||
data = await self._scope._honcho._async_http_client.post(
|
||||
routes.conclusions_list(self._scope.workspace_id),
|
||||
body=body,
|
||||
query=build_query(page),
|
||||
)
|
||||
|
||||
|
|
@ -1684,8 +1689,9 @@ class ConclusionScopeAio:
|
|||
async def fetch_next(
|
||||
next_page: int,
|
||||
) -> AsyncPage[ConclusionResponse, Conclusion]:
|
||||
next_data = await self._scope._honcho._async_http_client.get(
|
||||
routes.conclusion_derived(self._scope.workspace_id, conclusion_id),
|
||||
next_data = await self._scope._honcho._async_http_client.post(
|
||||
routes.conclusions_list(self._scope.workspace_id),
|
||||
body=body,
|
||||
query=build_query(next_page),
|
||||
)
|
||||
return AsyncPage(next_data, ConclusionResponse, transform, fetch_next)
|
||||
|
|
|
|||
|
|
@ -397,10 +397,14 @@ class ConclusionScope:
|
|||
size: Number of results per page. Default: 50.
|
||||
reverse: If True, reverses the default newest-first ordering.
|
||||
|
||||
Sugar for ``list`` with a ``parent_id`` filter; an unknown
|
||||
``conclusion_id`` yields an empty page rather than an error.
|
||||
|
||||
Returns:
|
||||
Paginated response containing Conclusion objects
|
||||
"""
|
||||
self._honcho._ensure_workspace()
|
||||
body: dict[str, Any] = {"filters": {"parent_id": conclusion_id}}
|
||||
|
||||
def build_query(page_num: int) -> dict[str, Any]:
|
||||
query: dict[str, Any] = {"page": page_num, "size": size}
|
||||
|
|
@ -408,8 +412,9 @@ class ConclusionScope:
|
|||
query["reverse"] = "true"
|
||||
return query
|
||||
|
||||
data = self._honcho._http.get(
|
||||
routes.conclusion_derived(self.workspace_id, conclusion_id),
|
||||
data = self._honcho._http.post(
|
||||
routes.conclusions_list(self.workspace_id),
|
||||
body=body,
|
||||
query=build_query(page),
|
||||
)
|
||||
|
||||
|
|
@ -419,8 +424,9 @@ class ConclusionScope:
|
|||
def fetch_next(
|
||||
next_page: int,
|
||||
) -> SyncPage[ConclusionResponse, Conclusion]:
|
||||
next_data = self._honcho._http.get(
|
||||
routes.conclusion_derived(self.workspace_id, conclusion_id),
|
||||
next_data = self._honcho._http.post(
|
||||
routes.conclusions_list(self.workspace_id),
|
||||
body=body,
|
||||
query=build_query(next_page),
|
||||
)
|
||||
return SyncPage(next_data, ConclusionResponse, transform, fetch_next)
|
||||
|
|
|
|||
|
|
@ -136,9 +136,3 @@ def conclusions_query(workspace_id: str) -> str:
|
|||
|
||||
def conclusion(workspace_id: str, conclusion_id: str) -> str:
|
||||
return f"/{API_VERSION}/workspaces/{workspace_id}/conclusions/{conclusion_id}"
|
||||
|
||||
|
||||
def conclusion_derived(workspace_id: str, conclusion_id: str) -> str:
|
||||
return (
|
||||
f"/{API_VERSION}/workspaces/{workspace_id}/conclusions/{conclusion_id}/derived"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -397,10 +397,10 @@ describe('Conclusions', () => {
|
|||
})
|
||||
|
||||
// ===========================================================================
|
||||
// Derived Conclusions (GET /conclusions/:id/derived)
|
||||
// Derived Conclusions (list + parent_id filter)
|
||||
// ===========================================================================
|
||||
|
||||
describe('GET /conclusions/:id/derived', () => {
|
||||
describe('derived() via parent_id filter', () => {
|
||||
test('leaf conclusion has no derived conclusions', async () => {
|
||||
const peer = await client.peer('derived-conclusion-peer', { metadata: {} })
|
||||
const session = await client.session('derived-conclusion-session', { metadata: {} })
|
||||
|
|
|
|||
|
|
@ -222,17 +222,14 @@ export class ConclusionScope {
|
|||
reverse?: boolean
|
||||
}
|
||||
): Promise<PageResponse<ConclusionResponse>> {
|
||||
await this._ensureWorkspace()
|
||||
return this._http.get<PageResponse<ConclusionResponse>>(
|
||||
`/${API_VERSION}/workspaces/${this.workspaceId}/conclusions/${conclusionId}/derived`,
|
||||
{
|
||||
query: {
|
||||
page: params.page,
|
||||
size: params.size,
|
||||
reverse: params.reverse ? 'true' : undefined,
|
||||
},
|
||||
}
|
||||
)
|
||||
// Sugar for list with a parent_id filter; an unknown conclusionId
|
||||
// yields an empty page rather than an error.
|
||||
return this._list({
|
||||
filters: { parent_id: conclusionId },
|
||||
page: params.page,
|
||||
size: params.size,
|
||||
reverse: params.reverse,
|
||||
})
|
||||
}
|
||||
|
||||
private async _delete(conclusionId: string): Promise<void> {
|
||||
|
|
|
|||
|
|
@ -132,11 +132,14 @@ def get_documents_with_filters(
|
|||
# Apply additional filters if provided
|
||||
stmt = apply_filter(stmt, models.Document, filters)
|
||||
|
||||
# Order by created_at (newest first by default)
|
||||
# created_at is the transaction timestamp, so documents created in the
|
||||
# same batch share it -- id keeps pagination deterministic.
|
||||
if reverse:
|
||||
stmt = stmt.order_by(models.Document.created_at.asc())
|
||||
stmt = stmt.order_by(models.Document.created_at.asc(), models.Document.id.asc())
|
||||
else:
|
||||
stmt = stmt.order_by(models.Document.created_at.desc())
|
||||
stmt = stmt.order_by(
|
||||
models.Document.created_at.desc(), models.Document.id.desc()
|
||||
)
|
||||
|
||||
return stmt
|
||||
|
||||
|
|
|
|||
|
|
@ -153,40 +153,6 @@ async def get_conclusion(
|
|||
return schemas.Conclusion.model_validate(documents[0])
|
||||
|
||||
|
||||
@router.get(
|
||||
"/{conclusion_id}/derived",
|
||||
response_model=Page[schemas.Conclusion],
|
||||
)
|
||||
async def get_derived_conclusions(
|
||||
workspace_id: str = Path(...),
|
||||
conclusion_id: str = Path(...),
|
||||
reverse: bool | None = Query(
|
||||
False,
|
||||
description="Whether to reverse the order of results",
|
||||
),
|
||||
db: AsyncSession = read_db,
|
||||
):
|
||||
"""
|
||||
Get the Conclusions derived from the given Conclusion — i.e. those that list it
|
||||
in their `source_ids`. Traverses the reasoning tree upward (source -> derived).
|
||||
Results are ordered by recency unless `reverse` is true, and paginated.
|
||||
"""
|
||||
documents = await crud.get_documents_by_ids(
|
||||
db,
|
||||
workspace_name=workspace_id,
|
||||
document_ids=[conclusion_id],
|
||||
)
|
||||
if not documents:
|
||||
raise ResourceNotFoundException("Conclusion not found")
|
||||
|
||||
stmt = crud.get_child_observations(
|
||||
workspace_name=workspace_id,
|
||||
parent_id=conclusion_id,
|
||||
reverse=reverse or False,
|
||||
)
|
||||
return await apaginate(db, stmt)
|
||||
|
||||
|
||||
@router.delete(
|
||||
"/{conclusion_id}",
|
||||
status_code=204,
|
||||
|
|
|
|||
|
|
@ -225,7 +225,9 @@ class TestConclusionRoutes:
|
|||
db_session, test_workspace.name, test_peer.name, test_peer2.name
|
||||
)
|
||||
|
||||
# Create conclusions
|
||||
# Create conclusions with distinct timestamps — docs committed in one
|
||||
# transaction share created_at, which would make ordering arbitrary
|
||||
base = datetime.datetime.now(datetime.timezone.utc)
|
||||
doc1 = models.Document(
|
||||
workspace_name=test_workspace.name,
|
||||
observer=test_peer.name,
|
||||
|
|
@ -233,6 +235,7 @@ class TestConclusionRoutes:
|
|||
content="First conclusion",
|
||||
embedding=[0.1] * 1536,
|
||||
session_name=test_session.name,
|
||||
created_at=base,
|
||||
)
|
||||
db_session.add(doc1)
|
||||
await db_session.flush()
|
||||
|
|
@ -244,6 +247,7 @@ class TestConclusionRoutes:
|
|||
content="Second conclusion",
|
||||
embedding=[0.2] * 1536,
|
||||
session_name=test_session.name,
|
||||
created_at=base + datetime.timedelta(seconds=1),
|
||||
)
|
||||
db_session.add(doc2)
|
||||
await db_session.commit()
|
||||
|
|
@ -1675,7 +1679,7 @@ class TestConclusionRoutes:
|
|||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""Test traversing the reasoning tree upward via /derived"""
|
||||
"""Test traversing the reasoning tree upward via the parent_id filter"""
|
||||
test_workspace, test_peer = sample_data
|
||||
|
||||
test_peer2 = models.Peer(
|
||||
|
|
@ -1692,8 +1696,9 @@ class TestConclusionRoutes:
|
|||
db_session, test_workspace.name, test_peer.name, test_peer2.name
|
||||
)
|
||||
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/{premise.id}/derived"
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/list",
|
||||
json={"filters": {"parent_id": premise.id}},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
|
@ -1710,8 +1715,9 @@ class TestConclusionRoutes:
|
|||
assert child["times_derived"] == 2
|
||||
|
||||
# derived1 is itself a source of derived2
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/{derived1.id}/derived"
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/list",
|
||||
json={"filters": {"parent_id": derived1.id}},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
|
@ -1741,9 +1747,10 @@ class TestConclusionRoutes:
|
|||
db_session, test_workspace.name, test_peer.name, test_peer2.name
|
||||
)
|
||||
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/{premise.id}/derived",
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/list",
|
||||
params={"reverse": "true"},
|
||||
json={"filters": {"parent_id": premise.id}},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
|
@ -1774,8 +1781,9 @@ class TestConclusionRoutes:
|
|||
db_session, test_workspace.name, test_peer.name, test_peer2.name
|
||||
)
|
||||
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/{derived2.id}/derived"
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/list",
|
||||
json={"filters": {"parent_id": derived2.id}},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
|
@ -1789,15 +1797,16 @@ class TestConclusionRoutes:
|
|||
client: TestClient,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""Test 404 when the source conclusion doesn't exist"""
|
||||
"""A nonexistent parent yields an empty page, not an error"""
|
||||
test_workspace, _test_peer = sample_data
|
||||
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/{generate_nanoid()}/derived"
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/conclusions/list",
|
||||
json={"filters": {"parent_id": str(generate_nanoid())}},
|
||||
)
|
||||
|
||||
assert response.status_code == 404
|
||||
assert "not found" in response.json()["detail"].lower()
|
||||
assert response.status_code == 200
|
||||
assert response.json()["items"] == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_conclusions_filter_by_source_ids_contains(
|
||||
|
|
|
|||
Loading…
Reference in New Issue