feat(artifacts): repository-artifact layer — artifacts, dependencies, config keys; graph 2.2.0 - #206
Merged
Merged
Conversation
…it parseManifest's catch Each of the four DocumentBuilderFactory switches is now backed by a test that fails when that specific switch alone is removed: disallow-doctype-decl via the existing behavioural probe, the two external-entity features via a getFeature() config assertion, and FEATURE_SECURE_PROCESSING via a dedicated test (getFeature() cannot attribute it -- it reads true on an untouched factory regardless of whether setFeature was ever called). parseManifest now gives pom.xml its own try/catch narrowed to the three checked types it declares, and no catch at all on the Gradle branch, so a future bug in either no longer risks being misreported under the other branch's failure contract.
Keeps the from-disk helper written exactly once, per the design plan's constraint, instead of task 6 recreating the same three lines in CodeAnalyzer.java the way the python reference duplicates it across dependencies.py and core.py.
- guard the yaml branch against a top-level scalar document (not null, not a Map, not a List) minting a spurious key="" entry - give the compose env-dual-mint a delimiter distinct from CanId.configKeyId's "@key/", so it can no longer collide with a plain yaml dotted path that legitimately starts with "env." - dedupe the ENV/ARG dockerfile scanners into one scanner plus two small per-directive tail parsers
…structor CanId.configKeyEnvDualMintId(artifactId, bareKey) now owns the "@key:env/" delimiter and the collision-freedom rationale, matching the eight other can:// id constructors already funneled through CanId. ConfigKeys.java:157 calls it instead of concatenating the string itself, alongside the existing configKeyId call for the dockerfile arg. case one call earlier. ArtifactModelTest gains a case next to configKeyIdNestsUnderItsArtifact asserting the new shape and that it cannot collide with configKeyId for the same key name.
….2.0 Fills in the neutral Artifact/Package labels V2SchemaCatalog already reserved, adds ConfigKey, and emits HAS_ARTIFACT/DEFINES_CONFIG/ DECLARES_DEPENDENCY/LOCKS. This is the Neo4j half of #197 that was deferred on the premise that the Java projector ran off a legacy model and could not carry a v2 addition -- that stopped being true once the v2 graph projection shipped, so the deferral is withdrawn. Package nodes are graph-only (minted from CanId.purlMaven); analysis.json continues to carry only the bare group/name, matching python. Nodes and containment edges are un-prefixed -- cross-language merge targets, same reasoning python's schema.py already states at its own declaration. DECLARES_DEPENDENCY carries a `_k`=kind MERGE discriminant via the L4 overlay's keyedEdge machinery, since one manifest may declare a package under two kinds. LOCKS fans out to every lock artifact present, since lock pins are merged upstream with no per-lock attribution -- a known limitation carried over from codeanalyzer-python as-is. codeanalyzer-python deliberately did not bump its own schema version for this layer ("no consumers yet"), so a consumer cannot detect the layer's presence from python's version alone. Java bumps to 2.2.0 anyway, the better behaviour and a deliberate divergence from python here.
Two gaps found in review of the graph contract 2.2.0 work: - schema.neo4j.json (via V2SchemaCatalog.uniquenessConstraints()) promised artifact_id/package_id/configkey_id uniqueness constraints, but Schema.CONSTRAINTS -- the hand-maintained list CypherWriter and BoltWriter actually execute -- never gained them, so no load ever created what the document promised. Added the same three, verbatim matching the auto-derived names/alias so the document and the executed DDL agree exactly, not merely both exist. - CypherWriter's wipe traversal couldn't reach :Artifact/:ConfigKey at all: HAS_ARTIFACT hangs off :JApplication directly (not off the :JModule/:JCompilationUnit the wipe's first hop was scoped to), so a repeat push of the same application after a config-bearing artifact was removed left the old :Artifact and its :ConfigKey rows as permanent orphans -- the same bug class the v1/v2 unification already guards against, recreated for this new label family. Fixed by folding HAS_ARTIFACT into the app-anchor's first hop and DEFINES_CONFIG into DESCENDANTS, reusing the existing wipe mechanism rather than adding a second one. :Package stays unreachable from the wipe on purpose, same as :JPackage/:JAnnotation -- a purl-keyed Package is a cross-app merge target, not owned by one application's push. Verified live: pushed a fixture into a throwaway Neo4j 5 container, removed a config-bearing artifact from the analyzed repo, pushed again -- the removed Artifact and its ConfigKeys are gone (0 orphans), the still-present pom.xml/Package survive untouched.
… guards The prior fix (a025f6c) widened CypherWriter's wipe to reach :Artifact/ :ConfigKey so a re-push wouldn't leave them stale. That was wrong: both are un-prefixed cross-language merge targets by design (CanId.artifactId's own javadoc -- the `artifact` id segment exists precisely so a sibling-language analyzer over the same repository lands on the same node). When it does, its own edges attach to that shared node. A Java wipe that reaches it DETACH DELETEs everything on it, including that other analyzer's edges, and recreates only what Java itself declares -- silently destroying data in a tool Java cannot see or repair. Reverted DESCENDANTS and the wipe's first hop to their pre-widening form (functionally identical to 26150a5, comments extended to name all three shared labels -- Package, Artifact, ConfigKey -- and the reason). This restores the tradeoff already accepted for Package/JPackage/ JAnnotation: a stale Artifact/ConfigKey survives a re-push after its file is removed from the repo, recoverable with a full re-push or a separate sweep. That is the safe direction versus the alternative of occasionally corrupting a sibling tool's graph with no way to detect it. Two tests added, both proven to catch what they guard against (verified by temporarily reintroducing the removed code/line and confirming each fails, then restoring): - wipeStaysOffTheCrossLanguageArtifactPackageSubgraph: the negative counterpart to the existing "wipe covers both generations" test -- asserts the app-anchor hop stays exactly J_HAS_UNIT|J_HAS_MODULE and DESCENDANTS never contains HAS_ARTIFACT/DEFINES_CONFIG/ DECLARES_DEPENDENCY/LOCKS, so widening this again requires deliberately overriding an explicit assertion, not just missing a gap. - constraintsStayInSyncWithTheCatalog: every (merge_label, key) pair V2SchemaCatalog.uniquenessConstraints() derives (what schema.neo4j.json promises) has a semantically matching entry in Schema.CONSTRAINTS (what CypherWriter/BoltWriter actually execute) -- the exact document-vs-DDL drift the previous review round found and fixed by hand, now guarded so a future schema addition can't reintroduce it silently.
…es not have Four documentation defects in the repository-artifact layer, all of the same class: prose that survived from an early spec instead of describing what the code emits. - ArtifactDiscovery's class javadoc told a maintainer that later tasks read `source` back out of the returned artifacts "so that later reading never has to touch the filesystem again". That is the exact inverse of the binding constraint: `source` is empty under --no-artifact-text and a truncated prefix past --artifact-text-max-bytes, so extraction reads from disk via DependencyView.readFromDisk. Two adversarial tests already fence the behaviour; the front-door javadoc was inviting the regression they guard. - JArtifact listed roles "build, ci, deploy, config, dependency-lock" and called them user-assigned. They are rule-assigned by ArtifactDiscovery's RULES table, and the emitted vocabulary is dependency-manifest, tool-config, container-image, service-topology, iac, ci, env, legal, docs, script, unknown. - JDependency promised ecosystems "maven, npm, gradle, pypi, golang"; there is no setEcosystem caller in the tree, so the field is always "maven". Matches the reference's candour about "pypi" being the only one it emits. - JApplication called the `dependencies` List "indexed by" a purl; it is one entry per declaration, sorted by (name, declaredIn). - ArtifactDiscovery's classify() comment claimed "k8s/*.yml" matches only directly under k8s/, contradicting globMatches' own correct comment thirteen lines down. The code is right (fnmatch semantics: '*' crosses '/'), so "k8s/base/svc.yml" matches too and the comment was the wrong half.
Every deliberate divergence on this branch is stated in a comment where it occurs; this one was not, and it is the one that matters most. configKeyEnvDualMintId builds `<artifact-id>@key:env/<NAME>`. The reference builds `<artifact-id>@key/env.<NAME>` (config_keys.py:577 into ids.py:39). Ours is the correct grammar and stays: the reference's provably self-collides, because a plain yaml dotted path can itself begin with the segment `env.`, so one document with both a top-level `env:` block and a compose environment block yields two records under one id. The reference guarded the bare-name collision and missed the dotted-path one. The javadoc already argued why a prefixed key routed through configKeyId would be unsound, but never said the reference does exactly that, nor named the price: ConfigKey is an un-prefixed cross-language merge target, so in a polyglot repo the two analyzers currently mint different nodes for the same environment variable, splitting the very nodes that label exists to share. That stands until the reference converges. Grammar unchanged.
…ct-text form Closes three test blind spots in the repository-artifact layer. LOCKS is a per-package fact, so a coordinate declared in two manifests and pinned by one lockfile walks the mint twice for one (lock, package) pair. The reference guards that with a `seen` set (neo4j/project.py:350-359) because its RowBuilder.finish() only sorts. Ours dedupes edges by (type, from, to, _k) there, so the repeat mint already collapses and a guard would be dead code -- an assertion, not a belief: with that dedupe disabled the new test reports 2 rows, with it 1. The test pins the deduped-bag contract at the observable boundary whichever layer keeps it, and asserts the DECLARES_DEPENDENCY half too: it stays one row per declaring manifest and must never be collapsed per package. The divergence from the reference's implementation is now stated where it occurs. The artifact-layer edge tests asserted only `to`, props and key, never `from` -- one-sided coverage of the kind that already let a wrong decision ship on this layer. All four types now assert both endpoints. Verified by mutation: reversing each of the four fails, and so do two wrong-but-plausible sources (LOCKS from the declaring manifest, DECLARES_DEPENDENCY from a wrong artifact id) that only the new `from` assertions can catch. Nothing exercised a bare --artifact-text. Its fallbackValue = "true" exists only because picocli 4.1.0 resolves the bare negatable form to the opposite of what negatable=true implies, so a cleanup of that "redundant" attribute would silently invert a user-visible flag. The new CLI test asserts the bare form matches the no-flag default, and separately that capture is genuinely on -- equality alone would also hold if both runs captured nothing, which is exactly what the inverted flag produces. Removing fallbackValue fails it.
This was referenced Aug 31, 2026
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
1 participant
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.
Closes #197, a child of epic codellm-devkit/.github#45. Plan: docs/design/plans/2026-08-29-repository-artifact-layer.md.
What lands
An inventory of every non-source file in a repository, the dependencies declared in them, and the configuration keys they define — emitted into
analysis.jsonand projected into the Neo4j graph.JArtifact/JDependency/JConfigKey, anchored on the application as sibling maps sosymbol_tablestays strictly code-keyed. Config keys nest inside their owning artifact.pom.xmlorDockerfile. Classification is a first-match-wins glob table, and no file is ever dropped: unmatched files are inventoried astext/unknown, undecodable ones asbinary. The only exception is a.javafile no rule names, which the symbol table owns.pom.xmlthrough a hardened DOM, a deliberately shallow Gradle reader, andgradle.lockfilereconciliation. Net-new: dependencies were previously only downloaded as jars, never modelled..properties, YAML, XML descriptors, Dockerfiles and.envfiles, with${...}references recognised but not resolved.Artifact,PackageandConfigKeywithHAS_ARTIFACT,DEFINES_CONFIG,DECLARES_DEPENDENCYandLOCKS. This is the part Repository-artifact layer for java — artifact / dependency / config_key nodes #197 originally deferred;V2SchemaCataloghad already reserved the first two labels.--artifact-text/--no-artifact-textand--artifact-text-max-bytes.It emulates codeanalyzer-python v1.3.0, not the epic spec
Deliberately, and on explicit instruction. The spec mandates an
artifact_kindenum, ascopeenum and a shared ecosystem mapping table; none of the three exist in the reference implementation — its entire package contains oneEnum, unrelated. What ships there is a bareformatstring plus a many-valuedroleslist, a four-valuekind, and a hardcoded ecosystem literal. That contradiction is filed as codellm-devkit/.github#48 and needs resolving there, not here.Four places Java cannot be literal, each stated where it occurs rather than left to be discovered:
providedandsystemscopes have no slot in the four-valuekind; both map tobuild, and<optional>wins over the scope mapping — which matches the reference, where every optional group maps tooptionalregardless of its name.JDependencycarriesgroupadditively and the purl ispkg:maven/<group>/<artifact>.provides_importsandunresolved_importsare not implemented at all. Roughly half the reference's dependency machinery exists to bridge the gap between a PyPI distribution name and its import name. Java packages are declared, so a heuristic there would be strictly worse than emitting nothing.One deliberate improvement on the reference: our config-key id grammar for compose-style env vars is
@key:env/<NAME>where the reference uses@key/env.<NAME>. The reference's form provably self-collides when a plain YAML path begins with the segmentenv.. SinceConfigKeyis a cross-language merge target, the two analyzers will mint different nodes for the same compose file until the reference converges — recorded inCanId's javadoc and being raised on #48.Verification
Full suite green apart from
CodeAnalyzerIntegrationTest, which needs a Docker daemon and fails identically onmain. Beyond the suite: the graph was loaded into a throwaway Neo4j 5 container twice — once confirming all four edge types non-zero with no dangling endpoints and_kpresent onDECLARES_DEPENDENCYbut notLOCKS, and once confirming a re-push after removing a file leaves no orphans while the sharedPackagenode survives.Three things worth a reviewer's attention, all of which the review chain caught rather than shipped:
disallow-doctype-declrejects a DOCTYPE-bearing payload before any entity is consulted, andgetFeature(FEATURE_SECURE_PROCESSING)reportstrueeven when never set.Artifact,PackageorConfigKey. An earlier iteration widened it to stop stale nodes after a file is removed; that would haveDETACH DELETEd nodes a sibling analyzer shares, destroying its edges. The staleness is the accepted tradeoff and matches the reference, which accepts the same. A negative-invariant test now pins it.source, which may be empty or truncated depending on the capture flag. The helper exists exactly once, in contrast to the reference's own duplicated pair, and two adversarial tests fence it.Follow-ups, not in this PR
config_use/J_USES_CONFIG(the reference's detector mechanism is call-shaped and has no purchase on Spring's@Valueannotation injection); POM parent and BOM-import inheritance; alibs.versions.tomlparser, without which a version catalog's dependencies are invisible while the artifact reports clean extraction;${VAR:default}yielding no reference, which the reference also gets wrong; and per-entry degradation so one unreadable file cannot abort a run.Separately, this branch surfaced a pre-existing gap it did not fix: the conformance schema's
localIdpattern does not match the compound body-node keysSdgVerticesemits, so an L4 payload does not validate against this repo's own schema. Filed separately.