Modernize Gradle, add hybrid vector search and GitHub ingestion

[WilliamAGH]

This pull request introduces several structural and standards improvements across the project, focusing on build/configuration hygiene, development workflow, and coding standards documentation. The changes clean up obsolete files, centralize configuration, enhance Makefile maintainability, and significantly expand the coding standards in AGENTS.md.

Key changes:

Build and Configuration Hygiene:

• Removed .dockerignore, .gitattributes, and .pre-commit-config.yaml as part of streamlining and centralizing configuration management. This reduces redundancy and potential confusion by consolidating build and formatting logic elsewhere. [1][2][3]
• Updated the Dockerfile to move static analysis configs (pmd, spotbugs) into the config/ directory, and added JVM flags for native access and memory safety. Also removed an unused environment variable. [1][2][3]

Development Workflow Improvements:

• Refactored the Makefile to source shared logic and variables from config/make/common.mk, greatly reducing duplication and centralizing environment handling, argument building, and port management. New targets were added for more granular doc/repo processing. [1][2][3][4]
• Updated health checks and utility scripts to use the new default port variable, improving consistency and maintainability.

Documentation and Coding Standards:

• Expanded AGENTS.md with new and clarified rules, especially around naming discipline (banning generic/abbreviated names, enforcing intent-revealing identifiers), Git safety, and explicit root cause handling. Added new rule sections and extended existing ones for stricter agent guidance. [1][2][3][4][5]
• Updated README.md with a new banner image and improved project description for clarity and marketing.

Summary of most important changes:

Build/configuration cleanup:

• Removed obsolete or redundant files: .dockerignore, .gitattributes, .pre-commit-config.yaml. [1][2][3]
• Moved static analysis configs to config/ and improved Docker JVM flags. [1][2]

Makefile and workflow enhancements:

• Refactored Makefile to use shared includes, centralized environment/argument logic, and added new doc/repo processing targets. [1][2][3][4]

Coding standards and documentation:

• Major expansion and clarification of rules in AGENTS.md, especially for naming, Git behavior, and error handling. [1][2][3][4][5]
• Improved README.md with a new banner and clearer project summary.

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

frontend/src/lib/services/sse.ts (1)

85-125: ⚠️ Potential issue | 🟠 Major

Stop using fallback patterns for invalid SSE payloads.
validateWithSchema correctly logs validation failures via logZodFailure() with full context, but the SSE code ignores these failures and falls back to unvalidated data (status/error messages, text) or silently skips callbacks (citations, providers). This violates the rule against fallback paths and swallowing validation errors. When an SSE payload fails validation, it should fail fast with an error callback, not degrade to raw eventData.

🛠️ Example fix pattern

 if (normalizedType === SSE_EVENT_STATUS) {
const parsed = tryParseJson(eventData, source)
const validated = validateWithSchema(StreamStatusSchema, parsed, `${source}:status`)
- callbacks.onStatus?.(validated.success ? validated.validated : { message: eventData })+ if (!validated.success) {+ callbacks.onError?.({ message: 'Invalid stream status payload' })+ return+ }+ callbacks.onStatus?.(validated.validated)
return
}

Apply similarly to error, citation, provider, and text branches.

🤖 Fix all issues with AI agents

In `@frontend/src/lib/services/streamRecovery.ts`:
- Around line 86-104: The top-of-file doc comments for buildStreamRetryStatus
and buildStreamRecoverySucceededStatus merely restate the function names; remove
or replace them with short "why" style comments (or delete entirely) that
explain the UX intent only (e.g., "Shown to users when streaming temporarily
fails to indicate a retry" for buildStreamRetryStatus and "Shown when a retry
succeeds and streaming resumes" for buildStreamRecoverySucceededStatus) so the
code remains self-documenting while preserving rationale; locate the comments
above the functions buildStreamRetryStatus and
buildStreamRecoverySucceededStatus and update or remove them accordingly.
In `@scripts/lib/github_identity.sh`:
- Around line 91-96: The CANONICAL_COLLECTION_NAME assignment can silently
produce malformed names if encode_collection_segment fails; change the code to
capture each encoded segment into temporary vars (e.g., owner_enc and name_enc)
by calling encode_collection_segment "$REPOSITORY_OWNER" and
encode_collection_segment "$REPOSITORY_NAME", check their exit status and/or
that they are non-empty, and if either call failed log an error and exit
non‑zero instead of continuing; only build CANONICAL_COLLECTION_NAME from the
validated owner_enc and name_enc values to ensure no empty segments are used.
- Line 212: The condition uses an unset-sensitive parameter expansion (if [ -n
"$REPO_URL" ]; then) which will fail under set -u; change the test to use a safe
default expansion such as ${REPO_URL:-} so it reads like the existing if but
with ${REPO_URL:-} to avoid nounset errors when REPO_URL is undefined (update
the if condition referencing REPO_URL accordingly).
In `@src/main/java/com/williamcallahan/javachat/service/HybridSearchService.java`:
- Around line 257-261: In the hashCode method, rename the generic local variable
`result` to an intent-revealing name such as `hash`: update the declaration and
subsequent usage so the computation uses `int hash =
java.util.Objects.hash(sparseVector, retrievalFilter);` and `hash = 31 * hash +
java.util.Arrays.hashCode(denseVector);` (ensure the method still returns the
renamed variable) so references to `sparseVector`, `retrievalFilter`, and
`denseVector` remain unchanged.
- Around line 164-176: Replace the parallel-list fan-out (collectionNames +
futures) with a single Map<String, CompletableFuture<List<ScoredPoint>>> that
maps each collection name to its future (created via
buildHybridQueryRequest(...),
QdrantListenableFutureBridge.toCompletableFuture(...) and
qdrantClient.queryAsync(...)); then refactor collectFanOutResults to accept that
map plus the existing scoredPointsByUuid and collectionFailures (reducing the
positional parameters below five) and update the other occurrence around lines
305-339 similarly so both call sites and the collectFanOutResults signature use
the map-based input to avoid index coupling.
- Line 393: Rename the private record ScoredResult to a domain-specific name
(e.g., ScoredPointMatch) and update all references to it (constructor usages,
variable declarations, return types, and pattern matches) to the new identifier;
ensure the declaration line `private record ScoredResult(String id, double
score, ScoredPoint point, String collection)` is changed to `private record
ScoredPointMatch(...)` and adjust any code in methods that construct or consume
ScoredResult so compilation and intent-revealing naming are preserved.
- Around line 53-99: The HybridSearchService constructor currently accepts six
parameters; refactor it to accept a single QueryEncodingServices parameter
instead of the three encoding params so the constructor signature becomes four
arguments (QdrantClient, QueryEncodingServices, AppProperties,
Optional<QdrantGitHubCollectionDiscovery>) and update field assignment to use
the injected QueryEncodingServices directly; also register/provide a
QueryEncodingServices bean (or factory) where the service is wired so callers
supply the composed record (constructed from EmbeddingClient,
LexicalSparseVectorEncoder, QdrantRetrievalConstraintBuilder) rather than
passing those three individually.
In `@src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java`:
- Around line 184-205: Extract the literal "url" into a single named constant
(e.g. private static final String URL_PAYLOAD_KEY = "url") in
HybridVectorService and replace all inline occurrences — the List.of("url") in
ScrollPoints building, the retrievedPoint.getPayloadMap().get("url") lookup, and
any other usages around lines 308-325 — with that constant; ensure
imports/visibility match the class scope and run tests to confirm no behavior
change.

🧹 Nitpick comments (2)

src/main/java/com/williamcallahan/javachat/service/GuidedLearningService.java (2)

141-159: Consider skipping enrichment when no book docs.
If bookDocuments is empty, returning emptyEnrichment() avoids ungrounded hints and saves the extra enrichment call. Tiny fun: no ingredients, no recipe.

♻️ Suggested short‑circuit

 List<Document> retrievedDocuments = retrievalService.retrieve(query);
List<Document> bookDocuments = filterToBook(retrievedDocuments);
+ if (bookDocuments.isEmpty()) {+ return emptyEnrichment();+ }
List<String> snippets = bookDocuments.stream()

---

236-273: Cache check could reduce repeat generation.
If the cache is intended to save LLM work, consider checking getCachedLessonMarkdown(slug) before starting retrieval/streaming. Tiny fun: reheating leftovers beats cooking again.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java`:
- Around line 270-301: In doUpsert, add defensive checks after calling
EmbeddingBatchEmbedder.embedDocuments to ensure embeddings is non-null, has the
same size as documents, and that no embedding entry is null before building
points; if sizes mismatch or nulls are found, either filter out the
corresponding Document(s) (and log which were dropped) or throw a clear
IllegalStateException so the upsert is aborted. Specifically validate the
embeddings list returned by embeddingClient via
EmbeddingBatchEmbedder.embedDocuments, adjust the loop that builds points
(HybridVectorPointFactory.HybridVectorSet / HybridVectorPointFactory.buildPoint)
to only process aligned pairs, and avoid calling qdrantClient.upsertAsync with
mismatched counts.

🧹 Nitpick comments (1)

src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java (1)

178-189: Optional cleanup: Objects.requireNonNull(List.of(...)) is redundant.
Tiny tidbit: removing tiny redundancies keeps the hot path extra crisp.

♻️ Proposed tidy-up

- List<String> urlPayloadFields = Objects.requireNonNull(List.of(URL_PAYLOAD_FIELD), "urlPayloadFields");+ List<String> urlPayloadFields = List.of(URL_PAYLOAD_FIELD);