feat: l9-codegraph — CodeGraph + PlanGraph dual-graph engine - #6
Open
cryptoxdog wants to merge 2 commits into
Open
feat: l9-codegraph — CodeGraph + PlanGraph dual-graph engine#6cryptoxdog wants to merge 2 commits into
cryptoxdog wants to merge 2 commits into
Conversation
…n-repo template - CodeGraph engine: multi-language tree-sitter AST parser (Python/TS/JS/JSX/TSX) - CodeGraphBuilder: repo-scoped DETACH DELETE + CodeDef/INVOKES Neo4j writes - EgoGraphRetriever: pure Cypher ego-graph search, 1-2 hops, no APOC - PlanGraph engine: RPG-inspired service constellation graph - PlanGraphBuilder: constellation-scoped upsert (PlanService/PlanInterface/FLOWS_TO/DEPENDS_ON) - PlanGraphRetriever: service neighborhood search + Kahn's topological build order - DriftDetector: planned status vs CodeDef presence in repo, explicit repo param - domains/revopsos/constellation.yaml: 8-service RevOpsOS constellation - Action router: 6 actions (search_codegraph, build_codegraph, search_plangraph, build_order, check_drift, load_constellation) - 21 unit tests, all green, zero Neo4j required - Lint clean (ruff), formatted
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 28261015 | Triggered | Generic Password | 31e961a | engine/plangraph/retriever.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
WalkthroughThis pull request introduces a comprehensive CodeGraph and PlanGraph architecture for analyzing repository code structure and managing service dependency graphs. It adds lifecycle hooks for application initialization, Neo4j-backed graph builders and retrievers for code definitions/references, service constellation management with drift detection, and configuration updates to support these new subsystems. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CodegraphLifecycle
participant CodeGraphBuilder
participant Parser
participant Neo4j
Client->>CodegraphLifecycle: execute(action="build_codegraph", payload)
CodegraphLifecycle->>CodegraphLifecycle: validate payload (repo)
CodegraphLifecycle->>CodeGraphBuilder: clone repo & construct builder
CodegraphLifecycle->>CodeGraphBuilder: build()
CodeGraphBuilder->>Parser: find_files()
Parser-->>CodeGraphBuilder: list of source files
loop for each file
CodeGraphBuilder->>Parser: parse_file(filepath)
Parser-->>CodeGraphBuilder: {definitions, references}
CodeGraphBuilder->>Neo4j: create CodeDef nodes + INVOKES edges
end
Neo4j-->>CodeGraphBuilder: nodes created
CodeGraphBuilder-->>CodegraphLifecycle: {repo, file_count, total_defs, total_refs}
CodegraphLifecycle-->>Client: {status, result}
sequenceDiagram
participant Client
participant CodegraphLifecycle
participant EgoGraphRetriever
participant Neo4j
Client->>CodegraphLifecycle: execute(action="search_codegraph", payload)
CodegraphLifecycle->>CodegraphLifecycle: validate payload (term, repo)
CodegraphLifecycle->>EgoGraphRetriever: search(term, repo, hops)
EgoGraphRetriever->>Neo4j: MATCH center node (term, repo)
Neo4j-->>EgoGraphRetriever: center CodeDef
EgoGraphRetriever->>Neo4j: MATCH neighbors via INVOKES (hops)
Neo4j-->>EgoGraphRetriever: neighbor CodeDef nodes
EgoGraphRetriever->>EgoGraphRetriever: _flatten(nodes, edges)
EgoGraphRetriever-->>CodegraphLifecycle: {nodes, edges, flat_text}
CodegraphLifecycle-->>Client: search result
sequenceDiagram
participant Client
participant CodegraphLifecycle
participant PlanGraphBuilder
participant SpecParser
participant Neo4j
Client->>CodegraphLifecycle: execute(action="load_constellation", payload)
CodegraphLifecycle->>SpecParser: parse(spec_file)
SpecParser-->>CodegraphLifecycle: {services, interfaces, flows}
CodegraphLifecycle->>PlanGraphBuilder: load(spec)
PlanGraphBuilder->>Neo4j: clear existing constellation data
Neo4j-->>PlanGraphBuilder: cleared
loop for each service
PlanGraphBuilder->>Neo4j: create PlanService node
end
loop for each dependency
PlanGraphBuilder->>Neo4j: create DEPENDS_ON edge
end
loop for each flow
PlanGraphBuilder->>Neo4j: create FLOWS_TO or FEEDS_BACK_TO edge
end
Neo4j-->>PlanGraphBuilder: all relationships created
PlanGraphBuilder-->>CodegraphLifecycle: {constellation, counts}
CodegraphLifecycle-->>Client: load result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31e961a8aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+71
to
+72
| MERGE (n:CodeDef {name: d, repo: $repo}) | ||
| SET n.file = $file, n.language = $lang, n.updated = timestamp() |
There was a problem hiding this comment.
Merging CodeDef on only {name, repo} collapses different symbols that share a name across files into one node, and the subsequent SET n.file = $file overwrites whichever file was written earlier. In repositories with common names like main, run, or handler, this corrupts both node metadata and INVOKES edges, so graph search and drift checks can report incorrect dependencies.
Useful? React with 👍 / 👎.
| from engine.codegraph.handler import handle_search_codegraph | ||
|
|
||
| result = await handle_search_codegraph(payload) | ||
| return JSONResponse({"status": "ok", "action": action, "result": result}) |
There was a problem hiding this comment.
Each action branch wraps handler output in {"status": "ok"} with HTTP 200, even when handlers return {"error": ...} (e.g., missing required fields, clone/build failures, disabled engines). This makes failed operations indistinguishable from successful ones to callers that rely on status code or top-level status, which can let automation proceed on invalid results.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@domains/revopsos/constellation.yaml`:
- Around line 1-9: Add a YAML document start marker by inserting `---` at the
top of the file so the file begins with a YAML document start before the
existing L9_META block; specifically update the file containing the L9_META
comments and the `constellation: revopsos` entry so parsers recognize a valid
YAML document start.
In `@engine/boot.py`:
- Around line 26-34: The startup method currently creates self._driver via
AsyncGraphDatabase.driver but never uses it; either remove that driver creation
from CodegraphLifecycle.startup or change handlers to reuse it: update startup
to not call AsyncGraphDatabase.driver if you want handlers
(handle_search_codegraph, handle_build_codegraph) to manage their own
connections, or modify those handlers (and the classes EgoGraphRetriever and
CodeGraphBuilder) to accept and use the injected async driver (self._driver)
instead of creating new synchronous GraphDatabase.driver instances so
connections are reused.
In `@engine/codegraph/builder.py`:
- Around line 82-102: The comment on the UNWIND $defs block is incorrect: the
code in the session.run (UNWIND $defs ...) creates edges from every def in defs
to each ref (not "first def" only), so update the comment near the session.run
to state "link every def in this file to each ref target" (or similar) and
adjust the metric update for total_refs to reflect edges created by multiplying
defs × refs (e.g., replace the current total_refs += len(refs) with total_refs
+= len(defs) * len(refs) or increment by len(defs) inside the ref loop);
reference symbols: session.run, UNWIND $defs, defs, ref_name, total_refs.
In `@engine/codegraph/handler.py`:
- Line 35: Validate the "hops" value before converting to int: instead of
calling hops = int(payload.get("hops", 1)) directly, read raw_hops =
payload.get("hops", 1) and check its type/format (e.g., if isinstance(raw_hops,
str) and raw_hops.isdigit() or try converting in a small try/except), catch
ValueError, and return/raise a clear BadRequest/HTTP error with a message like
"Invalid hops value: must be an integer" (or set a default) so callers get a
descriptive error; update the conversion site that uses payload.get("hops", 1)
and the hops variable accordingly.
- Around line 86-88: The current except block in handler.py logs and returns
str(e) which may contain a URL with an embedded GitHub token; update the except
handler around logger.error("codegraph.clone_failed", repo=repo, error=str(e))
and the subsequent return to sanitize the error before logging/returning by
stripping or masking credentials from any URL patterns (e.g., remove
username:token@host or replace sensitive segments with "<redacted>") using a
small sanitizer function (e.g., sanitize_error(msg)) and call it on str(e) so
both logger.error and the returned {"error": ...} contain the sanitized message
instead of the raw exception.
In `@engine/codegraph/parser.py`:
- Around line 124-154: The lazy initializer _init_languages mutates the global
LANGUAGE_MAP without synchronization; add a module-level lock (e.g.,
LANGUAGE_INIT_LOCK = threading.Lock()) and wrap the initialization body in a
with LANGUAGE_INIT_LOCK: block, re-checking if LANGUAGE_MAP is already populated
inside the locked section before proceeding; update imports to include threading
and ensure CodeLineParser users still call _init_languages() unchanged so
concurrent instantiation of CodeLineParser is serialized by the lock.
In `@engine/codegraph/retriever.py`:
- Around line 66-72: The Cypher range quantifier cannot accept a parameter ($h);
update the call that builds the query for neighbors_result so the numeric upper
bound is embedded as a literal: validate and coerce hops to a safe integer (>=1
and within a sane max), then construct the Cypher string with the literal upper
bound e.g. "MATCH (c:CodeDef {name: $t, repo: $r})-[:INVOKES*1..{h}]-(n:CodeDef
{repo: $r}) RETURN DISTINCT n" while still passing term and repo as parameters
to s.run; reference the neighbors_result call, the s.run invocation, the hops
variable, and the INVOKES range so you replace the parameterized quantifier with
a sanitized literal.
In `@engine/plangraph/builder.py`:
- Around line 155-167: The _ensure_indexes method currently catches all
exceptions and logs them at debug level; change it to catch
neo4j.exceptions.ClientError specifically around the two session.run calls (to
quietly ignore benign index-exists errors) and let any other Exception propagate
or be logged at error level via logger.error with the exception details; update
the except blocks to reference neo4j.exceptions.ClientError and add a broader
except Exception as e that calls logger.error("plangraph_index_error",
error=str(e)) so configuration/permission errors are visible while
index-already-exists remains suppressed.
- Around line 40-70: The load method currently opens a session and runs multiple
write statements (e.g., creating PlanService nodes in the loop that runs
MERGE/SET for each svc) without an explicit transaction, leaving the DB prone to
partial commits if a failure occurs; refactor load to perform all write work
inside a single transactional function (via session.execute_write or
session.begin_transaction / tx.run) that encapsulates the loops for services,
dependencies, interfaces, and flows so the driver will commit only if the entire
operation succeeds and automatically roll back on error, keeping calls to
_ensure_indexes() and _clear() as needed (move _clear() inside the same
transaction if you require atomic clearing+load) and surface/raise errors from
the transactional function for proper logging/handling.
In `@engine/plangraph/drift.py`:
- Around line 130-137: The check_all implementation currently opens a session to
load service_names and then calls check_service which opens a new session per
service, causing N+1 sessions; refactor so you reuse a single session: either
modify check_service to accept an existing session parameter (e.g., add an
optional session arg to check_service and use it if provided) or extract the
session-bound logic into a private helper (e.g.,
_check_service_with_session(session, svc, constellation, repo)) and call that
for each svc inside the single with self.driver.session() block that builds
services_rec/service_names.
- Around line 26-35: The DriftDetector class currently requires callers to call
close() manually; add context manager support by implementing __enter__ and
__exit__ methods on the class so it can be used with "with DriftDetector(...) as
detector:" and will always close the GraphDatabase driver; implement __enter__
to return self, implement __exit__(exc_type, exc, tb) to call self.close() (and
optionally suppress exceptions only if intended), and ensure the existing
close() continues to close self.driver to avoid resource leaks (referencing the
class DriftDetector, its __init__, close, and the self.driver attribute).
In `@engine/plangraph/handler.py`:
- Around line 15-21: The module currently instantiates Settings() at import time
via the _settings variable, which freezes environment-derived config and
prevents tests or runtime changes from taking effect; change this to lazy
initialization by replacing the top-level _settings = Settings() with a getter
(e.g., get_settings() or a cached property) that constructs and/or caches a
Settings instance on first use, and update all uses of _settings in
functions/classes in this module to call that getter so configuration is read
lazily.
- Around line 116-154: The handler handle_load_constellation passes spec_file
directly into SpecParser.parse allowing path traversal; before calling
SpecParser(spec_dir=...).parse(spec_file) resolve the candidate path using
pathlib (e.g. combine spec_dir and spec_file into a Path, call
resolve(strict=False) and then verify the resolved path is within the intended
spec_dir root (use Path.is_relative_to or compare resolved_path.commonpath with
resolved_spec_dir.resolve()); if the check fails or the target is not a regular
file, return an error like "invalid spec_file" and do not call SpecParser.parse;
keep using the symbols spec_dir, spec_file, SpecParser.parse, _DOMAINS_DIR and
handle_load_constellation to locate and update the logic.
In `@engine/plangraph/retriever.py`:
- Around line 130-159: The _topological_sort currently appends any nodes in
cycles to topo and groups, which masks cycle presence; change _topological_sort
to return a third value or a flag (e.g., return (topo, groups, has_cycle) or
(topo, groups, remaining) where has_cycle = bool(remaining)) and set has_cycle
True when remaining is non-empty instead of silently appending cycle nodes;
update the build_order function to consume the new return value and include
has_cycle in its returned dict (or raise a descriptive exception from
_topological_sort if you prefer failing fast), and update any call sites that
expect the old two-tuple to handle the new contract.
In `@engine/plangraph/spec_parser.py`:
- Around line 36-50: The YAML loader may return None for empty files, causing
AttributeError when calling _extract_services, _extract_interfaces, and
_extract_flows on raw; after calling yaml.safe_load(path.read_text(...)) assign
raw = raw or {} (or otherwise guard) so raw is a dict before passing it into
self._extract_services, self._extract_interfaces, and self._extract_flows;
update the block around the yaml.safe_load call and the variable raw to ensure
those methods always receive a mapping instead of None.
In `@engine/settings.py`:
- Around line 16-29: Remove all os.getenv() calls from the Settings field
defaults and switch to pydantic v2 configuration: replace the inner Config class
with model_config = ConfigDict(env_file=".env", extra="ignore"), import Field
and ConfigDict from pydantic, and declare each setting with a proper default and
Field(..., env="ENV_NAME") when the environment variable name differs (e.g.,
neo4j_uri -> Field("bolt://localhost:7687", env="NEO4J_BOLT_URI"), port ->
Field(8002, env="PORT"), debug/codegraph_enabled/plangraph_enabled ->
Field(True/False, env="DEBUG"/"L9_CODEGRAPH_ENABLED"/"L9_PLANGRAPH_ENABLED") so
pydantic-settings will load .env and perform type conversion for the fields
app_name, debug, port, neo4j_uri, neo4j_user, neo4j_password, github_token,
codegraph_enabled, and plangraph_enabled.
In `@pyproject.toml`:
- Line 4: Update the pyproject.toml description value (the description = "L9
microservice — replace with your service name" entry) to a concise, accurate
summary of this service’s purpose—replace the placeholder text with the real
service name and/or short purpose statement so the description key reflects the
actual project intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2761fa19-9bdc-4ca4-9746-ec0e45a78153
📒 Files selected for processing (22)
Dockerfilechassis/__init__.pydomains/revopsos/constellation.yamlengine/boot.pyengine/codegraph/__init__.pyengine/codegraph/builder.pyengine/codegraph/handler.pyengine/codegraph/parser.pyengine/codegraph/retriever.pyengine/main.pyengine/plangraph/__init__.pyengine/plangraph/builder.pyengine/plangraph/drift.pyengine/plangraph/handler.pyengine/plangraph/retriever.pyengine/plangraph/schema.pyengine/plangraph/spec_parser.pyengine/settings.pypyproject.tomlsonar-project.propertiestests/test_codegraph_unit.pytests/test_plangraph_unit.py
Comment on lines
+1
to
+9
| # --- L9_META --- | ||
| # l9_schema: 1 | ||
| # layer: [domain] | ||
| # tags: [plangraph, constellation, revopsos] | ||
| # status: active | ||
| # --- /L9_META --- | ||
| # RevOpsOS constellation — 8-service revenue operations platform | ||
|
|
||
| constellation: revopsos |
There was a problem hiding this comment.
Add YAML document start marker.
The YAML linter flags a missing document start marker. Adding --- improves YAML standards compliance and compatibility with multi-document parsers.
📝 Proposed fix
# --- L9_META ---
# l9_schema: 1
# layer: [domain]
# tags: [plangraph, constellation, revopsos]
# status: active
# --- /L9_META ---
+---
# RevOpsOS constellation — 8-service revenue operations platform
constellation: revopsos📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # --- L9_META --- | |
| # l9_schema: 1 | |
| # layer: [domain] | |
| # tags: [plangraph, constellation, revopsos] | |
| # status: active | |
| # --- /L9_META --- | |
| # RevOpsOS constellation — 8-service revenue operations platform | |
| constellation: revopsos | |
| # --- L9_META --- | |
| # l9_schema: 1 | |
| # layer: [domain] | |
| # tags: [plangraph, constellation, revopsos] | |
| # status: active | |
| # --- /L9_META --- | |
| --- | |
| # RevOpsOS constellation — 8-service revenue operations platform | |
| constellation: revopsos |
🧰 Tools
🪛 GitHub Check: YAML Validation
[warning] 9-9:
9:1 [document-start] missing document start "---"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@domains/revopsos/constellation.yaml` around lines 1 - 9, Add a YAML document
start marker by inserting `---` at the top of the file so the file begins with a
YAML document start before the existing L9_META block; specifically update the
file containing the L9_META comments and the `constellation: revopsos` entry so
parsers recognize a valid YAML document start.
Comment on lines
+26
to
+34
| async def startup(self) -> None: | ||
| logger.info("CodegraphLifecycle.startup — Neo4j: %s", settings.neo4j_uri) | ||
| # Import here to avoid loading neo4j at module level (keeps tests fast) | ||
| from neo4j import AsyncGraphDatabase | ||
| self._driver = AsyncGraphDatabase.driver( | ||
| settings.neo4j_uri, | ||
| auth=(settings.neo4j_user, settings.neo4j_password), | ||
| ) | ||
| logger.info("CodegraphLifecycle.startup complete") |
There was a problem hiding this comment.
Driver created at startup is unused.
The AsyncGraphDatabase.driver created here is never passed to handlers. Each handler (handle_search_codegraph, handle_build_codegraph) creates its own synchronous GraphDatabase.driver connection via EgoGraphRetriever and CodeGraphBuilder (see engine/codegraph/handler.py lines 42-46, 90-96).
Consider either:
- Passing
self._driverto handlers for connection reuse - Removing the startup driver if handlers manage their own connections
Current implementation creates unnecessary Neo4j connections.
Option A: Remove unused driver setup
If handlers will continue managing their own connections:
async def startup(self) -> None:
logger.info("CodegraphLifecycle.startup — Neo4j: %s", settings.neo4j_uri)
- # Import here to avoid loading neo4j at module level (keeps tests fast)
- from neo4j import AsyncGraphDatabase
- self._driver = AsyncGraphDatabase.driver(
- settings.neo4j_uri,
- auth=(settings.neo4j_user, settings.neo4j_password),
- )
+ # Handlers manage their own Neo4j connections
logger.info("CodegraphLifecycle.startup complete")
async def shutdown(self) -> None:
- if self._driver:
- await self._driver.close()
logger.info("CodegraphLifecycle.shutdown complete")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/boot.py` around lines 26 - 34, The startup method currently creates
self._driver via AsyncGraphDatabase.driver but never uses it; either remove that
driver creation from CodegraphLifecycle.startup or change handlers to reuse it:
update startup to not call AsyncGraphDatabase.driver if you want handlers
(handle_search_codegraph, handle_build_codegraph) to manage their own
connections, or modify those handlers (and the classes EgoGraphRetriever and
CodeGraphBuilder) to accept and use the injected async driver (self._driver)
instead of creating new synchronous GraphDatabase.driver instances so
connections are reused.
Comment on lines
+82
to
+102
| if defs and refs: | ||
| for ref_name in refs: | ||
| # For each ref in this file, create an INVOKES edge from | ||
| # any def in this file that could call it. | ||
| # Simplified: link the first def in this file to the ref target. | ||
| # More accurate: all defs in this file may reference it. | ||
| session.run( | ||
| """ | ||
| UNWIND $defs AS caller | ||
| MATCH (a:CodeDef {name: caller, repo: $repo}) | ||
| MERGE (b:CodeDef {name: $ref, repo: $repo}) | ||
| ON CREATE SET b.file = 'unknown', b.language = $lang, | ||
| b.updated = timestamp(), b.synthetic = true | ||
| MERGE (a)-[:INVOKES]->(b) | ||
| """, | ||
| defs=defs, | ||
| ref=ref_name, | ||
| repo=self.repo, | ||
| lang=parsed.get("language", "unknown"), | ||
| ) | ||
| total_refs += len(refs) |
There was a problem hiding this comment.
Comment contradicts implementation; edge count may be misleading.
-
Comment on lines 85-87 says "Simplified: link the first def in this file to the ref target" but the code
UNWIND $defslinks all defs to each ref. -
total_refscounts reference targets per file, but the actual edges created arelen(defs) × len(refs)per file due to the UNWIND pattern.
📝 Proposed fix for comment accuracy
# Write CodeRef nodes and INVOKES edges
if defs and refs:
for ref_name in refs:
- # For each ref in this file, create an INVOKES edge from
- # any def in this file that could call it.
- # Simplified: link the first def in this file to the ref target.
- # More accurate: all defs in this file may reference it.
+ # For each ref in this file, create an INVOKES edge from
+ # every def in this file to the ref target.
+ # This is intentionally broad — refinement requires call-graph analysis.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/codegraph/builder.py` around lines 82 - 102, The comment on the UNWIND
$defs block is incorrect: the code in the session.run (UNWIND $defs ...) creates
edges from every def in defs to each ref (not "first def" only), so update the
comment near the session.run to state "link every def in this file to each ref
target" (or similar) and adjust the metric update for total_refs to reflect
edges created by multiplying defs × refs (e.g., replace the current total_refs
+= len(refs) with total_refs += len(defs) * len(refs) or increment by len(defs)
inside the ref loop); reference symbols: session.run, UNWIND $defs, defs,
ref_name, total_refs.
| if not term or not repo: | ||
| return {"error": "search_codegraph requires 'term' and 'repo'"} | ||
|
|
||
| hops = int(payload.get("hops", 1)) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating hops before conversion.
If hops is a non-numeric string (e.g., "abc"), int() raises ValueError. While this is caught by the lifecycle's exception handler, a clearer error message would improve API usability.
💡 Optional improvement
- hops = int(payload.get("hops", 1))
+ raw_hops = payload.get("hops", 1)
+ try:
+ hops = int(raw_hops)
+ except (TypeError, ValueError):
+ return {"error": f"'hops' must be an integer, got: {raw_hops!r}"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| hops = int(payload.get("hops", 1)) | |
| raw_hops = payload.get("hops", 1) | |
| try: | |
| hops = int(raw_hops) | |
| except (TypeError, ValueError): | |
| return {"error": f"'hops' must be an integer, got: {raw_hops!r}"} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/codegraph/handler.py` at line 35, Validate the "hops" value before
converting to int: instead of calling hops = int(payload.get("hops", 1))
directly, read raw_hops = payload.get("hops", 1) and check its type/format
(e.g., if isinstance(raw_hops, str) and raw_hops.isdigit() or try converting in
a small try/except), catch ValueError, and return/raise a clear BadRequest/HTTP
error with a message like "Invalid hops value: must be an integer" (or set a
default) so callers get a descriptive error; update the conversion site that
uses payload.get("hops", 1) and the hops variable accordingly.
Comment on lines
+86
to
+88
| except Exception as e: | ||
| logger.error("codegraph.clone_failed", repo=repo, error=str(e)) | ||
| return {"error": f"Clone failed: {e}"} |
There was a problem hiding this comment.
Potential token exposure in error logs.
If the clone fails, str(e) may include the full URL with the embedded GitHub token. Consider sanitizing the error message before logging.
🔒 Proposed fix
except Exception as e:
- logger.error("codegraph.clone_failed", repo=repo, error=str(e))
- return {"error": f"Clone failed: {e}"}
+ # Sanitize error to avoid leaking token
+ error_msg = str(e).replace(token, "***") if token else str(e)
+ logger.error("codegraph.clone_failed", repo=repo, error=error_msg)
+ return {"error": f"Clone failed for {repo}"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/codegraph/handler.py` around lines 86 - 88, The current except block
in handler.py logs and returns str(e) which may contain a URL with an embedded
GitHub token; update the except handler around
logger.error("codegraph.clone_failed", repo=repo, error=str(e)) and the
subsequent return to sanitize the error before logging/returning by stripping or
masking credentials from any URL patterns (e.g., remove username:token@host or
replace sensitive segments with "<redacted>") using a small sanitizer function
(e.g., sanitize_error(msg)) and call it on str(e) so both logger.error and the
returned {"error": ...} contain the sanitized message instead of the raw
exception.
Comment on lines
+116
to
+154
| async def handle_load_constellation(payload: dict) -> dict: | ||
| """Load a constellation spec from a YAML file into Neo4j. | ||
|
|
||
| Payload: | ||
| constellation (str, required): Constellation identifier | ||
| spec_file (str, required): YAML filename (relative to domains/<constellation>/) | ||
| spec_dir (str, optional): Override directory path | ||
| """ | ||
| constellation = payload.get("constellation") | ||
| spec_file = payload.get("spec_file") | ||
| if not constellation or not spec_file: | ||
| return {"error": "load_constellation requires 'constellation' and 'spec_file'"} | ||
|
|
||
| if not _settings.plangraph_enabled: | ||
| return {"error": "PlanGraph engine is disabled (L9_PLANGRAPH_ENABLED=false)"} | ||
|
|
||
| spec_dir = payload.get("spec_dir") or str(_DOMAINS_DIR / constellation) | ||
|
|
||
| from .builder import PlanGraphBuilder | ||
| from .spec_parser import SpecParser | ||
|
|
||
| parser = SpecParser(spec_dir=spec_dir) | ||
| spec = parser.parse(spec_file) | ||
|
|
||
| if not spec.get("services"): | ||
| return {"error": f"No services found in spec '{spec_file}' at '{spec_dir}'"} | ||
|
|
||
| builder = PlanGraphBuilder( | ||
| neo4j_uri=_settings.neo4j_uri, | ||
| neo4j_password=_settings.neo4j_password, | ||
| constellation=constellation, | ||
| neo4j_user=_settings.neo4j_user, | ||
| ) | ||
| try: | ||
| result = builder.load(spec) | ||
| logger.info("plangraph.loaded", constellation=constellation, spec_file=spec_file) | ||
| return {"status": "loaded", **result} | ||
| finally: | ||
| builder.close() |
There was a problem hiding this comment.
Path traversal vulnerability in spec_file parameter.
The spec_file parameter from the payload is passed directly to SpecParser.parse() without sanitization. A malicious payload like {"spec_file": "../../../etc/passwd"} could read arbitrary files outside the intended spec directory.
🔒 Proposed fix to validate the resolved path
spec_dir = payload.get("spec_dir") or str(_DOMAINS_DIR / constellation)
from .builder import PlanGraphBuilder
from .spec_parser import SpecParser
parser = SpecParser(spec_dir=spec_dir)
+
+ # Validate spec_file doesn't escape spec_dir
+ spec_dir_path = Path(spec_dir).resolve()
+ resolved_path = (spec_dir_path / spec_file).resolve()
+ if not resolved_path.is_relative_to(spec_dir_path):
+ return {"error": f"Invalid spec_file path: '{spec_file}'"}
+
spec = parser.parse(spec_file)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/plangraph/handler.py` around lines 116 - 154, The handler
handle_load_constellation passes spec_file directly into SpecParser.parse
allowing path traversal; before calling
SpecParser(spec_dir=...).parse(spec_file) resolve the candidate path using
pathlib (e.g. combine spec_dir and spec_file into a Path, call
resolve(strict=False) and then verify the resolved path is within the intended
spec_dir root (use Path.is_relative_to or compare resolved_path.commonpath with
resolved_spec_dir.resolve()); if the check fails or the target is not a regular
file, return an error like "invalid spec_file" and do not call SpecParser.parse;
keep using the symbols spec_dir, spec_file, SpecParser.parse, _DOMAINS_DIR and
handle_load_constellation to locate and update the logic.
Comment on lines
+130
to
+159
| def _topological_sort(self, dep_map: dict[str, list[str]]) -> tuple[list[str], list[list[str]]]: | ||
| """Kahn's algorithm — returns (flat order, parallel groups). | ||
|
|
||
| dep_map: {service_name: [dependencies]} | ||
| """ | ||
| # in_degree = number of unresolved dependencies per node | ||
| in_degree = {n: len(deps) for n, deps in dep_map.items()} | ||
| dependents = self._build_dependents(dep_map) | ||
| queue: deque[str] = deque(sorted(n for n, d in in_degree.items() if d == 0)) | ||
| topo: list[str] = [] | ||
| groups: list[list[str]] = [] | ||
|
|
||
| while queue: | ||
| level = list(queue) | ||
| queue.clear() | ||
| groups.append(sorted(level)) | ||
| topo.extend(sorted(level)) | ||
| for node in level: | ||
| for dep_node in dependents.get(node, []): | ||
| in_degree[dep_node] -= 1 | ||
| if in_degree[dep_node] == 0: | ||
| queue.append(dep_node) | ||
|
|
||
| remaining = sorted(n for n in in_degree if n not in topo) | ||
| if remaining: | ||
| logger.warning("plangraph.cycle_detected", nodes=remaining) | ||
| topo.extend(remaining) | ||
| groups.append(remaining) | ||
|
|
||
| return topo, groups |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Cycle handling produces potentially misleading output.
When a cycle is detected, remaining nodes are appended to topo and groups without distinguishing them from validly ordered nodes. This could lead callers to believe the build order is valid when it's not. Consider returning a separate has_cycle flag or raising an exception.
♻️ Proposed improvement to signal cycle presence
- return topo, groups
+ has_cycle = len(remaining) > 0
+ return topo, groups, has_cycleThen update build_order to include has_cycle in the return dict:
- topo, groups = self._topological_sort(dep_map)
+ topo, groups, has_cycle = self._topological_sort(dep_map)
flat_text = self._flatten_build_order(constellation, topo, groups)
return {
"constellation": constellation,
"topological_order": topo,
"parallel_groups": groups,
+ "has_cycle": has_cycle,
"flat_text": flat_text,
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/plangraph/retriever.py` around lines 130 - 159, The _topological_sort
currently appends any nodes in cycles to topo and groups, which masks cycle
presence; change _topological_sort to return a third value or a flag (e.g.,
return (topo, groups, has_cycle) or (topo, groups, remaining) where has_cycle =
bool(remaining)) and set has_cycle True when remaining is non-empty instead of
silently appending cycle nodes; update the build_order function to consume the
new return value and include has_cycle in its returned dict (or raise a
descriptive exception from _topological_sort if you prefer failing fast), and
update any call sites that expect the old two-tuple to handle the new contract.
Comment on lines
+36
to
+50
| path = self.spec_dir / filename | ||
| try: | ||
| raw = yaml.safe_load(path.read_text(encoding="utf-8")) | ||
| except FileNotFoundError: | ||
| logger.error("spec_not_found", path=str(path)) | ||
| return {"services": [], "interfaces": [], "flows": []} | ||
| except yaml.YAMLError as e: | ||
| logger.error("spec_parse_error", path=str(path), error=str(e)) | ||
| return {"services": [], "interfaces": [], "flows": []} | ||
|
|
||
| services = self._extract_services(raw) | ||
| interfaces = self._extract_interfaces(raw) | ||
| flows = self._extract_flows(raw) | ||
|
|
||
| return {"services": services, "interfaces": interfaces, "flows": flows} |
There was a problem hiding this comment.
Potential AttributeError if YAML file is empty.
If the YAML file is empty or contains only comments, yaml.safe_load() returns None. Passing None to the _extract_* methods will cause AttributeError when calling .get() on Line 46-48.
🐛 Proposed fix to handle empty YAML files
try:
raw = yaml.safe_load(path.read_text(encoding="utf-8"))
except FileNotFoundError:
logger.error("spec_not_found", path=str(path))
return {"services": [], "interfaces": [], "flows": []}
except yaml.YAMLError as e:
logger.error("spec_parse_error", path=str(path), error=str(e))
return {"services": [], "interfaces": [], "flows": []}
+ if not isinstance(raw, dict):
+ logger.error("spec_invalid_format", path=str(path))
+ return {"services": [], "interfaces": [], "flows": []}
+
services = self._extract_services(raw)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/plangraph/spec_parser.py` around lines 36 - 50, The YAML loader may
return None for empty files, causing AttributeError when calling
_extract_services, _extract_interfaces, and _extract_flows on raw; after calling
yaml.safe_load(path.read_text(...)) assign raw = raw or {} (or otherwise guard)
so raw is a dict before passing it into self._extract_services,
self._extract_interfaces, and self._extract_flows; update the block around the
yaml.safe_load call and the variable raw to ensure those methods always receive
a mapping instead of None.
Comment on lines
+16
to
+29
| class Settings(BaseSettings): | ||
| app_name: str = "l9-codegraph" | ||
| debug: bool = os.getenv("DEBUG", "false").lower() == "true" | ||
| port: int = int(os.getenv("PORT", "8002")) | ||
| neo4j_uri: str = os.getenv("NEO4J_BOLT_URI", "bolt://localhost:7687") | ||
| neo4j_user: str = os.getenv("NEO4J_USER", "neo4j") | ||
| neo4j_password: str = os.getenv("NEO4J_PASSWORD", "") | ||
| github_token: str = os.getenv("GITHUB_TOKEN", "") | ||
| codegraph_enabled: bool = os.getenv("L9_CODEGRAPH_ENABLED", "true").lower() == "true" | ||
| plangraph_enabled: bool = os.getenv("L9_PLANGRAPH_ENABLED", "true").lower() == "true" | ||
|
|
||
| def load_config() -> PlatformConfig: | ||
| return PlatformConfig() | ||
| class Config: | ||
| env_file = ".env" | ||
| extra = "ignore" |
There was a problem hiding this comment.
Fix pydantic-settings usage: remove os.getenv() from defaults and use ConfigDict.
Two issues:
-
Bypassed env var resolution: Using
os.getenv()as field defaults evaluates at import time, before pydantic-settings loads the.envfile. This defeats the purpose of BaseSettings and causes.envvalues to be ignored. -
Deprecated Config class: The pipeline flagged that class-based
Configis deprecated in Pydantic v2.
🐛 Proposed fix
-from pydantic_settings import BaseSettings
+from pydantic_settings import BaseSettings, SettingsConfigDict
class Settings(BaseSettings):
app_name: str = "l9-codegraph"
- debug: bool = os.getenv("DEBUG", "false").lower() == "true"
- port: int = int(os.getenv("PORT", "8002"))
- neo4j_uri: str = os.getenv("NEO4J_BOLT_URI", "bolt://localhost:7687")
- neo4j_user: str = os.getenv("NEO4J_USER", "neo4j")
- neo4j_password: str = os.getenv("NEO4J_PASSWORD", "")
- github_token: str = os.getenv("GITHUB_TOKEN", "")
- codegraph_enabled: bool = os.getenv("L9_CODEGRAPH_ENABLED", "true").lower() == "true"
- plangraph_enabled: bool = os.getenv("L9_PLANGRAPH_ENABLED", "true").lower() == "true"
+ debug: bool = False
+ port: int = 8002
+ neo4j_uri: str = "bolt://localhost:7687"
+ neo4j_user: str = "neo4j"
+ neo4j_password: str = ""
+ github_token: str = ""
+ codegraph_enabled: bool = True
+ plangraph_enabled: bool = True
- class Config:
- env_file = ".env"
- extra = "ignore"
+ model_config = SettingsConfigDict(
+ env_file=".env",
+ extra="ignore",
+ env_prefix="", # or define prefixes as needed
+ )Then use Field(alias="NEO4J_BOLT_URI") for fields with non-standard env var names, or configure env_nested_delimiter / validation aliases as needed.
🧰 Tools
🪛 GitHub Actions: Quality Gates
[warning] 16-16: PydanticDeprecatedSince20: Support for class-based config is deprecated, use ConfigDict instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/settings.py` around lines 16 - 29, Remove all os.getenv() calls from
the Settings field defaults and switch to pydantic v2 configuration: replace the
inner Config class with model_config = ConfigDict(env_file=".env",
extra="ignore"), import Field and ConfigDict from pydantic, and declare each
setting with a proper default and Field(..., env="ENV_NAME") when the
environment variable name differs (e.g., neo4j_uri ->
Field("bolt://localhost:7687", env="NEO4J_BOLT_URI"), port -> Field(8002,
env="PORT"), debug/codegraph_enabled/plangraph_enabled -> Field(True/False,
env="DEBUG"/"L9_CODEGRAPH_ENABLED"/"L9_PLANGRAPH_ENABLED") so pydantic-settings
will load .env and perform type conversion for the fields app_name, debug, port,
neo4j_uri, neo4j_user, neo4j_password, github_token, codegraph_enabled, and
plangraph_enabled.
| name = "l9-service" | ||
| name = "l9-codegraph" | ||
| version = "0.1.0" | ||
| description = "L9 microservice — replace with your service name" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Update the project description.
The description still contains the template placeholder text. Consider updating it to reflect the actual service purpose.
-description = "L9 microservice — replace with your service name"
+description = "L9 CodeGraph — dual-graph microservice for code intelligence and service planning"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| description = "L9 microservice — replace with your service name" | |
| description = "L9 CodeGraph — dual-graph microservice for code intelligence and service planning" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` at line 4, Update the pyproject.toml description value (the
description = "L9 microservice — replace with your service name" entry) to a
concise, accurate summary of this service’s purpose—replace the placeholder text
with the real service name and/or short purpose statement so the description key
reflects the actual project intent.
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.
Summary
Implements the full dual-graph microservice on the golden-repo template.
CodeGraph Engine (RepoGraph ICLR 2025 inspired)
CodeGraphBuilder: repo-scoped DETACH DELETE, writesCodeDef+INVOKESedgesEgoGraphRetriever: pure Cypher ego-graph (1–2 hops), no APOC, returnsflat_textbuild_codegraph(gitpython clone) +search_codegraphPlanGraph Engine (RPG-inspired)
PlanGraphBuilder: constellation-scoped upsert — PlanService/PlanInterface/FLOWS_TO/DEPENDS_ON/FEEDS_BACK_TOPlanGraphRetriever: service neighborhood + Kahn's topological build order (parallel wave groups)DriftDetector: planned vs CodeDef presence, explicit repo paramSpecParser: YAML → normalized dictDomain Spec
domains/revopsos/constellation.yaml: 8-service RevOpsOS constellationAPI
6 actions wired into
POST /v1/execute:search_codegraph,build_codegraph,search_plangraph,build_order,check_drift,load_constellationQuality
Summary by CodeRabbit
Release Notes
New Features
Configuration
Tests