fix: enforce internal embeddings contract
This commit is contained in:
@@ -90,3 +90,60 @@ git diff --check
|
|||||||
- the focused harness verification still emits two pre-existing warnings:
|
- the focused harness verification still emits two pre-existing warnings:
|
||||||
- `DeprecationWarning` from `testcontainers.postgres`
|
- `DeprecationWarning` from `testcontainers.postgres`
|
||||||
- `FutureWarning` because `resources` currently flows through the legacy config translation path
|
- `FutureWarning` because `resources` currently flows through the legacy config translation path
|
||||||
|
|
||||||
|
## Fix round 1 — 2026-08-08
|
||||||
|
|
||||||
|
### Findings addressed
|
||||||
|
|
||||||
|
- HIGH: external top-level `embeddings` remained an operational fallback and could still load
|
||||||
|
- MEDIUM: non-object embed JSON payloads escaped as raw `AttributeError`
|
||||||
|
|
||||||
|
### RED evidence
|
||||||
|
|
||||||
|
Command:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd harness
|
||||||
|
./.venv/bin/pytest tests/test_internal_embeddings.py tests/test_config_resources.py tests/test_ollama_ensure.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Observed before the fix:
|
||||||
|
|
||||||
|
- exit code `1`
|
||||||
|
- `2 failed, 36 passed, 2 warnings`
|
||||||
|
|
||||||
|
Representative failures:
|
||||||
|
|
||||||
|
- `AttributeError: 'list' object has no attribute 'get'` from `response.json()` returning a JSON array
|
||||||
|
- `Failed: DID NOT RAISE ConfigError` for top-level external `embeddings.provider=openai_compatible`
|
||||||
|
|
||||||
|
### GREEN evidence
|
||||||
|
|
||||||
|
Command:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd harness
|
||||||
|
./.venv/bin/pytest tests/test_internal_embeddings.py tests/test_config_resources.py tests/test_ollama_ensure.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Observed after the fix:
|
||||||
|
|
||||||
|
- exit code `0`
|
||||||
|
- `38 passed, 2 warnings`
|
||||||
|
|
||||||
|
Touched-file lint:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd harness
|
||||||
|
./.venv/bin/ruff check tht/config.py tht/vectorstore/embeddings.py tests/test_internal_embeddings.py tests/test_config_resources.py
|
||||||
|
```
|
||||||
|
|
||||||
|
- exit code `0`
|
||||||
|
- `All checks passed!`
|
||||||
|
|
||||||
|
### Minimal fix
|
||||||
|
|
||||||
|
- validated the final active `cfg.embeddings` contract after config loading, so legacy top-level
|
||||||
|
embedding inputs now fail explicitly unless they exactly match the internal Ollama contract
|
||||||
|
- converted non-mapping embed JSON payloads into controlled `EmbeddingsError` failures with
|
||||||
|
sanitized diagnostics instead of raw attribute errors
|
||||||
|
|||||||
@@ -106,7 +106,7 @@ roots:
|
|||||||
sessions: {runtime_root / 'sessions'}
|
sessions: {runtime_root / 'sessions'}
|
||||||
artifacts: {runtime_root / 'artifacts'}
|
artifacts: {runtime_root / 'artifacts'}
|
||||||
indexes: {runtime_root / 'indexes'}
|
indexes: {runtime_root / 'indexes'}
|
||||||
embeddings: {{base_url: http://embedding.invalid, model: embed, dim: 768}}
|
embeddings: {{provider: ollama_internal, base_url: http://embedding:11434, model: qwen3-embedding:0.6b, dim: 1024}}
|
||||||
""")
|
""")
|
||||||
monkeypatch.setenv("THT_DATA_ROOT", str(data_root))
|
monkeypatch.setenv("THT_DATA_ROOT", str(data_root))
|
||||||
|
|
||||||
@@ -204,7 +204,7 @@ dwh:
|
|||||||
vectors:
|
vectors:
|
||||||
type: thoth_vector_http
|
type: thoth_vector_http
|
||||||
writer: {base_url: https://vectors.test/, api_key: writer}
|
writer: {base_url: https://vectors.test/, api_key: writer}
|
||||||
embeddings: {base_url: http://ollama:11434, dim: 768}
|
embeddings: {provider: ollama_internal, base_url: http://embedding:11434, model: qwen3-embedding:0.6b, dim: 1024}
|
||||||
"""
|
"""
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -302,6 +302,25 @@ dwh:
|
|||||||
load_config(workspace)
|
load_config(workspace)
|
||||||
|
|
||||||
|
|
||||||
|
def test_rejects_external_top_level_embedding_configuration(tmp_path):
|
||||||
|
workspace = tmp_path / "workspace.yaml"
|
||||||
|
workspace.write_text(
|
||||||
|
"""
|
||||||
|
dwh:
|
||||||
|
type: postgres_direct
|
||||||
|
connection: {database: analytics, schema: mart, user: reader, password: secret}
|
||||||
|
embeddings:
|
||||||
|
provider: openai_compatible
|
||||||
|
base_url: https://embedding.example.test
|
||||||
|
model: text-embedding-3-large
|
||||||
|
dim: 3072
|
||||||
|
"""
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(ConfigError, match="ollama_internal|provider|base_url|model|1024"):
|
||||||
|
load_config(workspace)
|
||||||
|
|
||||||
|
|
||||||
def test_builds_typed_evidence_sources_and_keeps_legacy_compatible(tmp_path):
|
def test_builds_typed_evidence_sources_and_keeps_legacy_compatible(tmp_path):
|
||||||
common = """
|
common = """
|
||||||
dwh:
|
dwh:
|
||||||
|
|||||||
@@ -137,3 +137,20 @@ def test_internal_embeddings_reject_non_finite_values():
|
|||||||
|
|
||||||
with pytest.raises(EmbeddingsError, match="finite|finit"):
|
with pytest.raises(EmbeddingsError, match="finite|finit"):
|
||||||
embedder.embed(["alpha"])
|
embedder.embed(["alpha"])
|
||||||
|
|
||||||
|
|
||||||
|
def test_internal_embeddings_reject_non_object_json_payload():
|
||||||
|
from tht.vectorstore.embeddings import OllamaInternalEmbeddings
|
||||||
|
|
||||||
|
embedder = OllamaInternalEmbeddings(
|
||||||
|
EmbeddingsConfig(
|
||||||
|
provider="ollama_internal",
|
||||||
|
base_url="http://embedding:11434",
|
||||||
|
model="qwen3-embedding:0.6b",
|
||||||
|
dim=1024,
|
||||||
|
),
|
||||||
|
session=_Session([_Response([_vector(1.0)])]),
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(EmbeddingsError, match="response|payload|embeddings"):
|
||||||
|
embedder.embed(["alpha"])
|
||||||
|
|||||||
@@ -454,6 +454,7 @@ def load_config(path: Path) -> Config:
|
|||||||
if cfg.runtime_identity is not None
|
if cfg.runtime_identity is not None
|
||||||
else path.resolve().as_posix()
|
else path.resolve().as_posix()
|
||||||
)
|
)
|
||||||
|
_validate_active_embeddings_config(cfg.embeddings, path)
|
||||||
return cfg
|
return cfg
|
||||||
|
|
||||||
|
|
||||||
@@ -499,6 +500,35 @@ def _validate_internal_embedding_contract(raw: dict[str, Any], path: Path) -> No
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_active_embeddings_config(
|
||||||
|
embeddings: "EmbeddingsConfig | None",
|
||||||
|
path: Path,
|
||||||
|
) -> None:
|
||||||
|
if embeddings is None:
|
||||||
|
return
|
||||||
|
if embeddings.provider != "ollama_internal":
|
||||||
|
raise ConfigError(
|
||||||
|
f"Configurazione non valida in {path}:\n"
|
||||||
|
"embeddings.provider deve essere 'ollama_internal'"
|
||||||
|
)
|
||||||
|
if embeddings.model != "qwen3-embedding:0.6b":
|
||||||
|
raise ConfigError(
|
||||||
|
f"Configurazione non valida in {path}:\n"
|
||||||
|
"embeddings.model deve essere 'qwen3-embedding:0.6b'"
|
||||||
|
)
|
||||||
|
if embeddings.dim != 1024:
|
||||||
|
raise ConfigError(
|
||||||
|
f"Configurazione non valida in {path}:\n"
|
||||||
|
"embeddings.dim deve essere 1024"
|
||||||
|
)
|
||||||
|
if not _is_allowed_internal_embedding_url(embeddings.base_url):
|
||||||
|
raise ConfigError(
|
||||||
|
f"Configurazione non valida in {path}:\n"
|
||||||
|
"embeddings.base_url deve usare http://embedding:11434 "
|
||||||
|
"oppure un endpoint loopback di sviluppo su porta 11434"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _is_allowed_internal_embedding_url(value: Any) -> bool:
|
def _is_allowed_internal_embedding_url(value: Any) -> bool:
|
||||||
if not isinstance(value, str):
|
if not isinstance(value, str):
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -33,7 +33,12 @@ class OllamaInternalEmbeddings:
|
|||||||
raise EmbeddingsError(
|
raise EmbeddingsError(
|
||||||
f"internal Ollama embeddings request failed for model {self.model}"
|
f"internal Ollama embeddings request failed for model {self.model}"
|
||||||
) from exc
|
) from exc
|
||||||
payload = response.json()
|
try:
|
||||||
|
payload = response.json()
|
||||||
|
except ValueError as exc:
|
||||||
|
raise EmbeddingsError("internal Ollama returned an invalid JSON response") from exc
|
||||||
|
if not isinstance(payload, dict):
|
||||||
|
raise EmbeddingsError("internal Ollama returned a non-object response payload")
|
||||||
embeddings = payload.get("embeddings")
|
embeddings = payload.get("embeddings")
|
||||||
if not isinstance(embeddings, list):
|
if not isinstance(embeddings, list):
|
||||||
raise EmbeddingsError("internal Ollama response is missing embeddings")
|
raise EmbeddingsError("internal Ollama response is missing embeddings")
|
||||||
|
|||||||
Reference in New Issue
Block a user