Skip to content

Conversation

@amotl
Copy link
Member

@amotl amotl commented Nov 15, 2025

About

Just maintenance, and an attempt for a CI stability improvement.

References

@coderabbitai
Copy link

coderabbitai bot commented Nov 15, 2025

Walkthrough

Updates CI matrix to test newer OTP/Elixir versions and switches to expanded array formatting, relaxes the postgrex version constraint, and alters a test to use vector-similarity ordering with LIMIT 3 and updated parameters/expected IDs.

Changes

Cohort / File(s) Summary
CI / Workflow Configuration
​.github/workflows/lang-elixir-postgrex.yml
Updates CI matrix OTP versions from ['26','27']['27','28'], Elixir versions from ['1.17','1.18']['1.18','1.19'], and converts the version lists to an expanded multiline array format.
Dependency Management
by-language/elixir-postgrex/mix.exs
Changes postgrex dependency constraint from ~> 0.21.0 to ~> 0.21 (broader 0.21.x range).
Tests
by-language/elixir-postgrex/test/basic_test.exs
Replaces plain ID sort with KNn / VECTOR_SIMILARITY ordering and LIMIT 3; expands query parameters to two vectors and updates expected result IDs to [42, 44, 45].

Sequence Diagram(s)

sequenceDiagram
  participant Test as BasicTest
  participant DB as PostgreSQL
  rect rgb(232,245,233)
    Note over Test,DB: New behavior — KNn vector ordering with LIMIT 3
    Test->>DB: Execute KNn query (2 vectors, ORDER BY VECTOR_SIMILARITY, LIMIT 3)
    DB-->>Test: Return top 3 rows (IDs: 42, 44, 45)
    Test->>Test: Assert returned IDs == [42, 44, 45]
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review points:
    • Confirm CI matrix values and multiline array formatting match intended combinations.
    • Verify postgrex constraint change aligns with project dependency policy.
    • Validate test vector query parameters, ordering clause, and expected IDs.

Poem

🐇 I hopped through code with a curious nose,

Vectors sorted, three friends chose.
CI refreshed and versions grown,
Postgres loosened, seeds newly sown.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: upgrading to Erlang/OTP 28 and Elixir 1.19, which align with the workflow and dependency updates in the changeset.
Description check ✅ Passed The description is related to the changeset, explaining it as maintenance work aimed at CI stability improvement with a reference to GH-1251.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch elixir-fix-flakyness

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c05670f and 7cea6ad.

📒 Files selected for processing (1)
  • .github/workflows/lang-elixir-postgrex.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1257
File: .github/workflows/lang-csharp-npgsql.yml:85-100
Timestamp: 2025-11-15T16:07:11.693Z
Learning: In the cratedb-examples repository CI workflows for .NET/Npgsql testing, conditional branches in test scripts (such as for "latest" versions) are intentionally kept even when not currently in the matrix, to allow easy toggling on/off by modifying the matrix values without changing the script logic.
📚 Learning: 2025-11-15T16:07:11.693Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1257
File: .github/workflows/lang-csharp-npgsql.yml:85-100
Timestamp: 2025-11-15T16:07:11.693Z
Learning: In the cratedb-examples repository CI workflows for .NET/Npgsql testing, conditional branches in test scripts (such as for "latest" versions) are intentionally kept even when not currently in the matrix, to allow easy toggling on/off by modifying the matrix values without changing the script logic.

Applied to files:

  • .github/workflows/lang-elixir-postgrex.yml
🔇 Additional comments (1)
.github/workflows/lang-elixir-postgrex.yml (1)

42-49: Matrix update correctly addresses previous review feedback.

The changes update OTP from ['26', '27'] to ['27', '28'] and Elixir from ['1.17', '1.18'] to ['1.18', '1.19'], and crucially, the ineffective exclusion rule referencing the obsolete '1.17' version has been removed. Since erlef/setup-beam resolves version strings like '1.18' to the latest available patch (>= 1.18.4, which supports OTP 28), all resulting combinations are compatible:

  • OTP 27 + Elixir 1.18 ✓
  • OTP 27 + Elixir 1.19 ✓
  • OTP 28 + Elixir 1.18 ✓ (safe via auto-resolved patch)
  • OTP 28 + Elixir 1.19 ✓

The multiline array formatting also improves readability. Based on learnings from the codebase, conditional branches in CI workflows (like "latest" version handling) are often kept for easy toggling without script changes.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

By using `ORDER BY VECTOR_SIMILARITY()` within the vector storage
integration test, we hope for a better sorting stability, i.e.
less flaky outcomes.
- Elixir 1.17 is no longer supported on Erlang/OTP 28.
- Relax Postgrex dependency to 0.21.
@amotl amotl linked an issue Nov 15, 2025 that may be closed by this pull request
@amotl amotl marked this pull request as ready for review November 15, 2025 18:01
coderabbitai[bot]

This comment was marked as resolved.

@amotl amotl requested review from seut and surister November 15, 2025 18:11
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16ff431 and c05670f.

📒 Files selected for processing (1)
  • .github/workflows/lang-elixir-postgrex.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1257
File: .github/workflows/lang-csharp-npgsql.yml:85-100
Timestamp: 2025-11-15T16:07:11.693Z
Learning: In the cratedb-examples repository CI workflows for .NET/Npgsql testing, conditional branches in test scripts (such as for "latest" versions) are intentionally kept even when not currently in the matrix, to allow easy toggling on/off by modifying the matrix values without changing the script logic.
📚 Learning: 2025-11-15T16:07:11.693Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1257
File: .github/workflows/lang-csharp-npgsql.yml:85-100
Timestamp: 2025-11-15T16:07:11.693Z
Learning: In the cratedb-examples repository CI workflows for .NET/Npgsql testing, conditional branches in test scripts (such as for "latest" versions) are intentionally kept even when not currently in the matrix, to allow easy toggling on/off by modifying the matrix values without changing the script logic.

Applied to files:

  • .github/workflows/lang-elixir-postgrex.yml
📚 Learning: 2025-07-23T22:00:51.593Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:44-48
Timestamp: 2025-07-23T22:00:51.593Z
Learning: In the cratedb-examples repository, hard-coded credentials like "crate:crate" in Docker Compose files are acceptable for demonstration purposes to maintain simplicity and avoid unnecessary layers of indirection, even when flagged by security tools like Checkov.

Applied to files:

  • .github/workflows/lang-elixir-postgrex.yml
🪛 actionlint (1.7.8)
.github/workflows/lang-elixir-postgrex.yml

52-52: value "1.17" in "exclude" does not match in matrix "elixir-version" combinations. possible values are "1.18", "1.19"

(matrix)

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.

CI: Elixir integration tests are flaky

2 participants