Skip to content

Conversation

JKamsker
Copy link
Collaborator

This pull request introduces a comprehensive overhaul to the project's build, test, versioning, and release automation, as well as updates to project documentation and target frameworks. The changes modernize the CI/CD pipeline with GitHub Actions, standardize versioning using GitVersion, update project files to target .NET 8, and provide new documentation for contributors. Additionally, minor code improvements and new utility projects are included.

CI/CD and Versioning Automation:

  • Added multiple GitHub Actions workflows for CI, prerelease publishing, release publishing, and version tagging, automating build, test, packaging, and deployment processes (.github/workflows/ci.yml, .github/workflows/publish-prerelease.yml, .github/workflows/publish-release.yml, .github/workflows/tag-version.yml). [1] [2] [3] [4]
  • Introduced GitVersion for semantic versioning, including configuration files and MSBuild integration (GitVersion.yml, .config/dotnet-tools.json, Directory.Build.props). [1] [2] [3]

Project Structure and Documentation:

  • Added AGENTS.md with detailed repository guidelines covering project structure, build/test commands, coding standards, testing, commit/PR practices, and versioning.
  • Added a new sample project LiteDB.RollbackRepro for reproducible test scenarios.

Target Framework and Build Updates:

  • Updated all major project files to target .NET 8 (net8.0) and set language version to latest where appropriate, removing legacy framework targets (LiteDB.Benchmarks/LiteDB.Benchmarks.csproj, LiteDB.Shell/LiteDB.Shell.csproj, LiteDB.Stress/LiteDB.Stress.csproj). [1] [2] [3]

Code Quality and Testing Improvements:

  • Marked test and report threads as background threads in stress test execution to improve shutdown behavior (LiteDB.Stress/Test/TestExecution.cs). [1] [2]
  • Enabled disk write failure simulation in both debug and testing builds (ConsoleApp1/Program.cs).
  • Added missing test utility namespace import to LiteDB.Tests/Database/AutoId_Tests.cs.

DavidNemecek and others added 30 commits September 20, 2025 13:48
…tions-and-upgrade-.net

Modernize build targets and CI/CD
…handling' into codex/consolidate-test-database-handling
…ueryplan-for-composite-sorting

; Conflicts:
;	LiteDB.Tests/Internals/Sort_Tests.cs
@Copilot Copilot AI review requested due to automatic review settings September 28, 2025 09:36
@JKamsker JKamsker marked this pull request as draft September 28, 2025 09:37
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request implements a comprehensive modernization of LiteDB's development and release infrastructure while introducing multi-field sorting capabilities to the query engine. The changes include migration from AppVeyor to GitHub Actions, introduction of GitVersion for semantic versioning, target framework updates to .NET 8, and significant enhancements to the ORDER BY functionality supporting multiple sort keys.

Key changes:

  • Complete CI/CD modernization with GitHub Actions workflows for builds, testing, and automated releases
  • Implementation of multi-field ORDER BY support with new ThenBy/ThenByDescending methods
  • Project structure improvements including new test utilities and updated target frameworks

Reviewed Changes

Copilot reviewed 101 out of 102 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
.github/workflows/ New CI/CD workflows for automated testing, prerelease publishing, release publishing, and version tagging
GitVersion.yml GitVersion configuration for semantic versioning with branch-specific policies
LiteDB/Engine/Query/ Enhanced query engine with multi-field sorting support via new OrderBy, SortKey, and SortContainer classes
LiteDB/Client/Database/ Extended public API with ThenBy/ThenByDescending methods for multi-field sorting
LiteDB.Tests/ Comprehensive test coverage for new sorting functionality and updated test infrastructure
LiteDB/LiteDB.csproj Updated target frameworks to netstandard2.0 and net8.0, removed legacy framework support

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 48 to +51
_disk = disk;
_order = order;
if (orders == null) throw new ArgumentNullException(nameof(orders));
if (orders.Count == 0) throw new ArgumentException("Orders must contain at least one segment", nameof(orders));

Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

The null check should be performed before assigning _disk to follow fail-fast principles and avoid partial object initialization in case of argument validation failure.

Copilot uses AI. Check for mistakes.

Comment on lines +13 to +23
: base(values?.Select(x => x ?? BsonValue.Null) ?? throw new ArgumentNullException(nameof(values)))
{
if (orders == null) throw new ArgumentNullException(nameof(orders));

_orders = orders as int[] ?? orders.ToArray();

if (_orders.Length != this.Count)
{
throw new ArgumentException("Orders length must match values length", nameof(orders));
}
}
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

There's a potential race condition or logic error: this.Count depends on the base constructor completing with the transformed values, but if the values enumerable is consumed multiple times with different results, the length validation could be incorrect. Consider storing the values array first and validating lengths before calling the base constructor.

Suggested change
: base(values?.Select(x => x ?? BsonValue.Null) ?? throw new ArgumentNullException(nameof(values)))
{
if (orders == null) throw new ArgumentNullException(nameof(orders));
_orders = orders as int[] ?? orders.ToArray();
if (_orders.Length != this.Count)
{
throw new ArgumentException("Orders length must match values length", nameof(orders));
}
}
: base((values == null ? throw new ArgumentNullException(nameof(values)) : values.Select(x => x ?? BsonValue.Null)).ToArray())
{
if (orders == null) throw new ArgumentNullException(nameof(orders));
_orders = orders as int[] ?? orders.ToArray();
// Since we materialized values above, we can safely use this.Count
if (_orders.Length != this.Count)
{
throw new ArgumentException("Orders length must match values length", nameof(orders));
}

Copilot uses AI. Check for mistakes.

Comment on lines +79 to +89
public static SortKey FromBsonValue(BsonValue value, IReadOnlyList<int> orders)
{
if (value is SortKey sortKey) return sortKey;

if (value is BsonArray array)
{
return new SortKey(array.ToArray(), orders);
}

return new SortKey(new[] { value }, orders);
}
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

When value is already a SortKey, returning it directly ignores the provided orders parameter, which could lead to incorrect sorting behavior if the existing SortKey has different ordering requirements than expected.

Copilot uses AI. Check for mistakes.

Comment on lines +157 to +167
protected IEnumerable<BsonDocument> OrderBy(IEnumerable<BsonDocument> source, OrderBy orderBy, int offset, int limit)
{
var keyValues = source
.Select(x => new KeyValuePair<BsonValue, PageAddress>(expr.ExecuteScalar(x, _pragmas.Collation), x.RawId));
var segments = orderBy.Segments;

using (var sorter = new SortService(_tempDisk, order, _pragmas))
if (segments.Count == 1)
{
sorter.Insert(keyValues);
var segment = segments[0];
var keyValues = source
.Select(doc => new KeyValuePair<BsonValue, PageAddress>(segment.Expression.ExecuteScalar(doc, _pragmas.Collation), doc.RawId));

LOG($"sort {sorter.Count} keys in {sorter.Containers.Count} containers", "SORT");
using (var sorter = new SortService(_tempDisk, new[] { segment.Order }, _pragmas))
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

For single-segment sorting, creating a new array new[] { segment.Order } for each method call is inefficient. Consider caching common single-element arrays or passing the order directly to avoid unnecessary allocations.

Copilot uses AI. Check for mistakes.

Comment on lines 108 to 114
public ILiteQueryable<T> OrderBy(BsonExpression keySelector, int order = Query.Ascending)
{
if (_query.OrderBy != null) throw new ArgumentException("ORDER BY already defined in this query builder");
if (_query.OrderBy.Count > 0) throw new ArgumentException("Multiple OrderBy calls are not supported. Use ThenBy for additional sort keys.");

_query.OrderBy = keySelector;
_query.Order = order;
_query.OrderBy.Add(new QueryOrder(keySelector, order));
return this;
}
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

The error message suggests using ThenBy for additional sort keys, but the restriction prevents multiple OrderBy calls entirely. This breaks the fluent API pattern where multiple OrderBy calls could reasonably reset the sort criteria. Consider allowing multiple OrderBy calls by clearing the existing OrderBy list instead of throwing an exception.

Copilot uses AI. Check for mistakes.

Comment on lines +10 to +11
<MaxCpuCount>1</MaxCpuCount> <!-- Single-threaded to avoid resource contention -->
<DisableParallelization>true</DisableParallelization>
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

Setting MaxCpuCount to 1 and disabling parallelization may significantly slow down CI builds. Consider using a higher CPU count (e.g., 2-4) while still maintaining stability, as completely disabling parallel execution eliminates the benefits of modern CI infrastructure.

Suggested change
<MaxCpuCount>1</MaxCpuCount> <!-- Single-threaded to avoid resource contention -->
<DisableParallelization>true</DisableParallelization>
<MaxCpuCount>4</MaxCpuCount> <!-- Use 4 CPUs for parallel test execution -->

Copilot uses AI. Check for mistakes.

Introduces vector search capabilities for AI/ML applications like semantic search and RAG.

- Adds a new `BsonVector` type (`float[]`) and an HNSW-inspired index for fast Approximate Nearest Neighbor (ANN) search.
- Supports Cosine, Euclidean, and Dot Product distance metrics.
- Exposes a new fluent query API via `TopKNear()` and `WhereNear()` extensions.
- Adds SQL/`BsonExpression` support with the `VECTOR_SIM()` function.
- Includes a new demo project (`LiteDB.Demo.Tools.VectorSearch`) for a complete, end-to-end example using Google Gemini embeddings.
* Add issue 2561 repro console app

* Add ReproRunner CLI and migrate Issue 2561 repro (#2659)

* Add repro runner CLI and transaction monitor repro

* Refine repro runner UI with Spectre.Console

* Refactor repro runner CLI to Spectre.Console.Cli

* Update RunCommand to enhance output table with detailed repro status and versioning

* Move rollback repro into ReproRunner (#2661)

* Add shared messaging utilities to ReproRunner (#2662)

* Add shared messaging utilities for repro runner

* move repro run

* Refactor repro runner build and execution pipeline (#2663)

* Refactor repro runner build and execution pipeline

* Document ReproRunner public APIs

* Improve repro run logging display

* Serialize repro run log updates

* Throttle run UI refresh rate

* Update default fps

* Fix Issue_2586_RollbackTransaction repro

* Add suppression for console log output in ReproExecutor

* Enhance table rendering by enabling expansion for better layout in RunCommand

* Enhance RunCommand to track and display build and execution states in real-time

* Add readme to sln

* Enforce repro configuration handshake (#2668)

* Add state-aware repro evaluation and JSON reporting (#2667)

* Implement state-aware repro evaluation and reporting

* Refresh run command table with overall status (#2669)

* Refresh run table columns

* Refactor run command UI for CI

* Align overall repro status with evaluation results (#2671)

* Align overall repro status with evaluation

* Update display logic in the reprorunner run

* Add repro for DiskService disposal failure (#2689)
Add GitHub release script for creating prerelease tags & disable build for no-code changes.
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.

2 participants