Skip to content

ci: try to make tests more stable#5789

Merged
Xuanwo merged 1 commit intomainfrom
luban/adapt-height
Jan 22, 2026
Merged

ci: try to make tests more stable#5789
Xuanwo merged 1 commit intomainfrom
luban/adapt-height

Conversation

@Xuanwo
Copy link
Copy Markdown
Collaborator

@Xuanwo Xuanwo commented Jan 22, 2026

This PR will try to make tests more stable


Parts of this PR were drafted with assistance from Codex (with gpt-5.2) and fully reviewed and edited by me. I take full responsibility for all changes.

@github-actions github-actions Bot added python ci Github Action or Test issues labels Jan 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

PR Review

Summary: Test stability improvements for FTS result comparison + uv lockfile refresh.

P1 Issues

1. Potential bug in sort fallback logic (test_scalar_index.py:2409-2411)

When _rowid is not present, the fallback sorts by all columns except _score. However, if the DataFrame has only a _score column (unlikely but possible), sort_cols becomes an empty list. While the if sort_cols: guard prevents an error, it means no sorting happens, which could cause test flakiness in edge cases.

2. dtype=object conversion may mask type mismatches (test_scalar_index.py:2446-2449)

Converting all non-score columns to dtype=object before comparison is convenient but loses type information. The original code handled object vs non-object columns differently. If there's a legitimate type mismatch between single-machine and distributed results, this change would silently ignore it.

Consider keeping type-aware comparison or at least logging/asserting dtype equality:

assert single_df[col].dtype == distributed_df[col].dtype, f"Column {col} dtype mismatch"

Minor Observations

  • The lockfile changes are routine (uv adding upload-time metadata). The large diff is just metadata, no package version changes.
  • The kind="mergesort" choice is correct for stable sorting.

Overall the test stability improvements are reasonable, just the type-handling concern above warrants consideration.

Copy link
Copy Markdown
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

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

looks good to me!

@Xuanwo Xuanwo merged commit 50999bd into main Jan 22, 2026
13 of 14 checks passed
@Xuanwo Xuanwo deleted the luban/adapt-height branch January 22, 2026 18:03
majin1102 pushed a commit to majin1102/lance that referenced this pull request Jan 23, 2026
This PR will try to make tests more stable

---

**Parts of this PR were drafted with assistance from Codex (with
`gpt-5.2`) and fully reviewed and edited by me. I take full
responsibility for all changes.**
jackye1995 pushed a commit to jackye1995/lance that referenced this pull request Jan 23, 2026
This PR will try to make tests more stable

---

**Parts of this PR were drafted with assistance from Codex (with
`gpt-5.2`) and fully reviewed and edited by me. I take full
responsibility for all changes.**
jackye1995 pushed a commit that referenced this pull request Jan 23, 2026
This PR will try to make tests more stable

---

**Parts of this PR were drafted with assistance from Codex (with
`gpt-5.2`) and fully reviewed and edited by me. I take full
responsibility for all changes.**
vivek-bharathan pushed a commit to vivek-bharathan/lance that referenced this pull request Feb 2, 2026
This PR will try to make tests more stable

---

**Parts of this PR were drafted with assistance from Codex (with
`gpt-5.2`) and fully reviewed and edited by me. I take full
responsibility for all changes.**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Github Action or Test issues python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants