Skip to content

v0.1.10: add storage-neutral cache lifecycle and task state models#147

Open
rickisba wants to merge 3 commits into
mainfrom
codex/-pr-#146-p2
Open

v0.1.10: add storage-neutral cache lifecycle and task state models#147
rickisba wants to merge 3 commits into
mainfrom
codex/-pr-#146-p2

Conversation

@rickisba

Copy link
Copy Markdown
Collaborator

Motivation

Description

  • Introduce frozen Pydantic lifecycle contracts in core/state_models.py: shared enums (ArtifactState, ReplicaState, DataPlaneTaskState, QueueWorkState), immutable _StateModel, transition_to() that returns a copy plus TransitionResult, InvalidStateTransition with structured detail, and deterministic stable_id helpers; direct field assignment now raises Pydantic frozen-instance ValidationError.
  • Add an immutable Legacy projection in kdn_server/legacy_state.py that: normalizes and validates kid, resolves runtime Replica directory as <kv_root>/<kid> for health checks, rejects path-escaping values, treats kv_rel_dir as legacy metadata (validated but not authoritative), emits a machine-readable legacy_kv_rel_dir_mismatch warning when kv_rel_dir differs from kid, and never treats kv_rel_dir="." as making the KV root a valid Replica directory.
  • Keep existing behavior and contracts unchanged where required (enum wire values, terminal-state semantics, retryable failed, stable sorted allowed-target output, SQLite layout and mark_kv_ready semantics, and injection/routing logic), and do not alter injection code or manifests.
  • Add focused regression tests: test/test_state_models.py (immutability, idempotency, copy-on-transition, structured terminal rejection) and test/test_legacy_kv_state_mapping.py (runtime-kid checks, stale kv_rel_dir, . handling, path escape rejection, immutability of projection, and read-only mapping).
  • Make core.forward_request lazily import its optional aiohttp dependency to allow CPU-only state-contract tests to import the package without aiohttp installed.

Testing

  • python3 -m compileall core kdn_server proxy — completed successfully.
  • pytest -q test/test_state_models.py — passed: 7 passed (with unrelated SWIG deprecation warnings).
  • pytest -q test/test_legacy_kv_state_mapping.py — passed: 6 passed (with unrelated SWIG deprecation warnings).
  • pytest -q test/test_state_models.py test/test_legacy_kv_state_mapping.py test/test_instance_capability.py test/test_instance_capability_registration.py proxy/resource/test_instance_pool_gpu.py — collection stopped with environment-dependent errors due to missing optional test dependencies, specifically ModuleNotFoundError: No module named 'fastapi' in test/test_instance_capability.py and test/test_instance_capability_registration.py (these are unrelated to the new state-model changes).
  • pytest -q --ignore=test/test_kv_injector_reuse.py — collection stopped for the same environment-dependent reasons (missing fastapi) and one pre-existing test that reads a placeholder path raising FileNotFoundError; these failures are not introduced by this PR.

Notes and current status

  • Local commit created: f491ba96f0494217e9c75cb9c4506948c7060ecc containing the changes above, and the changed files are core/__init__.py, core/state_models.py, kdn_server/legacy_state.py, test/test_state_models.py, and test/test_legacy_kv_state_mapping.py.
  • Attempt to push the branch to update the remote PR failed due to environment network/proxy restrictions (fatal: unable to access 'https://github.com/AstraNetLab/CacheRoute.git/': CONNECT tunnel failed, response 403), so the remote PR was not updated from this environment; the commit and tests are present locally as reported above.
  • Limitations: runtime push to the repository and execution of tests requiring network or optional packages (fastapi, aiohttp) could not be completed in this environment; those are environment-dependent and pre-existing outside the scope of the fixes here.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7489ecd52

ℹ️ 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".

Comment thread kdn_server/legacy_state.py Outdated
Comment on lines +42 to +43
get = row.get if hasattr(row, "get") else lambda key, default=None: getattr(row, key, default)
kid = _safe_component(get("kid"), "kid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Support sqlite3.Row in legacy mapping

When this helper receives a row directly from the legacy SQLite database, it always rejects it: TextDatabase._connect() configures sqlite3.Row, which has neither .get() nor field attributes, so this fallback returns None for kid even though row["kid"] exists, and _safe_component raises. Handle mapping-style subscription before falling back to attribute access.

Useful? React with 👍 / 👎.

Comment thread kdn_server/legacy_state.py Outdated
directory_exists = runtime_directory.is_dir()
kv_ready = bool(get("kv_ready", False))
artifact_state = ArtifactState.READY if kv_ready else ArtifactState.PENDING
replica_state = ReplicaState.READY if kv_ready and directory_exists else ReplicaState.FAILED

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep unbuilt legacy replicas pending

For every normal text-only row with kv_ready=0, this assigns ReplicaState.FAILED regardless of whether any build or copy has failed. Consumers of this projection will therefore count newly registered or not-yet-built knowledge as failed and receive misleading resource-state visibility; use PENDING when kv_ready is false and reserve FAILED for stale kv_ready=1 metadata whose runtime directory is missing.

AGENTS.md reference: AGENTS.md:L219-L230

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Merge-blocking architecture alignment after the updated #139

The state-model implementation is broadly compatible with the updated direction and does not need to be redesigned, but the PR should not merge until its documentation and contract wording make the LMCache ownership boundary unambiguous.

Required before merge:

  1. Update doc/state-models.md so that:
    • CacheArtifact is a knowledge- and compatibility-specific logical materialization identity;
    • CacheReplica is a logical observation/reference to KV data physically managed by LMCache (or by the explicitly named Legacy compatibility path), not a CacheRoute-owned physical placement;
    • DataPlaneTask represents a logical operation that a later LMCache adapter may execute, not a new CacheRoute storage/transfer engine;
    • physical chunk existence, backend tier, block residency, move/pin/clear/prefetch execution, and runtime health remain LMCache facts.
  2. Add equivalent clarifying docstrings/comments in core/state_models.py; keep the existing storage-neutral fields and opaque non-secret location_key.
  3. Do not add KV bytes, Redis keys, credentials, cache serialization, block allocation, storage tables, or a CacheRoute-owned backend.
  4. Preserve the current fixes already visible in the branch:
    • dictionary/sqlite3.Row/attribute record access;
    • kv_ready=0 without confirmed files maps to PENDING;
    • stale kv_ready=1 with a missing runtime directory maps the Replica to FAILED/UNHEALTHY;
    • immutable models and validated copy-on-transition.
  5. Add or adjust assertions proving that the contracts are logical/storage-neutral and that opaque location identities cannot carry credentials or connection strings.
  6. Refresh the PR body. It currently says the latest local fixes could not be pushed, but the remote PR now contains the corrected mapper and immutable model changes. Report the actual remote head and current test results.
  7. Run the focused Python 3 validation in an environment with the documented test dependencies. No GitHub status checks are currently attached to the PR head, so the focused results need to be recorded explicitly.

Keep this PR scoped to updated #139. Do not implement the LMCache adapter or #140 protocol here.

@rickisba rickisba changed the title v0.1.10: add cache lifecycle and task state models v0.1.10: add storage-neutral cache lifecycle and task state models Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.1.10-2: Define Storage-Neutral KDN, Cache Observation, Serving Task, and Queue State Models

1 participant