Skip to content

fix: remove max length - #264

Merged
stephantul merged 1 commit into
mainfrom
remove-max-length
Sep 8, 2026
Merged

fix: remove max length#264
stephantul merged 1 commit into
mainfrom
remove-max-length

Conversation

@stephantul

@stephantul stephantul commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

This PR sets the max length on the static model to None. This improves performance very slightly, but is a good fix nonetheless. I also had to update our mypy target version to 3.12. This doesn't have any consequences beyond mypy maybe missing typing errors for older python versions. But we'd still catch those in CI if they are causing breakage.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/semble/index/dense.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The runtime change appears safe to merge, but the non-blocking compatibility-check and lockfile consistency issues should be corrected.

The model2vec call is valid for the locked dependency and implements the stated truncation change; remaining findings concern weakened Python 3.10/3.11 static checking and inconsistent lock-generation metadata rather than an established runtime failure.

Files Needing Attention: pyproject.toml, uv.lock

Comments Outside Diff (1)

  1. uv.lock, line 16 (link)

    P2 Lock Configuration Is Inconsistent

    The regenerated lock records a three-day dependency exclusion window, while pyproject.toml still specifies one week. This inconsistency can cause avoidable lockfile changes during later validation or regeneration. Regenerate the lock using the committed project configuration.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "fix: remove max length" | Re-trigger Greptile

Comment thread pyproject.toml

[tool.mypy]
python_version = "3.10"
python_version = "3.12"

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 Mypy Target Weakens Compatibility

The package still supports and advertises Python 3.10 and 3.11, but mypy now analyzes the source as Python 3.12. This allows code using 3.12-only typing APIs to pass mypy and potentially fail for supported users on an untested path. Keep the mypy target aligned with the minimum supported Python version, or add a separate Python 3.10 compatibility check.

Suggested change
python_version = "3.12"
python_version = "3.10"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why did you think I changed it though? Genuine question. Did you think I changed this for fun?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You’re right — I didn’t think you changed it for fun. I focused on the compatibility implication of the diff and framed it as though the version change lacked justification, even though the PR description explicitly explains why it was needed and that CI covers breakage. That was a poor reading of the context on my part. The narrower point is only that a 3.12 mypy target no longer statically checks 3.10/3.11-specific typing behavior, but given your CI coverage and the intentional tradeoff, this comment isn’t actionable and should be dismissed.

@stephantul
stephantul merged commit 8e52ea7 into main Sep 8, 2026
17 checks passed
@stephantul
stephantul deleted the remove-max-length branch September 8, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant