Repository navigation
feat(kdn): define versioned cache service and LMCache gateway contracts - #154
Conversation
![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: 471293f841
ℹ️ 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".
| kind = GatewayTransportKind(transport_kind) | ||
| if kind is GatewayTransportKind.MOCK: |
There was a problem hiding this comment.
When transport_kind differs from capabilities.transport_kind, the factory silently constructs the requested adapter anyway; for example, create_gateway("mock", v1_http_capabilities) bypasses CapabilitySnapshot.coherent_profile() and runs a Mock gateway while advertising v1 HTTP capabilities. This defeats the explicit Legacy/Mock isolation and can produce simulated results in an experiment configured for a production transport, so reject mismatched kinds before constructing the adapter.
Useful? React with 👍 / 👎.
| class OperationIntentRequest(ArtifactRequest): | ||
| idempotency_key: str = Field(min_length=1) | ||
| endpoint_id: str | ||
| endpoint_generation: int = Field(ge=1) |
There was a problem hiding this comment.
An operation request inherits the optional compatibility_profile_id=None, so it passes contract validation without compatibility metadata, but MockGateway.submit_operation() immediately passes that value to CacheOperationTask, where it is required, causing a late ValidationError. Redeclare this field as a required non-empty string on OperationIntentRequest so invalid wire requests are rejected at the contract boundary and tasks retain explicit compatibility typing.
AGENTS.md reference: AGENTS.md:L81-L85
Useful? React with 👍 / 👎.
| covered_ranges: tuple[tuple[int, int], ...] = () | ||
| total_tokens: int = Field(ge=0) |
There was a problem hiding this comment.
The contract currently accepts negative or reversed ranges such as covered_ranges=((-1, 4), (8, 3)). Any gateway response containing such data still validates successfully and can make scheduling or experiment accounting report nonsensical cache coverage, so validate each range's non-negative, increasing bounds before admitting the response.
Useful? React with 👍 / 👎.
| return self._response(request, outcome=OutcomeCode.SUCCESS if observation else OutcomeCode.STALE, observation=observation) | ||
| def lookup_tokens(self, request): return self._response(request, outcome=OutcomeCode.UNSUPPORTED) |
There was a problem hiding this comment.
When a stored Legacy observation exists but its TTL has expired—or it is a projection without a usable observation timestamp—this method still returns OutcomeCode.SUCCESS. The Mock adapter correctly checks is_fresh(), and callers relying on the response outcome can otherwise route using obsolete cache state; return STALE unless the observation exists and is fresh.
Useful? React with 👍 / 👎.
| if task and not task.terminal: | ||
| task = task.transition("cancelled"); self._tasks[key] = task | ||
| return self._response(request, outcome=OutcomeCode.CANCELLED if task else OutcomeCode.STALE, operation=task) |
There was a problem hiding this comment.
If cancellation is requested after a task has already succeeded or failed, the transition is skipped but the response still reports OutcomeCode.CANCELLED while embedding the unchanged terminal operation. Consumers that inspect only the outcome will record a successful or failed task as cancelled, so return a non-cancelled outcome when the existing task was already terminal.
Useful? React with 👍 / 👎.
1 participant
Closes #140
Motivation
autoruntime profiles and reject sensitive or physical payloads at the contract boundary.Description
kdn_server/contracts/includingcommon.py,errors.py,knowledge.py, andcache_service.pythat define frozen Pydantic envelopes,VersionedMessage,TokenInput, structuredContractError, and the Cache Service Facade request/response types.kdn_server/gateway/includingprofiles.pyandcapabilities.pywith explicit tri-stateSupportStateand negotiation checks that avoid implicit Legacy/Mock usage byv1profiles.kdn_server/gateway/protocol.pyand provided test adapters: a deterministic CPU-onlyMockGateway(mock.py) implementing idempotent operation submission, text-fallback behavior, async completion/cancellation, and a read-onlyLegacyCacheAdapter(legacy.py) that rejects v1 write operations.test/kdn/test_cache_service_contracts.pyexercising version/profile validation, serialization round-trips, gateway vocabulary and capability semantics, token fallback, idempotency conflict behavior, completion/cancellation, Legacy isolation, and rejection of secret/physical fields.Testing
python3 -m compileall -q cacheroute_compat kdn_server/contracts kdn_server/gateway test/kdn— passed.python3 -m pytest -q test/kdn— collection blocked due to missingpydanticin the environment (installation was prevented by network/proxy restrictions in this run).python3 -m pytest -q test/test_runtime_compat.py— collection blocked due to missingnumpyin the environment.python3 -m pytest -q test/test_instance_capability.py test/test_instance_capability_registration.py— collection blocked due to missingnumpyandfastapiin the environment.git diff --check— no whitespace or diff-check issues.If CI provides Pydantic / NumPy / FastAPI, the added tests under
test/kdnshould run and validate the contracts and gateway adapters as described.Codex Task