-
Notifications
You must be signed in to change notification settings - Fork 2.9k
build(dbgpt-ext): relax spacy pin to >=3.8 for Python 3.13 support (#3006) #3185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| import pytest | ||
|
|
||
| from dbgpt.core import Chunk | ||
| from dbgpt.rag.text_splitter.text_splitter import ( | ||
| CharacterTextSplitter, | ||
|
|
@@ -63,3 +65,26 @@ def test_character_text_splitter_empty_doc() -> None: | |
| output = splitter.split_text(text) | ||
| expected_output = ["db", "gpt"] | ||
| assert output == expected_output | ||
|
|
||
|
|
||
| def test_spacy_text_splitter() -> None: | ||
| """Smoke test SpacyTextSplitter against the installed spacy version. | ||
|
|
||
| Guards the spacy>=3.8 upgrade (Python 3.13 support): verifies the public | ||
| spacy API used by SpacyTextSplitter (spacy.load / nlp(text).sents) still | ||
| works. Skips when spacy or the en pipeline is unavailable so the suite | ||
| stays green without the optional ``rag`` extra installed. | ||
| """ | ||
| pytest.importorskip("spacy", reason="spacy (rag extra) not installed") | ||
|
|
||
| from dbgpt.rag.text_splitter.text_splitter import SpacyTextSplitter | ||
|
|
||
| try: | ||
| splitter = SpacyTextSplitter(pipeline="en_core_web_sm", chunk_size=1000) | ||
| except Exception as e: # pragma: no cover - depends on model download | ||
| pytest.skip(f"spacy pipeline unavailable: {e}") | ||
|
Comment on lines
+84
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Files matching test_splitters.py:"
fd -a 'test_splitters\.py$' . || true
echo "Relevant lines:"
for f in $(fd 'test_splitters\.py$' .); do
echo "--- $f"
sed -n '70,100p' "$f" | cat -n
done
echo "Search SpacyTextSplitter tests:"
rg -n "SpacyTextSplitter|spacy|BLE001|skip\(f" packages/dbgpt-core/src/dbgpt packages -g '*.py' | head -n 120 || true
echo "Ruff config references:"
(fd 'pyproject\.toml|ruff\.toml|\.ruff\.toml$' . | xargs -r sed -n '1,220p' | rg -n "ruff|select|ignore|BLE001|flake8-blind-except|flake8-blind-except" -C 3 || true)Repository: eosphoros-ai/DB-GPT Length of output: 7028 🌐 Web query:
💡 Result: The Ruff rule BLE001 (blind-except) is designed to identify and flag overly broad exception handling, specifically Citations:
Let unknown The current 🧰 Tools🪛 Ruff (0.16.1)[warning] 84-84: Do not catch blind exception: (BLE001) Sources: Path instructions, Linters/SAST tools |
||
|
|
||
| text = "This is the first sentence. Here is the second one. And a third." | ||
| output = splitter.split_text(text) | ||
| assert output, "SpacyTextSplitter returned no chunks" | ||
| assert "first sentence" in " ".join(output) | ||
|
Comment on lines
+87
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate file and tests =="
git ls-files | rg 'test_splitters\.py|splitter|text_splitter' | head -200
echo
echo "== relevant test section =="
sed -n '1,150p' packages/dbgpt-core/src/dbgpt/rag/text_splitter/tests/test_splitters.py
echo
echo "== locate SpacyTextSplitter implementation =="
rg -n "class SpacyTextSplitter|def _merge_splits|chunk_size|merge_splits" packages/dbgpt-core/src/dbgpt/rag text 2>/dev/null | head -200Repository: eosphoros-ai/DB-GPT Length of output: 11735 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== SpacyTextSplitter implementation relevant lines =="
sed -n '351,388p' packages/dbgpt-core/src/dbgpt/rag/text_splitter/text_splitter.py
sed -n '452,590p' packages/dbgpt-core/src/dbgpt/rag/text_splitter/text_splitter.py
sed -n '690,715p' packages/dbgpt-core/src/dbgpt/rag/text_splitter/text_splitter.py
echo
echo "== merge_splits implementation for SpacyTextSplitter (around 697-747) =="
sed -n '697,748p' packages/dbgpt-core/src/dbgpt/rag/text_splitter/text_splitter.py
echo
echo "== deterministic behavior for sample text under SpacyTextSplitter semantics =="
python3 - <<'PY'
samples = [
"This is the first sentence. Here is the second one. And a third.",
"A. B. C. " + "x" * 200 + ". B. C.",
]
separator = "\n\n"
overlap = 0
chunk_size = 1000
for text in samples:
sent_text = text
merged_text = []
buffer = []
current = []
total = 0
for s in sent_text.splitlines(keepends=False):
s = s.strip()
# actual sent splitting path from code:
nlp = type('Fake', (), {"sentences": sent_text.split(". ")})()
# emulate relevant TextSplitter.merge_chunks_1 implementation with len(s) and separator joining.
nlp = type('Fake', (), {})()
nlp.sentences = iter((s for s in sent_text.split(". ") if s.strip()))
current = []
buffer = []
total = 0
for s in nlp.sentences:
s = s.strip()
if s:
current.append(s)
print("len(sentences):", len(current))
print("sentences:", current)
print("len(separator.join(current)):", len(separator.join(current)))
print("single_chunk_with_current_chunk_size:", len(separator.join(current)) <= chunk_size)
PYRepository: eosphoros-ai/DB-GPT Length of output: 10699 Assert sentence boundaries, not only non-empty output. With Source: Path instructions |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,9 @@ build-backend = "hatchling.build" | |
|
|
||
| [project.optional-dependencies] | ||
| rag = [ | ||
| "spacy==3.7", | ||
| # spacy 3.8+ is compiled with Cython 3 and ships wheels for Python 3.13. | ||
| # Pin below 4.0 to avoid picking up a future major release with API changes. | ||
| "spacy>=3.8,<4.0", | ||
|
Comment on lines
+29
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)(pyproject\.toml|uv\.lock)$|test_splitters\.py$' | sed -n '1,120p'
echo
echo "== target pyproject =="
sed -n '1,120p' packages/dbgpt-ext/pyproject.toml 2>/dev/null || true
echo
echo "== related test references =="
fd -a test_splitters.py . | while read -r f; do
echo "--- $f"
sed -n '60,85p' "$f"
done
echo
echo "== search spaCy references =="
rg -n 'spacy|spaCy|3\.8\.7|3\.8\.6|3\.8\.' pyproject.toml packages -S | sed -n '1,220p'Repository: eosphoros-ai/DB-GPT Length of output: 50376 🌐 Web query:
💡 Result: spaCy version 3.8.7 was released to provide official support for Python 3.13 [1][2]. This support required compiling spaCy with Cython 3 and restructuring parts of the codebase, specifically moving factory function implementations to spacy.pipeline.factories and catalog registrations to spacy.registrations to manage import-time behavior changes associated with Python 3.13 [1][2]. Regarding spaCy version 3.8.6, it was not removed from existence but rather marked as a release that explicitly removed Python 3.13 compatibility [2][3]. The release notes for v3.8.6 specifically describe its purpose as to "Restore wheels, remove Python 3.13 compatibility" and to restore wheel support for ARM platforms [3]. Version 3.8.7 subsequently reintroduced and finalized the support for Python 3.13 [1][2]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== git status short =="
git status --short
echo
echo "== diff stat =="
git diff --stat || true
git diff --cached --stat || true
echo
echo "== spaCy references (focused) =="
rg -n 'spacy|spaCy|3\.8\.7|3\.8\.6|3\.8\.' packages pyproject.toml uv.lock --glob '!**/package-lock.json' --glob '!**/*.js' --glob '!**/*.jsx' --glob '!**/*.ts' --glob '!**/*.tsx' --glob '!**/*.md' --glob '!**/*.json' -SRepository: eosphoros-ai/DB-GPT Length of output: 21140 Raise the spaCy lower bound to 3.8.7.
Source: Path instructions 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
lockfile="$(fd --hidden --type f '^uv\.lock$' . | head -n1)"
test -n "$lockfile"
uv lock --check
rg -n -C 2 'name = "spacy"|version = ' "$lockfile"Repository: eosphoros-ai/DB-GPT Length of output: 197 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf 'Repository files of interest:\n'
git ls-files | rg '(^|/)(uv\.lock|pyproject\.toml|uv\.)$' || true
printf '\nWorkspace pyproject relevant dependency snippets:\n'
rg -n -C 3 'spacy|extra|extras' pyproject.toml packages/dbgpt-ext/pyproject.toml packages/dbgpt-ext 2>/dev/null || true
printf '\nTop-level lockfile package entries around spacy:\n'
if [ -f uv.lock ]; then
rg -n -C 8 'name = "spacy"|package name = "spacy"|spacy>=' uv.lock
else
echo "uv.lock not found at repository root"
fi
printf '\nPackage-level lockfile package entries around spacy:\n'
if [ -f packages/dbgpt-ext/uv.lock ]; then
rg -n -C 8 'name = "spacy"|package name = "spacy"|spacy' packages/dbgpt-ext/uv.lock
else
echo "packages/dbgpt-ext/uv.lock not found"
fiRepository: eosphoros-ai/DB-GPT Length of output: 50377 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
lock = Path("uv.lock").read_text(encoding="utf-8")
# Extract spaCy package block and the optional extra graph blocks.
blocks = []
current = None
for i, line in enumerate(lock.splitlines(), 1):
if re.match(r'^\[\[package\]\]$', line):
if current:
blocks.append(current)
current = {"name": "UNKNOWN", "version": "", "lines": [], "start": i}
if current:
if line.startswith("name = "):
current["name"] = line.split("=", 1)[1].strip().strip('"')
elif line.startswith("version = "):
current["version"] = line.split("=", 1)[1].strip().strip('"')
if current and line.startswith("[[package]]"):
pass
if current:
current["lines"].append((i, line))
spa = next((b for b in blocks if b["name"] == "spacy"), None)
print("spaCy package found:", bool(spa))
if spa:
print("spaCy version:", spa["version"])
print("spaCy line range:", f"{spa['start']}-" + str(spa['lines'][-1][0]))
for ln, text in spa["lines"]:
if ln <= spa["start"] + 30 or ln >= spa["lines"][-1][0] - 20:
print(f"{ln}: {text}")
for section in ("rag", "extras"):
print(f"\n[{section}] sections:")
found = False
for start, line in enumerate(lock.splitlines(), 1):
if line == f"[options.{section}]":
found = True
print(start, line)
for ln, text in enumerate(lock.splitlines()[start:start+70], start):
if text.strip().startswith("packages") or "spacy" in text.lower():
print(f"{ln}: {text}")
if text.strip() and text.strip().startswith("[") and not text.strip().startswith("[["):
assert ln > start+1
break
break
if not found:
print(f"missing {section}")
# Validate that spaCy version in lock satisfies ">=3.8,<4.0" using semantic ints.
if spa:
v = re.split(r"[._-]", spa["version"].split("+")[0].split("-")[0])
try:
v = tuple(int(part) for part in v)
except Exception as e:
v = None
print("\nParsed spaCy version tuple:", v)
if v:
print("satisfies >=3.8,<4.0:", ((3, 8) <= v < (4, 0)))
PYRepository: eosphoros-ai/DB-GPT Length of output: 6445 Update
Source: Path instructions |
||
| "markdown", | ||
| "bs4", | ||
| "python-pptx", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: eosphoros-ai/DB-GPT
Length of output: 14876
Keep the Spacy smoke test offline before construction.
SpacyTextSplitter.__init__callsspacy.cli.download(pipeline)before the constructor returns when the model is missing, soSpacyTextSplitter(pipeline="en_core_web_sm", ...)can start a real network download beforepytest.skip. Checkspacy.util.is_package("en_core_web_sm")or otherwise confirm model availability before constructing the splitter, or move this test to an explicit model-dependent integration test.🧰 Tools
🪛 Ruff (0.16.1)
[warning] 84-84: Do not catch blind exception:
Exception(BLE001)
Source: Path instructions