Skip to content

docs(audit): which model identity facts a local run records (#734, slice 1) - #905

Merged
github-actions[bot] merged 2 commits into
mainfrom
fix/734-model-identity-inventory
Nov 6, 2026
Merged

docs(audit): which model identity facts a local run records (#734, slice 1)#905
github-actions[bot] merged 2 commits into
mainfrom
fix/734-model-identity-inventory

Conversation

@cdeust

@cdeust cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Summary

First slice of #734: docs/audits/model-identity-inventory-2026-11-06.md lists which embedding and reranker identity facts a local run records, where each is exposed (get_telemetry, cortex doctor, the SQLite memory stamp, the two benchmark manifests), and where a recorded value does not describe what the run served. No behaviour change.

Refs #734 (slice 1 of 5; the issue stays open).

Root cause addressed by this slice: the acceptance asks for one effective manifest, and nobody had written down what the current fields actually mean. Findings, each with its code anchor in the document:

  • The embedding revision is pinned (DEFAULT_MODEL_REVISION), but no embedding file is hashed anywhere in mcp_server/.
  • The reranker sha256 is computed (model_sha256()) and recorded in both manifests, but never compared with an expected value; the value d3dd7b09… exists only in committed results.
  • No surface reports the embedding device or ModelState, nor the reranker's device or inference provider.
  • write_manifest.embedding_revision() records the configured pin, not what was loaded: in the probe, the engine sits in PACKAGE_ABSENT, serves fallback vectors, and the manifest still records 1110a243…. Neither manifest records the embedding mode.
  • memories.embedding_model (SQLite only) stores the vector space (neural/fallback), not the model.

Remaining slices (listed at the end of the document): record the mode and state actually served; one effective manifest shared by cortex doctor and the benchmarks; verify provisioned models against a pinned identity; an offline receipt labelled with the mode.

Type of change

  • Documentation only

Test plan

  • scripts/check_craftsmanship.py: OK.
  • pytest tests_py/scripts -k "doc or changelog or readme": 231 passed, 1 skipped.
  • pytest tests_py -k "changelog or docs_link or markdown or audit": 63 passed, 35 skipped.
  • git diff --check: clean.
  • Probe of get_embedding_engine(), embedding_revision() and reranker_fields() run in this container on main 79bd6c3, quoted in the document.
  • No code changed, so no new test; mutation survival not applicable.

Audit notes

  • Not verified: a LOADED neural engine and a loaded FlashRank model (neither torch nor the model files could be downloaded through this container's proxy), GPU devices, and the PostgreSQL store; those rows are read from the code.
  • Not run: the full pytest suite (environment installed from uv.lock without torch and sentence-transformers).

Reviewer checklist

  • CHANGELOG.md updated under the appropriate section (Unreleased / Added).
  • Documentation updated (docs/audits/).
  • No secrets / credentials / PII in the diff.

Generated by Claude Code

@claude
docs(audit): which model identity facts a local run records (#734, sl…
f549308 
…ice 1)

Inventory of the embedding and reranker identity facts Cortex records and
where each is exposed. The embedding revision is pinned but no embedding
file is hashed; the reranker sha256 is recorded but never compared with an
expected value; no surface reports the embedding device or load state; and
the benchmark manifest records the pinned embedding_model_revision even when
the run served fallback embeddings (probe in the document).

Refs #734

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYt64oiUt3uW4itfq5F
@cdeust cdeust mentioned this pull request Nov 6, 2026
Open
5 tasks
@cdeust
cdeust marked this pull request as ready for review November 6, 2026 15:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T15:41:51.321072Z 0901c97 Draft marked ready
🔒 Security Review ✅ Completed 2026-11-06T15:42:30.041687Z 0901c97 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @cdeust's task in 1m 48s —— View job

Review of PR #905 (review 1 of 3)

No blockers. The doc's claims match the code. No secrets are in the diff, and no CI test, craftsmanship rule or doc-claims gate reads these files.

Three small notes don't need action before merge:

  • Doc line 15: cites the wrong file for model_sha256(). It is in core/reranker_model.py. The file cited, infrastructure/reranker_cache.py, holds reranker_model_sha256().
  • Doc line 16: the reranker digest d3dd7b09 also appears in two docs files, not only in committed results.
  • Doc line 20: the "Offline guard" cell leaves out CORTEX_EMBEDDING_ZERO_DOWNLOAD, the setting that stops the background embedding download.

I didn't run pytest: no code changed and none of the existing tests read these files.

Verdict: approve

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 1 of 3: PR #905 (docs(audit): model identity inventory, #734 slice 1)

Five reviews ran: code quality, performance, test coverage, documentation accuracy and security. I checked every factual claim in docs/audits/model-identity-inventory-2026-11-06.md against mcp_server/ and benchmarks/. The central finding holds: EmbeddingEngine.revision (embedding_engine.py:100-102) returns the pinned revision whatever the load state, and write_manifest.embedding_revision() records it under a docstring that says "loaded" (benchmarks/lib/write_manifest.py:78). The ModelState members, the RerankerStatus states, the existence-only check behind CORTEX_RERANKER_OFFLINE (core/reranker.py:72), the SQLite-only embedding_model stamp and the 131 d3dd7b09 occurrences under benchmarks/results/ all match the code. No secrets. No test, craftsmanship rule or doc-claims gate reads docs/audits/ or the format of CHANGELOG.md.

Blockers: none.

Notes, no action before merge

  • Doc line 15: model_sha256() is in core/reranker_model.py:81. The file the doc cites, infrastructure/reranker_cache.py, holds reranker_model_sha256() (line 36), which does the hashing.
  • Doc line 16: "appears only in committed results" is too strong. d3dd7b09 also appears in benchmarks/reranker_matrix/README.md and docs/benchmarks/issue-514-retrieval-comparison.json. It is still never compared in code.
  • Doc line 20: the "Offline guard" cell leaves out CORTEX_EMBEDDING_ZERO_DOWNLOAD (embedding_downloads.py:21), which suppresses the background download. Like the reranker guard, it does not check identity.

Verdict: approve

@cdeust Claude

cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

Les trois notes de la revue 1 sont exactes : je les ai vérifiées dans le code. Je ne pousse rien pour elles, conformément à ADR-1119, qui réserve les pushs aux bloquants. Elles seront corrigées dans le document par la tranche 2 de #734 :

  • model_sha256() est dans core/reranker_model.py, qui délègue le hachage à reranker_model_sha256() (infrastructure/reranker_cache.py) ;
  • d3dd7b09… apparaît aussi dans benchmarks/reranker_matrix/README.md et docs/benchmarks/issue-514-retrieval-comparison.json, mais n'est toujours comparé nulle part dans le code ;
  • CORTEX_EMBEDDING_ZERO_DOWNLOAD supprime le téléchargement en arrière-plan, sans vérifier l'identité du modèle.

Generated by Claude Code

@cdeust Claude

cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

Le check install (macos-latest) du run 42245314530 a été annulé à 16:20:32Z sans qu'aucune étape ne s'exécute : l'annotation GitHub dit « The job was not acquired by Runner of type hosted even after multiple attempts » (file d'attente macOS arm64 saturée), puis merge et tag ont été sautés. La PR ne touche que docs/audits/ et CHANGELOG.md, et tous les autres jobs du même run sont verts (unit, regression, e2e, environment ×3, install ubuntu/windows) : c'est une panne de capacité des runners, pas la PR. J'ai relancé les jobs en échec (rerun-failed-jobs, tentative 2 du run), sans commit.

Non vérifié : l'issue de la tentative 2. La branche est aussi en retard d'un commit sur main ; le rebase est prêt localement (diff identique ligne à ligne, conflit CHANGELOG résolu en gardant les deux entrées), mais le push --force-with-lease a été refusé par les permissions de cette session d'autopilot, donc il n'est pas poussé.

Generated by Claude Code

@github-actions
github-actions Bot merged commit 8b8e4a0 into main Nov 6, 2026
43 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cdeust @claude