Repository navigation
bug(sdk): three SDK methods send query params the routes do not accept, so pagination and period are ignored #15119
Description
Problem
Three SDK methods send query parameters the route does not accept. AutoBotClient.get() drops
None values but passes everything else through (libs/autobot-sdk-python/autobot_sdk/client.py:116-121),
so the parameter is sent, ignored by FastAPI, and the caller's intent silently does not apply.
1. Pagination never advances. resources/knowledge.py:57 sends offset=:
raw = await self._c.get("/knowledge_base/entries", limit=limit, offset=offset, category=category)GET /api/knowledge_base/entries (autobot-backend/api/knowledge.py:1669) declares cursor,
not offset (api/knowledge.py:1678) — the route is cursor-paginated. Every call therefore
returns the first page regardless of the offset passed, and the route's real next_cursor
is not modelled by the SDK at all.
2/3. Analytics time window is ignored. resources/analytics.py:25 and :29 both send
period=. Neither get_usage_statistics (autobot-backend/api/analytics.py:369) nor
get_performance_metrics (api/analytics.py:288) declares such a query parameter, so both
always return whatever their default window is.
Acceptance criteria
-
knowledge.entries()paginates: a second call with the cursor from the first returns
different rows, asserted against a stubbed route rather than by inspection. - The SDK surfaces
next_cursorso a caller can page at all. - The two analytics methods either send a parameter the route accepts, or stop advertising
aperiodargument they cannot honour. - A guard asserts each SDK-sent query parameter name exists on the route signature it
targets, so the next one added fails CI instead of being ignored at runtime.
Notes
Found while delivering #15072, alongside #15114, #15116 and the field-name drift issue. Those
three are response-shape drift; this one is request-shape drift, which the response contract
guard added by #15072 does not cover.
Activity
fix(sdk): stop sending query parameters the routes do not accept (#15119
fix(sdk): stop sending query parameters the routes do not accept (#15119
chore(sdk): merge Dev_new_gui after #15136 squashed, keeping both fix…
chore(sdk): reconcile with the branch's already-pushed Dev_new_gui me…
fix(sdk): stop sending query parameters the routes do not accept (#15119
github-actions commentedon Aug 28, 2026
PR #15137 (merged to
Dev_new_gui) references this issue with a close keyword.If this issue is fully resolved, close it manually. If work remains, no action is needed.
mrveiss commentedon Aug 28, 2026
Verified against merged
fc3a93aa4— three criteria met, AC4 partially. NOT closing.PR #15137 merged as
fc3a93aa4.Closesdoes not fire onDev_new_gui, so this was open mechanically; verified per criterion rather than assumed.knowledge.entries()paginates, asserted against a stubbed routerepo_tests/sdk_request_url_test.py:329-360—MockTransportkeyed on thecursorparam, assertsseen_cursors == ["0","17"]and that the second page returns different rows. Fix atlibs/autobot-sdk-python/autobot_sdk/resources/knowledge.py:61,75next_cursorso a caller can page at alllibs/autobot-sdk-python/autobot_sdk/models.py:319, consumed atsdk_request_url_test.py:352,357periodresources/analytics.py:34,39,42,44— both now send no query params; routes declare none (autobot-backend/api/analytics.py:288,369)Why AC4 is not fully met
The guard is real and sound for what it checks — the oracle is built with
get_openapiand is protected against a vacuous pass atsdk_request_url_test.py:274-278, andrepo_testsis a gating pytest root (.github/workflows/ci.yml:287).But its input is the pinned request rows, not the SDK's method signatures. So:
Nonedefault is sent by every call, appears in the observed set, and is caught;Noneis dropped byAutoBotClient.get()when no row passes it, so the observed set is unchanged and the guard passes — while real callers who do pass it send a parameter the route ignores.Most optional parameters default to
None, so the case the criterion is about is the case the guard misses. AC4's operative clause is "so the next one added fails CI"; for the common shape, it does not. Filed as #15187, and this issue stays open until that closes.Two things this verification surfaced
autobot-backend/api/chat_sessions.pyaccepts, validates and echoespageat:345,:358,:370but never applies it (:362passes onlyper_pageas the limit). Every page returns the first slice, labelled with the requested page number. PR fix(sdk): stop sending query parameters the routes do not accept (#15119) #15137 newly wired the SDK to send it, so this is now reachable — the same defect class as this issue, one layer down, and the harder half: a rejected parameter eventually surfaces, a silently-ignored one does not. This guard passes it, becausepagegenuinely does exist on the route signature.python-suiteshards had not reported and the aggregatepython-suitecontext readskipping.python-suiteis not a required check (ci: Python unit-test suite is not a required check — red tests merged to Dev_new_gui twice #13162, ci: the backend Python suite gates nothing — python-suite is not a required check #14353), so the ten required contexts were green and my merge gate allowed it — but the deliverable of that PR was a guard, and I had no evidence the shard carrying it ever executed. My gate now refuses to pass a PR that touches test files while the shards are incomplete or not green.Refs #15137, #15186, #15187.
mrveiss commentedon Aug 29, 2026
Picked this up expecting the three-method fix described in the body to be unstarted; found it already merged in PR #15137 and this issue's remaining scope narrowed to AC4, tracked separately as #15187. Delivered that in PR #15265 (signature-derived query-parameter guard + contrast-mutation test), rather than duplicating #15137's work. Once #15265 lands and #15187 closes, this issue should be re-checked for closure per the earlier verification comment's own condition.
refactor(sdk-guard): split the query-parameter guard under the 600-li…
mrveiss commentedon Aug 29, 2026
AC4 now holds — closing
The earlier verification comment recorded three of four criteria met and this one only partially, then held the issue open on it. That remainder was split out as #15187 and is now merged as
965f21c80(PR #15265).knowledge.entries()paginates, asserted against a stubbed routenext_cursorAC4 was the interesting one, and it was not met by the original guard for a reason worth keeping on the record: that guard's input was the pinned rows, not the code. A parameter defaulting to
Noneis dropped by the client when no row passes it, so it never reached the wire to be checked — the guard passed while a real caller who did pass it sent something FastAPI ignores. "The next one added fails CI" was true only for non-Nonedefaults, which is the minority.The strengthened guard derives its parameter set from
inspect.signatureandtyping.get_type_hintsand force-calls each method so every optional parameter reaches the wire regardless of the rows — while still separately requiring that a pinned row exercise each name, so one hole was not traded for another.Explicitly not covered, so nobody assumes it is: #15186's class — a parameter the route accepts and then ignores. It exists on the signature, so it passes a name-existence check. That is a behavioural defect and needs a different kind of test; it is stated in the guard module itself rather than left to inference.
All four criteria met against merged code. Closing.