New cache strategy: two-path AMD resolution with in-memory caching#8
Draft
robotdan wants to merge 11 commits intosavant-build:mainfrom
Draft
New cache strategy: two-path AMD resolution with in-memory caching#8robotdan wants to merge 11 commits intosavant-build:mainfrom
robotdan wants to merge 11 commits intosavant-build:mainfrom
Conversation
Introduces a specification to rework dependency caching so that only AMD files are stored in the project-local .savant/cache while JARs, POMs, and sources are resolved from the shared ~/.m2/repository. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Key changes from v1: - Switch from disk-cached AMDs to in-memory generation on every build - Document the stale AMD mapping problem (MavenTools.toSavantDependencies bakes build file mappings into AMD XML at generation time) - Add cost/benefit analysis for in-memory vs disk-cached approach - Add mapping strategy audit plan (Phase 4) - Add performance benchmarking plan (Phase 5) - Expand multi-repo analysis confirming only savant-dependency-management and savant-core need code changes - Comprehensive test plan with 62 test cases - Note maven-bridge may be deprecated Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tifacts Key refinement: distinguish between Savant-native and Maven-sourced artifacts: - Savant-native AMDs are authoritative (publisher-created), cached globally in ~/.savant/cache. No stale-mapping problem. - Maven-sourced AMDs are generated in-memory per build from POMs using project-specific semanticVersions mappings. Never persisted to disk. - JARs are always global: ~/.m2 for Maven, ~/.savant/cache for Savant-native. - Per-project .savant/cache directory eliminated entirely. Resolve open question savant-build#3 (Savant repo handling). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ctory - AMD schema version: not applicable, we're not changing the AMD format - CacheProcess default dir: change from .savant/cache (per-project) to ~/.savant/cache (global), converging with integrationDir - Integration JARs already live in ~/.savant/cache, no change needed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Refactor Workflow.fetchMetaData() to use two-path strategy: 1. Try pre-built AMD file (Savant-native artifacts) 2. Generate AMD in-memory from POM (Maven artifacts, no disk write) - Add in-memory AMD cache (per Workflow instance) to avoid re-parsing - Add in-memory POM cache to avoid re-parsing shared parent/BOM POMs - Change CacheProcess default dir from .savant/cache to ~/.savant/cache - Eliminates stale AMD cache problem where changing semanticVersions mappings required manual cache deletion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 task
- Add fetchMetaData_performance test that times POM parse + translate for vertx-core (complex case with parent chain + BOM imports). First call: ~358ms, cached call: <1ms. - Fix stale comment on amdCache: key is "group:project:name:version" not "group:project:version" - Update spec status to IMPLEMENTED - IN REVIEW Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document actual timing from fetchMetaData_performance test: 358ms for complex vertx-core (worst case), <1ms for cache hits. Confirms the theoretical 200-400ms delta estimate is conservative. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New tests added to WorkflowTest: Savant-native AMD path (two-path resolution): - fetchMetaData_savantNativeAmd: pre-built AMD fetched from Savant repo, cached on disk, in-memory cache returns same instance - fetchMetaData_savantNativeAmd_withDependencies: Savant AMD with compile and runtime dependency groups parsed correctly - fetchMetaData_twoPathResolution_savantThenMaven: single workflow resolves Savant-native artifact (AMD on disk) and Maven artifact (AMD in-memory) Error paths: - fetchMetaData_missingAmdAndPom: ArtifactMetaDataMissingException thrown when neither AMD nor POM can be found - fetchMetaData_missingAmdInSavantRepo_noPomFallback: missing-amd fixture with no Maven fallback throws ArtifactMetaDataMissingException POM cache: - fetchMetaData_pomCacheSharesParents: two vertx artifacts sharing parent POM chain both resolve correctly (proves POM cache reuse works) Negative source cache: - fetchSource_negativeCacheShortCircuit: .neg marker created for missing source, second call returns null immediately via short-circuit Workflow configuration variants: - fetchMetaData_cacheOnlyWorkflow: Savant-only workflow (CacheProcess + URLProcess, no Maven) resolves Savant-native artifacts - fetchMetaData_mavenOnlyWorkflow: Maven-only workflow (MavenCacheProcess + MavenProcess, no Savant) resolves via POM-to-AMD translation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
Code Review: Test Coverage AdditionsAfter a thorough code review, the following test gaps were identified and addressed in commit 5d31c34: Savant-native AMD path (the "other half" of two-path resolution)
Error paths
POM cache verification
Negative source cache
Workflow configuration variants
All 86 tests pass (0 failures). |
Remove javadoc and section separator comments to match existing test style. Add inline comments inside test bodies instead. Add two new tests: AMD cache key includes artifact name (regression), and non-semantic version POM fallback path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Test style cleanup + 2 additional tests (ad9fb50) Style changes:
New tests added:
All 88 tests pass (0 failures). The 1 config failure is pre-existing ( |
Document why HashMap is safe (single-threaded resolution pipeline) and add an IMPORTANT note at the POM cache-put site warning that all POM mutations must complete before caching, since POM is mutable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Workflow.fetchMetaData()with a two-path AMD resolution strategy:~/.savant/cache)semanticVersionsmappings (never persisted to disk)Workflowinstance) to avoid redundant parsing within a single buildCacheProcessdefault directory from.savant/cache(per-project) to~/.savant/cache(global per-user), aligning with the existingintegrationDirdefaultsemanticVersionsmappings in the build file required manually deleting.savant/cacheCompanion PRs
Test plan
fetchMetaData_inMemoryCache- verifies same instance returned on second callfetchMetaData_mappingsAppliedFresh- verifies stale-cache fix (different mappings produce different results)mavenCentral/mavenCentralMapping- verifies AMD files NOT written to disk for Maven artifactsGroovyBuildFileParserTestin savant-core (companion PR)🤖 Generated with Claude Code