diff --git a/ENGINEERING_REPORT.md b/ENGINEERING_REPORT.md new file mode 100644 index 00000000..620bce4e --- /dev/null +++ b/ENGINEERING_REPORT.md @@ -0,0 +1,170 @@ +# FastEmbed engineering report + +Date: 10 October 2026 (Asia/Calcutta). + +## Delivered change + +Selected [qdrant/fastembed](https://github.com/qdrant/fastembed), created the +GitHub fork [irulappan151204/fastembed](https://github.com/irulappan151204/fastembed), +and implemented focused model-download reliability and diagnostic-security fixes. +The work is on `fix/model-download-resources`. The implementation and nine tests +are in commit `cab30737fc8234f903f09898d53e9ef094e9211c`; the README and this +report are committed separately. No changes were pushed to upstream. + +## Selection and repository evaluation + +All seven repositories exposed by the connected account were screened using +metadata and repository trees, with deeper inspection of the AI candidate and +the Python reporting applications' dependencies. + +| Repository | Assessment | +| --- | --- | +| `Chatting-with-Local-RAG` | The clearest personal AI project: MIT-licensed PDF RAG using LangChain, Chroma, and Ollama. Last push December 2024; a single notebook, no automated tests, invalid package names such as `OllamaEmbeddings` in requirements, and deprecated imports. Reviving it would require establishing a maintained application structure and dependency baseline. | +| `DSR_2.0` and a private reporting repository | Flask/SQL reporting applications. Their dependency lists and inspected structure show reporting rather than an established AI pipeline. | +| `rafithub` | TypeScript fitness website. | +| `newPortfolio` and a private portfolio repository | TypeScript portfolio websites. | +| `irulappan151204` | GitHub profile configuration. | + +Three external Python projects were compared: FastEmbed, NeuML's txtai, and +AnswerDotAI's rerankers. FastEmbed was pushed on 9 October 2026, has about +3,240 stars in the inspected snapshot, and combines an active maintainer, +compact codebase, real RAG use, and testable infrastructure. txtai was also +active but has a substantially broader orchestration and model surface; +rerankers' last observed push was December 2025. FastEmbed offered the best +balance for a bounded, verifiable contribution. This is an engineering judgment +about these inspected candidates, not a claim to have ranked all AI projects. + +Upstream issue review included the +[tuple reranking bug](https://github.com/qdrant/fastembed/issues/796), +[non-finite embedding report](https://github.com/qdrant/fastembed/issues/688), +[CPU/GPU runtime collision](https://github.com/qdrant/fastembed/issues/608), +and [Hub 2.x support](https://github.com/qdrant/fastembed/issues/742). +The tuple and NaN bugs already have open pull requests +[797](https://github.com/qdrant/fastembed/pull/797) and +[691](https://github.com/qdrant/fastembed/pull/691), so their fixes were not +duplicated. The delivered bugs were found by inspecting the current downloader. + +## Architecture, quality, dependencies, and documentation + +The baseline is FastEmbed 0.9.0, commit +`d076f083519746d132888079e6e790f2193c9c08`. + +Public facades select registered dense, sparse, image, late-interaction, and +cross-encoder implementations. Shared model management resolves cached files, +downloads weights/configuration, verifies metadata, and stages archives. +Tokenizers and image processors construct ONNX inputs; ONNX Runtime performs +inference. A worker pool supports ordered multiprocessing. Common numeric +helpers and MUVERA handle postprocessing. + +The codebase uses typed interfaces, dataclass model descriptions, pytest, +Ruff, mypy, and CI across Python 3.10–3.14. Existing cache validation, archive +path checks, bounded HTTP timeouts, and staging logic are valuable safeguards. +The main test suite depends heavily on downloaded models and some hardware. +The downloader lacked exception-safe HTTP response ownership and exposed +custom source URLs in three diagnostics. The README's usage examples were +useful, but this fork adds a reproducible contributor workflow and configuration +guidance. No architecture rewrite or inference-algorithm changes were needed. + +Runtime and test dependencies were installed from the existing `poetry.lock`. +Observed versions include NumPy 2.4.6, Requests 2.34.2, ONNX Runtime 1.30.0, +Hugging Face Hub 1.33.0, Tokenizers 0.23.2, Pillow 12.3.0, pytest 9.1.1, and +mypy 2.4.0. The upstream pre-commit Ruff pin, 0.3.4, was used for final lint +and formatting checks. `pyproject.toml` and `poetry.lock` remain unchanged. + +## Reproduced issues and fixes + +1. **Unclosed HTTP streams on failure.** Real local HTTP responses remained open + after 403/404 responses, malformed content-length values, and destination-open + errors. The response now has a context manager covering status validation, + header parsing, progress reporting, and file writing. Existing exception types, + timeouts, chunk sizes, return values, and successful file bytes are preserved. +2. **Sensitive source URLs in diagnostics.** Missing/zero-length warnings, + fallback-download logs, and missing-model-directory errors exposed a synthetic + signed-URL marker. Those diagnostics now omit the source URL; errors identify + the model and the warning uses the existing Loguru logger. +3. **Development-tool advisories.** A temporary Poetry 2.2.1 environment brought + in Dulwich 0.24.10. pip-audit reported 18 advisory entries: 14 for Dulwich and + four for Poetry, including overlapping advisory aliases. These were tooling + findings, not newly identified FastEmbed runtime vulnerabilities. Upgrading + the isolated tooling to Poetry 2.5.1 brought Dulwich 1.2.17 and resolved the + audit findings. The README pins the verified Poetry version. Poetry's + [published wheel-path advisory](https://github.com/python-poetry/poetry/security/advisories/GHSA-2599-h6xx-hpxp) + independently confirms the risk in the older tooling. + +Nine HTTP regression/compatibility cases exercise real local transport rather +than mocked responses. They cover the failure paths, successful bytes and +closure, absent/zero length, fallback logs, and incomplete archives. Captured +responses and logger sinks are cleaned up by test fixtures. The local-download +test overrides external-model offline mode for isolation. + +## Verification evidence + +Windows, CPython 3.11.15, CPU inference. No API keys or paid inference services +were used. + +| Verification | Observed result | +| --- | --- | +| Initial seven download tests, before production changes | 6 failed, 1 passed; four open-stream assertions and two URL-disclosure assertions reproduced the bugs. | +| Two added diagnostics tests, before their fixes | Both reproduced URL disclosure after fixing a logger-capture issue in the fixture. | +| Focused tests: download, model management, common utilities, image transforms, worker pool | 32 passed; the final run after the tooling upgrade took 8.37 seconds. | +| Existing `tests/test_text_cross_encoder.py`, `CI=1`, online model download | 6 passed in 52.72 seconds with the real `Xenova/ms-marco-MiniLM-L-6-v2` model: scores, batching, lazy loading, parallel inference, token counting, and session configuration. | +| Full suite, `HF_HUB_OFFLINE=1`, `CI=1`, dedicated cache | 57 passed, 23 skipped, 74 failed, 39 setup errors, 4 warnings. All 113 remaining failure/error entries report an uncached model. Full online-suite success is not claimed. | +| Ruff 0.3.4, explicit repository configuration | Check and format check passed for both changed Python files. | +| mypy with upstream CI flags | Success across 66 source files. | +| `uv pip check` | All 76 installed packages compatible. | +| pip-audit after tooling upgrade | 76 packages audited, zero known advisory entries, no skipped packages. This is a database snapshot, not proof of complete security. | +| Git checks | Diff whitespace check passed; license, notice, dependency declarations, and lockfile unchanged. | + +The full offline run also exposed one environment-sensitive assertion in a new +test. Its fixture was corrected, and the full run was repeated; the final counts +above include the correction. The four warnings concern the upstream +ColModernVBERT model update. The accompanying offline log names every failed +and errored test; initial/final dependency-audit JSON files preserve that audit's +progression. + +## Reproduction and local environment + +Use the installation, configuration, and commands in the updated README. +Poetry 2.5.1 successfully installed the unchanged lockfile. This host initially +omitted `PROCESSOR_ARCHITECTURE`, causing Poetry's marker parser to fail; +setting the process variable to its actual value, `AMD64`, resolved that issue. + +Git could not create files under Documents directly. The cloned source is in +the chat workspace's `work/fastembed`; its `.git` pointer refers to metadata in +`%TEMP%/codex-fastembed-20261010/.git`. The isolated virtual environment is +`%TEMP%/codex-fastembed-venv`. Caches were disabled or placed in temporary +directories. A fresh clone in a writable development directory is recommended +for continued work; the delivered Git bundle is a portable recovery copy. + +## Licensing and security boundaries + +The original Apache-2.0 `LICENSE`, Qdrant attribution, and third-party `NOTICE` +were retained without changes. A modified-file notice was added to the changed +source, and the README identifies the upstream revision and modifications. +Model weights have separate licenses, including restricted licenses mentioned +in `NOTICE`; the library's license does not replace them. + +GitHub's connector lacks a fork operation and the browser was signed out, so +the fork was created through GitHub's API using the existing Git Credential +Manager credential in memory. It was not printed, embedded in a URL, or saved +to a file. Git remotes use ordinary HTTPS URLs. Production applications and +databases were not accessed or modified. + +Kluster startup reported `connectionError`; automatic-review and +dependency-check tools were unavailable. No Kluster review result is claimed. +Testing, static checks, and the dependency-advisory audit are recorded separately. + +## Remaining limitations and next steps + +- Run the full online suite with all required licensed model assets and GPU + hardware, and the supported Python/OS matrix. The six real inference tests + cover one representative reranker, not every model family. +- The delivered diagnostics fixes do not sanitize all third-party HTTP + exceptions. Avoid indiscriminately logging raw exceptions or configuration; + a wider diagnostic audit would be a separate change. +- Monitor upstream PRs 691/797 and the CPU/GPU dependency and Hub-version + issues rather than applying broad compatibility changes without their tests. +- Keep the development tooling pin and advisory checks current. No throughput + improvement or memory-performance benchmark is claimed. +- Review this fork's branch before proposing an upstream contribution. No + upstream pull request or production deployment was made. diff --git a/README.md b/README.md index 206915a0..e864b3b4 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,88 @@ pip install fastembed pip install fastembed-gpu ``` +## Development of this fork + +This fork is based on [Qdrant FastEmbed](https://github.com/qdrant/fastembed) at +`d076f083519746d132888079e6e790f2193c9c08` (FastEmbed 0.9.0). It preserves +the upstream architecture, public API, dependency declarations, `poetry.lock`, +`LICENSE`, and `NOTICE`. + +### Install and test the source + +Python 3.10 or newer is required; this change was tested on Python 3.11.15 on +Windows using Poetry 2.5.1 and the existing dependency lockfile. + +```bash +git clone https://github.com/irulappan151204/fastembed.git +cd fastembed +git switch fix/model-download-resources +python -m pip install "poetry==2.5.1" +poetry install --only main,test,types +poetry run pytest tests/test_gcs_download.py tests/test_model_management.py tests/test_common.py tests/test_image_transform.py tests/test_parallel_processor.py +poetry run mypy fastembed --disallow-incomplete-defs --disallow-untyped-defs --disable-error-code=import-untyped +``` + +For linting, the upstream pre-commit hooks pin Ruff 0.3.4. With `uv` installed: + +```bash +uv tool run --from "ruff==0.3.4" ruff check --config pyproject.toml fastembed/common/model_management.py tests/test_gcs_download.py +uv tool run --from "ruff==0.3.4" ruff format --check --config pyproject.toml fastembed/common/model_management.py tests/test_gcs_download.py +``` + +The tests above use local HTTP fixtures and require no model weights, GPU, API +keys, paid services, or external model endpoints. The full suite is +`poetry run pytest`; it includes model downloads and hardware-dependent tests. +To run the representative reranker tests using CPU inference and the upstream +CI model-selection policy (which also cleans up downloaded test models): + +```bash +CI=1 poetry run pytest tests/test_text_cross_encoder.py +``` + +### Environment configuration + +FastEmbed accepts constructor arguments such as `cache_dir`, `threads`, +`providers`, and `local_files_only`. The following environment variables are +also useful: + +| Variable | Purpose | +| --- | --- | +| `FASTEMBED_CACHE_PATH` | Choose a writable directory for downloaded models. | +| `HF_HUB_OFFLINE=1` | Require cached models; uncached inference tests will fail. | +| `HF_ENDPOINT` | Select a trusted Hugging Face mirror if required. | +| `HF_TOKEN` | Optional authentication for restricted models; set privately and never commit it. | + +For example, in PowerShell: + +```powershell +$env:FASTEMBED_CACHE_PATH = Join-Path $env:TEMP "fastembed-models" +poetry run pytest tests/test_text_cross_encoder.py +``` + +In restricted Windows environments, a missing `PROCESSOR_ARCHITECTURE` can +cause Poetry to report `Could not parse version constraint`. Set that variable +to the actual architecture for the current process (`AMD64` on the tested +64-bit Windows host). If Documents disallows tool writes, put the checkout and +virtual environment in a writable development directory, or redirect tool +caches to a writable directory. `pytest -p no:cacheprovider` and Ruff's +`--no-cache` avoid writing their caches into the checkout. + +### Fixes in this fork + +- Streamed model-download responses now close on success and failure, including + HTTP errors, malformed length headers, and destination-open errors. Existing + exception types, timeouts, chunk sizes, and return values are preserved. +- The missing-length warning, fallback-download log, and missing-model-directory + error omit custom source URLs, which may contain signed access tokens. The + warning uses the existing Loguru logger and still explains disabled progress. +- Nine regression and compatibility tests use a real local HTTP server to + verify resource cleanup, unchanged file bytes, and safe diagnostics. + +These changes do not sanitize every third-party exception; callers should avoid +logging raw HTTP exceptions or sensitive configuration. Model weights have +their own licenses; see `NOTICE` and each model's license before using them. + ## 📖 Quickstart ```python diff --git a/fastembed/common/model_management.py b/fastembed/common/model_management.py index 583bef5a..44712498 100644 --- a/fastembed/common/model_management.py +++ b/fastembed/common/model_management.py @@ -1,3 +1,4 @@ +# Modified in this fork: close download streams and omit source URLs from diagnostics. import os import time import gzip @@ -126,7 +127,9 @@ def _get_model_description(cls, model_name: str) -> T: @classmethod def download_file_from_gcs(cls, url: str, output_path: str, show_progress: bool = True) -> str: """ - Downloads a file from Google Cloud Storage. + Downloads a file from Google Cloud Storage, closing its HTTP stream on every exit. + + Download warnings omit the URL because custom sources may contain credentials. Args: url (str): The URL to download the file from. @@ -137,37 +140,38 @@ def download_file_from_gcs(cls, url: str, output_path: str, show_progress: bool str: The path to the downloaded file. """ - response = requests.get(url, stream=True, timeout=(10, 120)) + with requests.get(url, stream=True, timeout=(10, 120)) as response: + # Handle HTTP errors + if response.status_code == 403: + raise PermissionError( + "Authentication Error: You do not have permission to access this resource. " + "Please check your credentials." + ) + # Otherwise an error page gets written out as though it were the archive. + response.raise_for_status() - # Handle HTTP errors - if response.status_code == 403: - raise PermissionError( - "Authentication Error: You do not have permission to access this resource. " - "Please check your credentials." - ) - # Otherwise an error page gets written out as though it were the archive. - response.raise_for_status() - - # Get the total size of the file - total_size_in_bytes = int(response.headers.get("content-length", 0)) - - # Warn if the total size is zero - if total_size_in_bytes == 0: - print(f"Warning: Content-length header is missing or zero in the response from {url}.") - - show_progress = bool(total_size_in_bytes and show_progress) - - with tqdm( - total=total_size_in_bytes, - unit="iB", - unit_scale=True, - disable=not show_progress, - ) as progress_bar: - with open(output_path, "wb") as file: - for chunk in response.iter_content(chunk_size=_DOWNLOAD_CHUNK_SIZE): - if chunk: # Filter out keep-alive new chunks - progress_bar.update(len(chunk)) - file.write(chunk) + # Get the total size of the file + total_size_in_bytes = int(response.headers.get("content-length", 0)) + + # Do not log signed URLs or credentials embedded in custom source URLs. + if total_size_in_bytes == 0: + logger.warning( + "Content-length header is missing or zero; download progress is disabled." + ) + + show_progress = bool(total_size_in_bytes and show_progress) + + with tqdm( + total=total_size_in_bytes, + unit="iB", + unit_scale=True, + disable=not show_progress, + ) as progress_bar: + with open(output_path, "wb") as file: + for chunk in response.iter_content(chunk_size=_DOWNLOAD_CHUNK_SIZE): + if chunk: # Filter out keep-alive new chunks + progress_bar.update(len(chunk)) + file.write(chunk) return output_path @classmethod @@ -583,7 +587,7 @@ def retrieve_model_gcs( model_tmp_dir = staging_dir / fast_model_name if not model_tmp_dir.is_dir() or model_tmp_dir.is_symlink(): raise ValueError( - f"The archive from {source_url} has no {fast_model_name} directory" + f"The archive for {model_name} has no {fast_model_name} directory" ) # Replace a stale empty model_dir, which Windows will not rename onto. rmdir leaves @@ -726,7 +730,7 @@ def download_model(cls, model: T, cache_dir: str, retries: int = 3, **kwargs: An ) except Exception: if not local_files_only: - logger.error(f"Could not download model from url: {url_source}") + logger.error(f"Could not download model {model.model} from its URL source") elif model.sources.deprecated_tar_struct: # Nothing is downloaded from the bucket anymore, but an old copy may still be cached. legacy_dir = Path(cache_dir) / f"fast-{model.model.split('/')[-1]}" diff --git a/tests/test_gcs_download.py b/tests/test_gcs_download.py new file mode 100644 index 00000000..1a303340 --- /dev/null +++ b/tests/test_gcs_download.py @@ -0,0 +1,185 @@ +"""Exercise streamed downloads over real HTTP without model downloads or credentials.""" + +import gzip +import threading +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from typing import Iterator + +import pytest +import requests +from loguru import logger + +from fastembed.common.model_management import ModelManagement +from fastembed.common.model_description import BaseModelDescription, ModelSource +from fastembed.common import model_management + + +@pytest.fixture +def download_server() -> Iterator[str]: + class Handler(BaseHTTPRequestHandler): + def do_GET(self) -> None: + route = self.path.split("?", 1)[0] + status = {"/forbidden": 403, "/missing": 404}.get(route, 200) + body = gzip.compress(b"\0" * 10240) if route == "/empty-archive" else b"model" + self.send_response(status) + if route == "/invalid-length": + self.send_header("Content-Length", "invalid") + elif route == "/zero-length": + self.send_header("Content-Length", "0") + elif route != "/no-length": + self.send_header("Content-Length", str(len(body))) + self.end_headers() + if route != "/zero-length": + self.wfile.write(body) + + def log_message(self, format: str, *args: object) -> None: + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +@pytest.fixture +def download_logs() -> Iterator[list[str]]: + messages: list[str] = [] + sink = logger.add(lambda message: messages.append(str(message))) + try: + yield messages + finally: + logger.remove(sink) + + +@pytest.fixture +def responses(monkeypatch: pytest.MonkeyPatch) -> Iterator[list[requests.Response]]: + """Capture real responses to observe their transport lifecycle, closing after failures.""" + captured: list[requests.Response] = [] + original_get = requests.get + + def get(*args, **kwargs): + response = original_get(*args, **kwargs) + captured.append(response) + return response + + monkeypatch.setattr(requests, "get", get) + try: + yield captured + finally: + for response in captured: + response.close() + + +@pytest.mark.parametrize( + ("route", "error"), + [ + ("/forbidden", PermissionError), + ("/missing", requests.HTTPError), + ("/invalid-length", ValueError), + ], +) +def test_failed_download_closes_response( + download_server: str, responses: list[requests.Response], tmp_path: Path, route, error +) -> None: + destination = tmp_path / "model.tar.gz" + destination.write_bytes(b"existing") + + with pytest.raises(error): + ModelManagement.download_file_from_gcs( + download_server + route, str(destination), show_progress=False + ) + + assert destination.read_bytes() == b"existing" + assert responses[0].raw.closed, "failed download left its HTTP stream open" + + +def test_destination_error_closes_response( + download_server: str, responses: list[requests.Response], tmp_path: Path +) -> None: + with pytest.raises(OSError): + ModelManagement.download_file_from_gcs( + download_server + "/model", str(tmp_path), show_progress=False + ) + + assert responses[0].raw.closed, "an unwritable destination left its HTTP stream open" + + +@pytest.mark.parametrize("route", ["/no-length", "/zero-length"]) +def test_download_warning_does_not_disclose_signed_url( + download_server: str, + tmp_path: Path, + capfd: pytest.CaptureFixture[str], + download_logs: list[str], + route: str, +) -> None: + destination = tmp_path / "model.tar.gz" + # This is a synthetic test marker, never a credential. + url = download_server + route + "?signature=synthetic-private-marker" + + assert ModelManagement.download_file_from_gcs(url, str(destination)) == str(destination) + + output = capfd.readouterr() + diagnostics = output.out + output.err + "".join(download_logs) + assert "synthetic-private-marker" not in diagnostics + assert "Content-length" in diagnostics + assert destination.read_bytes() == (b"model" if route == "/no-length" else b"") + + +def test_successful_download_keeps_bytes_and_closes_response( + download_server: str, responses: list[requests.Response], tmp_path: Path +) -> None: + destination = tmp_path / "model.tar.gz" + + assert ModelManagement.download_file_from_gcs( + download_server + "/model", str(destination), show_progress=False + ) == str(destination) + + assert destination.read_bytes() == b"model" + assert responses[0].raw.closed + + +def test_model_download_failure_does_not_log_signed_url( + download_server: str, + tmp_path: Path, + capfd: pytest.CaptureFixture[str], + download_logs: list[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + url = download_server + "/missing?signature=synthetic-private-marker" + model = BaseModelDescription( + model="test/model", + sources=ModelSource(url=url), + model_file="model.onnx", + description="Local HTTP fixture", + license="Apache-2.0", + size_in_GB=0.0, + ) + # This test exercises a loopback URL, independently of external model offline settings. + monkeypatch.delenv("HF_HUB_OFFLINE", raising=False) + monkeypatch.setattr(model_management.time, "sleep", lambda _: None) + + with pytest.raises(ValueError, match="Could not load model test/model"): + ModelManagement.download_model(model, str(tmp_path), retries=1) + + output = capfd.readouterr() + diagnostics = output.out + output.err + "".join(download_logs) + assert "synthetic-private-marker" not in diagnostics + assert "Could not download model" in diagnostics + + +def test_incomplete_model_archive_error_omits_signed_url( + download_server: str, tmp_path: Path +) -> None: + url = download_server + "/empty-archive?signature=synthetic-private-marker" + + with pytest.raises(ValueError) as error: + ModelManagement.retrieve_model_gcs("test/model", url, str(tmp_path)) + + assert "synthetic-private-marker" not in str(error.value) + assert "model directory" in str(error.value)