feat: Update embedding logic to bulk

[BenjaminMichaelis]

Description

Describe your changes here.

Fixes #Issue_Number (if available)

Ensure that your pull request has followed all the steps below:

• Code compilation
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary

[Copilot]

Pull request overview

This PR refactors markdown chunk handling and embedding generation to support bulk embedding and a staging-table swap workflow intended to keep the live vector collection available during rebuilds.

Changes:

• Introduces a MarkdownChunk model and updates chunking/output/tests to use Heading + ChunkText.
• Adjusts markdown preprocessing to preserve paragraph separators (blank lines) for paragraph-aware chunking.
• Updates embedding upload to batch-generate embeddings and load into a staging collection before swapping it into place in PostgreSQL.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
EssentialCSharp.Chat/Program.cs	Updates chunk stats/output to use MarkdownChunk fields.
EssentialCSharp.Chat.Tests/MarkdownChunkingServiceTests.cs	Updates assertions for the new chunk model (ChunkText).
EssentialCSharp.Chat.Shared/Services/MarkdownChunkingService.cs	Preserves blank lines for paragraph-aware splitting and emits MarkdownChunk instances.
EssentialCSharp.Chat.Shared/Services/FileChunkingResult.cs	Adds MarkdownChunk record and changes FileChunkingResult.Chunks type accordingly.
EssentialCSharp.Chat.Shared/Services/EmbeddingService.cs	Implements batch embedding + staging-then-swap upload strategy using Npgsql.
EssentialCSharp.Chat.Shared/Services/ChunkingResultExtensions.cs	Updates conversion to BookContentChunk using MarkdownChunk and adds deterministic IDs + ChunkIndex.
EssentialCSharp.Chat.Shared/Services/AISearchService.cs	Adds heading-based deduplication of vector search results.
EssentialCSharp.Chat.Shared/Models/BookContentChunk.cs	Adds ChunkIndex as stored metadata for chunks.

[Copilot]

The embedding generator call batches all chunks into a single request, but Azure OpenAI embeddings have an input-count limit (noted in the comment as 2048). If the book produces more than that many chunks, this will fail at runtime. Consider chunking texts into batches (<= provider limit) and merging the returned vectors back into chunkList (or throw a clear exception when the limit is exceeded).

Suggested change

	// ── Step 2: Batch-embed all chunks in a single API call ───────────────────────
	// IEmbeddingGenerator.GenerateAsync natively accepts IEnumerable<string>.
	// The single-string overload used previously is a convenience extension method
	// that wraps one item and calls this same method.
	var chunkList = bookContents.ToList();
	var texts = chunkList.Select(c => c.ChunkText).ToList();
	GeneratedEmbeddings<Embedding<float>> embeddings =
	await embeddingGenerator.GenerateAsync(texts, cancellationToken: cancellationToken);
	if (embeddings.Count != chunkList.Count)
	throw new InvalidOperationException(
	$"Embedding count mismatch: expected {chunkList.Count}, got {embeddings.Count}.");
	for (int i = 0; i < chunkList.Count; i++)
	{
	chunkList[i].TextEmbedding = embeddings[i].Vector;
	// ── Step 2: Batch-embed all chunks in provider-safe API calls ─────────────────
	// Azure OpenAI embeddings impose an input-count limit per request.
	// Process the texts in batches and merge the returned vectors back into the
	// original chunk list to preserve ordering.
	var chunkList = bookContents.ToList();
	var texts = chunkList.Select(c => c.ChunkText).ToList();
	const int maxEmbeddingBatchSize = 2048;
	for (int batchStart = 0; batchStart < texts.Count; batchStart += maxEmbeddingBatchSize)
	{
	int batchSize = Math.Min(maxEmbeddingBatchSize, texts.Count - batchStart);
	List<string> batchTexts = texts.GetRange(batchStart, batchSize);
	GeneratedEmbeddings<Embedding<float>> embeddings =
	await embeddingGenerator.GenerateAsync(batchTexts, cancellationToken: cancellationToken);
	if (embeddings.Count != batchSize)
	throw new InvalidOperationException(
	$"Embedding count mismatch for batch starting at index {batchStart}: expected {batchSize}, got {embeddings.Count}.");
	for (int i = 0; i < batchSize; i++)
	{
	chunkList[batchStart + i].TextEmbedding = embeddings[i].Vector;
	}

[Copilot]

These SQL statements interpolate collectionName/derived names directly into identifier-quoted SQL. If collectionName can be influenced outside trusted code, this becomes identifier-injection (quotes can be escaped/broken). Consider restricting collectionName to a safe identifier regex (e.g., letters/digits/underscore) before composing SQL, and use Npgsql's identifier-quoting helper to build the final identifiers consistently.

[Copilot]

The method documentation says the swap uses "three SQL RENAMEs", but the implementation performs two RENAME operations (live→old, staging→live) plus DROP TABLE statements. Update the comments so they accurately describe the actual DDL being executed.

Suggested change

	/// 4. Atomically swap staging → live via three SQL RENAMEs in a single transaction.
	/// PostgreSQL ALTER TABLE acquires AccessExclusiveLock automatically; no explicit
	/// LOCK TABLE is needed. The transaction ensures no reader sees an intermediate state.
	/// 5. Drop the old live backup table.
	/// 4. Atomically swap tables in a single transaction using two SQL RENAME operations
	/// (live → old, staging → live). PostgreSQL ALTER TABLE acquires
	/// AccessExclusiveLock automatically; no explicit LOCK TABLE is needed. The
	/// transaction ensures no reader sees an intermediate state.
	/// 5. Drop the old live backup table with DROP TABLE.

[Copilot]

GenerateBookContentEmbeddingsAndUploadToVectorStore now contains significant new behavior (batch embedding + staging/swap workflow) but there are no unit/integration tests covering it in the Chat test suite. Adding tests that at least verify batching semantics (e.g., embedding generator called with N inputs and staging collection UpsertAsync invoked once with embeddings assigned) would help prevent regressions; the swap step can be isolated behind an interface to make it testable without a live database.

[Copilot]

Local variable name candidates_list uses snake_case, which is inconsistent with the surrounding camelCase naming in this method (e.g., vectorSearchOptions, searchVector). Rename to candidatesList (or similar) for consistency and readability.

[BenjaminMichaelis]

Good overall approach — bulk embedding, deterministic IDs, and the staging-swap pattern are all solid improvements. A few things worth addressing before merging:

Must fix:

1. candidates_list (AISearchService.cs) violates C# camelCase convention — should be candidatesList.
2. NpgsqlDataSource? is nullable/optional in the constructor but GenerateBookContentEmbeddingsAndUploadToVectorStore throws immediately if null. This is a DI anti-pattern — either require it in the constructor or split into two classes. As-is, the service compiles and resolves but explodes only when the method is called.
3. Azure OpenAI batch limit (~2048 inputs) is mentioned in the summary comment but not enforced. A large book could silently exceed this and fail at runtime — consider batching internally.

Should fix:
4. SQL RENAME statements use raw string interpolation with collectionName, which is caller-controlled. Even though it currently comes from a constant, consider asserting/validating the name only contains safe characters (e.g., alphanumeric + underscore) to prevent accidental SQL issues.
5. If staging.UpsertAsync(chunkList, cancellationToken) throws, the staging table is left behind. Consider wrapping in try/catch and deleting staging on failure.

Nit:
6. ExtractChapterNumber silently returning null for non-chapter files is a meaningful behavioral change from the previous InvalidOperationException — worth a code comment noting this is intentional and that callers handle null.