feat(management): implement Management REST API for Knowledge Graphs

[jsell-rh]

Task: task-008
Spec: specs/management/knowledge-graphs.spec.md@85d49a379a52479b33f9b39994d76795066899a6

Merge

The orchestrator will squash-merge this PR automatically
once all pipeline steps pass.

---

This PR was created by hyperloop,
an AI agent orchestrator.

Summary by CodeRabbit

Release Notes

• New Features

  • Added Knowledge Graphs management page with ability to create and list knowledge graphs
  • Added Data Sources management page with sync monitoring, adapter configuration, and ontology editing
  • Introduced health check endpoints for application and database status monitoring
  • Enhanced authorization with secure entity redaction based on user permissions
  • Updated navigation with new "Explore," "Data," and "Connect" sections

• Improvements

  • Refined CORS configuration and credential handling
  • Added API endpoints for knowledge graph and data source operations

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 467ad4eb-5b10-4ac0-af62-2cfbae82d0a4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This pull request introduces a secure-enclave service for access-controlled graph queries, new management API endpoints for knowledge graphs and data sources, a job-package builder/reader for ZIP-based package handling, corresponding frontend pages, and extensive test coverage. It also updates CORS settings validation, workspace parent-workspace error handling, and includes multiple CI/check scripts for repository validation. Obsolete task/review documentation and state files are removed.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    Client->>+GraphSecureEnclaveService: search_by_slug(slug)
    GraphSecureEnclaveService->>+GraphQueryService: search_by_slug(slug)
    GraphQueryService-->>-GraphSecureEnclaveService: nodes with properties
    loop For each node
        GraphSecureEnclaveService->>GraphSecureEnclaveService: Extract knowledge_graph_id
        alt knowledge_graph_id present and valid
            GraphSecureEnclaveService->>+AuthorizationProvider: check_permission(KG, VIEW)
            AuthorizationProvider-->>-GraphSecureEnclaveService: authorized?
            alt authorized
                GraphSecureEnclaveService->>GraphSecureEnclaveService: Return full NodeRecord
            else denied
                GraphSecureEnclaveService->>GraphSecureEnclaveService: Return RedactedNodeRecord
            end
        else missing/invalid knowledge_graph_id
            GraphSecureEnclaveService->>GraphSecureEnclaveService: Return RedactedNodeRecord
        end
    end
    GraphSecureEnclaveService-->>-Client: filtered nodes with redaction

Loading

sequenceDiagram
    participant Builder as JobPackageBuilder
    participant ZipWriter as ZIP Archive
    participant Storage as Filesystem
    Builder->>Builder: add_content(bytes)
    Builder->>Builder: Deduplicate by SHA256
    Builder->>Builder: add_changeset_entry(entry)
    Builder->>Builder: set_checkpoint(state)
    Builder->>+Builder: build(output_dir)
    Builder->>Builder: Validate checkpoint present
    Builder->>Builder: Verify all content_refs exist
    Builder->>Builder: Compute manifest checksum
    Builder->>ZipWriter: Write manifest.json
    Builder->>ZipWriter: Write changeset.jsonl
    Builder->>ZipWriter: Write content/{hex_digest}
    Builder->>ZipWriter: Write state.json
    Builder->>ZipWriter: Validate all entry names
    ZipWriter->>+Storage: Save ZIP file
    Storage-->>-Builder: Path to archive
    Builder-->>-Builder: Return archive_path

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hyperloop/task-008

[jsell-rh]

@CodeRabbit recheck

[coderabbitai]

@jsell-rh Sure, I'll re-review the changes now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 3

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

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

⚠️ Outside diff range comments (1)

src/api/iam/application/services/workspace_service.py (1)

135-174: ⚠️ Potential issue | 🟠 Major

Validate the parent before checking CREATE_CHILD.

_check_workspace_permission() runs before the parent is loaded, so missing or cross-tenant parent IDs will typically short-circuit as UnauthorizedError. That makes ParentWorkspaceNotFoundError / ParentWorkspaceCrossTenantError effectively unreachable here, and a concurrent parent deletion can still surface DuplicateWorkspaceNameError first because name uniqueness is checked before the parent is revalidated.

Suggested ordering

-        # Check user has CREATE_CHILD permission on parent (before transaction)
-        # CREATE_CHILD = admin + editor + creator_tenant->view
-        # Root workspaces have creator_tenant set, so all tenant members can create children.
-        # Child workspaces don't, so only admin/editor can create sub-children.
-        has_create_child = await self._check_workspace_permission(
-            user_id=creator_id,
-            workspace_id=parent_workspace_id,
-            permission=Permission.CREATE_CHILD,
-        )
-
-        if not has_create_child:
-            raise UnauthorizedError(
-                f"User {creator_id.value} lacks create_child permission on parent workspace "
-                f"{parent_workspace_id.value}"
-            )
-
         try:
             async with self._session.begin():
+                parent = await self._workspace_repository.get_by_id(parent_workspace_id)
+                if not parent:
+                    raise ParentWorkspaceNotFoundError(
+                        f"Parent workspace {parent_workspace_id.value} does not exist"
+                    )
+
+                if parent.tenant_id != self._scope_to_tenant:
+                    raise ParentWorkspaceCrossTenantError(
+                        f"Parent workspace {parent_workspace_id.value} belongs to a different tenant"
+                    )
+
+                has_create_child = await self._check_workspace_permission(
+                    user_id=creator_id,
+                    workspace_id=parent_workspace_id,
+                    permission=Permission.CREATE_CHILD,
+                )
+                if not has_create_child:
+                    raise UnauthorizedError(
+                        f"User {creator_id.value} lacks create_child permission on parent workspace "
+                        f"{parent_workspace_id.value}"
+                    )
+
                 # Check name uniqueness within tenant
                 existing = await self._workspace_repository.get_by_name(
                     tenant_id=self._scope_to_tenant,
                     name=name,
                 )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/iam/application/services/workspace_service.py` around lines 135 -
174, Move parent existence and tenant validation before calling
_check_workspace_permission and before name-uniqueness check: first call
self._workspace_repository.get_by_id(parent_workspace_id) inside the transaction
(or before if necessary), raise ParentWorkspaceNotFoundError if None and
ParentWorkspaceCrossTenantError if parent.tenant_id != self._scope_to_tenant,
then call _check_workspace_permission(...) to compute has_create_child and raise
UnauthorizedError if false, and only after that call
self._workspace_repository.get_by_name(...) and raise
DuplicateWorkspaceNameError if a duplicate is found; update references to
has_create_child, _check_workspace_permission, get_by_id, get_by_name,
DuplicateWorkspaceNameError, ParentWorkspaceNotFoundError, and
ParentWorkspaceCrossTenantError accordingly so the permission check happens
after parent validation.

🟡 Minor comments (3)

src/dev-ui/app/tests/color-mode.test.ts-137-140 (1)

137-140: ⚠️ Potential issue | 🟡 Minor

Add the missing assertion for matchMedia non-usage.

The test comment says system preference is ignored when stored preference exists, but that behavior is not asserted.

Suggested patch

   it('does not apply system preference when stored preference exists', () => {
     localStorage.setItem('kartograph-color-mode', 'light')
     const mockMatchMedia = vi.fn().mockReturnValue({ matches: true })
     const isDark = { value: false }
@@
     initColorMode()
     // Stored 'light' takes precedence — system dark preference is ignored
     expect(isDark.value).toBe(false)
-    // matchMedia should not have been called (or called but result ignored since stored exists)
+    expect(mockMatchMedia).not.toHaveBeenCalled()
   })

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/dev-ui/app/tests/color-mode.test.ts` around lines 137 - 140, Add an
assertion to the test that verifies the system preference check was not used
when a stored preference exists: after asserting isDark.value is false, assert
that window.matchMedia was not called (or that the mock's call count is 0) so
the system dark preference is ignored; reference the test's isDark.value and the
window.matchMedia mock to locate where to insert this assertion.

.hyperloop/checks/check-cross-task-deferral.sh-14-15 (1)

14-15: ⚠️ Potential issue | 🟡 Minor

Regex pattern only matches exactly 3 digits, excluding the documented single-digit task formats.

Line 14 documents examples like task-0 and task-1, but line 39's regex "in task-[0-9][0-9][0-9]" requires exactly three digits. This mismatch will miss deferral comments for single and double-digit task numbers.

Suggested fix

-  "in task-[0-9][0-9][0-9]"
+  "in task-[0-9]+"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.hyperloop/checks/check-cross-task-deferral.sh around lines 14 - 15, The
regex "in task-[0-9][0-9][0-9]" only matches exactly three digits but the
comments mention task-0/task-1 (1–3 digits); update the pattern used in the
check to accept one to three digits (e.g., change "in task-[0-9][0-9][0-9]" to
"in task-[0-9]{1,3}" or "in task-[0-9]+") so single- and double-digit task
references are detected; apply this change where the regex literal/string is
defined in the script that currently contains "in task-[0-9][0-9][0-9]".

.hyperloop/checks/check-branch-rebased-on-alpha.sh-39-39 (1)

39-39: ⚠️ Potential issue | 🟡 Minor

Use git log --oneline -n 20 instead of piping to head.

With set -o pipefail enabled (line 14), the git log | head pipeline will exit with code 141 when head closes its stdin after 20 lines, causing git log to receive SIGPIPE. This causes the script to exit prematurely with the wrong exit code instead of the intended exit 1. Replace with git log --oneline -n 20 "${MERGE_BASE}..${BASE_BRANCH}".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.hyperloop/checks/check-branch-rebased-on-alpha.sh at line 39, Replace the
pipeline that runs git log piped into head (the line using git log --oneline
"${MERGE_BASE}..${BASE_BRANCH}" | head -20) with a single git log invocation
that limits output using the -n option (i.e. use git log --oneline -n 20 with
the same "${MERGE_BASE}..${BASE_BRANCH}" range) to avoid SIGPIPE when set -o
pipefail is enabled and preserve the intended exit behavior.

🧹 Nitpick comments (16)

src/api/tests/unit/graph/infrastructure/test_staging_table_manager.py (1)

61-183: Make SQL assertions order-insensitive and case-insensitive to reduce brittle failures

Several tests assume the first execute() call is the DDL (call_args_list[0]) and match uppercase SQL keywords exactly. These tests can fail on harmless refactors (extra pre-query, lowercase SQL) while behavior is still correct.

Refactor pattern

+def _all_executed_sql(cursor: MagicMock) -> list[str]:
+    return [str(call.args[0]).upper() for call in cursor.execute.call_args_list]
+
@@
-        create_sql = str(mock_cursor.execute.call_args_list[0][0][0])
-        assert "ON COMMIT DROP" in create_sql, (
+        sql_statements = _all_executed_sql(mock_cursor)
+        assert any("ON COMMIT DROP" in sql for sql in sql_statements), (
             "Graphid lookup table must use ON COMMIT DROP"
         )
@@
-        all_sqls = [str(c[0][0]) for c in mock_cursor.execute.call_args_list]
-        index_sqls = [s for s in all_sqls if "CREATE INDEX" in s and "logical_id" in s]
+        all_sqls = _all_executed_sql(mock_cursor)
+        index_sqls = [s for s in all_sqls if "CREATE INDEX" in s and "LOGICAL_ID" in s]

As per coding guidelines, "**: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/graph/infrastructure/test_staging_table_manager.py` around
lines 61 - 183, Tests that assert SQL content should not rely on call order or
exact casing; update assertions in tests for create_node_staging_table,
create_edge_staging_table, and create_graphid_lookup_table to collect all
executed SQL strings (use mock_cursor.execute.call_args_list), map them to
lowercase (e.g., [str(c[0][0]).lower() for c in ...]) and then perform
membership checks across that list for "on commit drop", "temp" or "temporary",
"start_graphid"/"end_graphid", "_ag_label_vertex", and for index creation look
for any statement containing "create index" and "logical_id"; keep the existing
checks that create_node_staging_table returns a name including the session id by
asserting the returned table_name contains the session id.

.hyperloop/checks/check-graceful-shutdown-cancel.sh (1)

19-19: Add explicit target-directory validation for clearer failures.

With set -e, a missing target dir exits abruptly via find. A direct guard gives a deterministic error and exit code.

Suggested fix

 TARGET_DIR="${1:-src}"
+
+if [[ ! -d "$TARGET_DIR" ]]; then
+    echo "check-graceful-shutdown-cancel: ERROR — target directory '$TARGET_DIR' does not exist" >&2
+    exit 2
+fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.hyperloop/checks/check-graceful-shutdown-cancel.sh at line 19, Add an
explicit validation after the TARGET_DIR="${1:-src}" assignment to check that
the directory exists and is a directory (e.g., test -d "$TARGET_DIR"); if the
check fails, print a clear error to stderr including the provided TARGET_DIR and
exit with a non-zero code so the script fails deterministically instead of
letting set -e surface an ambiguous find error.

.hyperloop/checks/check-fake-success-notifications.sh (1)

62-98: Deduplicate files before evaluation to avoid repeated reports.

A file matching multiple success patterns is processed multiple times, which can inflate found and duplicate output. Add a small dedupe map before API-call checks.

Proposed refactor

 found=0
+declare -A seen_files=()
@@
   for file in $success_files; do
+    if [[ -n "${seen_files[$file]:-}" ]]; then
+      continue
+    fi
+    seen_files["$file"]=1
+
     # Skip test files — mocked toasts in tests are expected
     if [[ "$file" == *".test."* ]] || [[ "$file" == *".spec."* ]]; then
       continue
     fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.hyperloop/checks/check-fake-success-notifications.sh around lines 62 - 98,
Currently a file that matches multiple SUCCESS_PATTERNS can be processed
repeatedly; after computing success_files add a dedupe step (e.g. declare -A
seen and skip duplicate filenames) before the inner "for file in $success_files;
do" loop so each filename is evaluated once (prevent duplicate output and
multiple increments of found); implement by adding something like "declare -A
seen" and at the top of the file loop check "if [[ -n \"${seen[$file]}\" ]];
then continue; fi; seen[$file]=1" so the subsequent API_CALL_PATTERNS check,
has_api_call logic, and found++ only happen once per unique file.

src/api/tests/unit/shared_kernel/job_package/test_reader.py (1)

255-275: Tighten path-safety tests to avoid false positives.

Right now the tests only assert a generic ValueError, so they can pass even if the failure is unrelated to unsafe entry names. Consider making the tampered archive otherwise schema-valid (e.g., include data in state.json) and matching the expected path-safety error reason.

Suggested test hardening

@@
-            zf.writestr("state.json", json.dumps({"schema_version": "1.0.0"}))
+            zf.writestr(
+                "state.json",
+                json.dumps({"schema_version": "1.0.0", "data": {}}),
+            )
@@
-        with pytest.raises(ValueError):
+        with pytest.raises(ValueError, match="[Pp]ath|[Tt]raversal|[Uu]nsafe|[Aa]bsolute"):
             JobPackageReader(evil_zip)
@@
-        with pytest.raises(ValueError):
+        with pytest.raises(ValueError, match="[Pp]ath|[Tt]raversal|[Uu]nsafe|[Aa]bsolute"):
             JobPackageReader(evil_zip)

Also applies to: 277-291

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/shared_kernel/job_package/test_reader.py` around lines 255
- 275, The test's tampered ZIP builder and assertions are too weak:
_build_tampered_zip currently only ensures a manifest/state but the test asserts
a generic ValueError which can mask unrelated failures; modify
_build_tampered_zip to produce an otherwise schema-valid archive (e.g., include
a valid "data" field in state.json and keep manifest/changeset valid) and then
update the test assertions to expect the specific path-safety failure (match the
exact exception type/message produced by the path-safety check) so the test only
passes when the failure is due to the unsafe entry name.

.hyperloop/checks/check-branch-has-commits.sh (1)

46-49: Avoid running git log twice for the same range

This executes the same log query twice. Cache once and reuse to reduce subprocess overhead.

Refactor suggestion

-if [[ -n "$(git log --oneline "$MERGE_BASE..HEAD" 2>/dev/null)" ]]; then
+ahead_log="$(git log --oneline "$MERGE_BASE..HEAD" 2>/dev/null || true)"
+if [[ -n "$ahead_log" ]]; then
   echo ""
-  git log --oneline "$MERGE_BASE..HEAD"
+  printf '%s\n' "$ahead_log"
 fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.hyperloop/checks/check-branch-has-commits.sh around lines 46 - 49, Cache
the git log output once instead of calling git log twice: run git log --oneline
"$MERGE_BASE..HEAD" once, store its output in a variable (e.g. commits), then
test the variable with [[ -n "$commits" ]] and print it if non-empty; update the
existing block that references MERGE_BASE and HEAD to use this cached variable
for both the emptiness check and the subsequent echo to eliminate the duplicate
subprocess call.

src/api/management/application/services/knowledge_graph_service.py (2)

295-297: Probe metadata is mislabeled for tenant-wide listing.

knowledge_graphs_listed(workspace_id=...) receives a tenant ID in this path, which can pollute workspace-level telemetry.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/management/application/services/knowledge_graph_service.py` around
lines 295 - 297, The probe call is passing a tenant-scoped ID into the
workspace_id parameter causing workspace-level telemetry pollution; update the
call to use the correct parameter name by passing
tenant_id=self._scope_to_tenant (or removing workspace_id and explicitly
providing tenant_id=self._scope_to_tenant) when invoking knowledge_graphs_listed
in KnowledgeGraphService so the probe receives the tenant identifier correctly
while keeping count=len(visible_kgs) unchanged.

---

285-294: list_all currently has sequential N+1 authorization calls.

Each KG incurs a separate awaited permission check in series, which can make this endpoint slow and spike authz backend load for large tenants.

Possible refactor (parallelized filtering)

+import asyncio
@@
-        visible_kgs: list[KnowledgeGraph] = []
-        for kg in all_kgs:
-            has_view = await self._check_permission(
-                user_id=user_id,
-                resource_type=ResourceType.KNOWLEDGE_GRAPH,
-                resource_id=kg.id.value,
-                permission=Permission.VIEW,
-            )
-            if has_view:
-                visible_kgs.append(kg)
+        async def _visible_or_none(kg: KnowledgeGraph) -> KnowledgeGraph | None:
+            has_view = await self._check_permission(
+                user_id=user_id,
+                resource_type=ResourceType.KNOWLEDGE_GRAPH,
+                resource_id=kg.id.value,
+                permission=Permission.VIEW,
+            )
+            return kg if has_view else None
+
+        checks = await asyncio.gather(*(_visible_or_none(kg) for kg in all_kgs))
+        visible_kgs = [kg for kg in checks if kg is not None]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/management/application/services/knowledge_graph_service.py` around
lines 285 - 294, The loop in list_all makes sequential N+1 calls to
_check_permission for each knowledge graph (ResourceType.KNOWLEDGE_GRAPH,
Permission.VIEW) which is slow; change list_all to perform permission checks in
parallel by creating coroutines for self._check_permission for every kg.id.value
(e.g., with asyncio.gather or an equivalent concurrent batch call), await them
all at once, and then filter all_kgs into visible_kgs based on the returned
booleans; keep the same semantics (user_id and permission) and preserve order if
needed.

src/api/management/dependencies/knowledge_graph.py (1)

49-53: Avoid duplicating encryption-key parsing logic across dependencies.

This parser sanitizes keys, but src/api/management/dependencies/data_source.py still uses plain split(","). The same env value can behave differently across endpoints. Prefer a shared parser/helper used in both places.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/management/dependencies/knowledge_graph.py` around lines 49 - 53,
Create a single shared parser for the encryption key env value (e.g.,
parse_encryption_keys(secret: str) -> list[str]) and use it instead of ad-hoc
.split(",") logic; implement the helper in a common module (utilities or a new
helpers file) and update the code that currently builds encryption_keys in
knowledge_graph.py (the encryption_keys list comprehension) and the plain
split(",") usage in data_source.py to call
parse_encryption_keys(settings.encryption_key.get_secret_value()), ensuring the
helper trims whitespace and ignores empty strings so both places behave
identically.

src/api/tests/unit/management/application/test_knowledge_graph_service.py (1)

649-650: Avoid asserting private aggregate internals in tests.

Line 649 and Line 650 assert DataSource._deleted directly, which makes tests brittle to internal refactors. Prefer asserting deletion via public behavior (e.g., emitted event or externally observable repository interaction contract).

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/management/application/test_knowledge_graph_service.py`
around lines 649 - 650, The test currently asserts the private attribute
DataSource._deleted on ds1/ds2, which is brittle; update the assertions to use
public behavior instead—e.g., verify the public deletion contract such as that
KnowledgeGraphService.delete_data_source() (or the aggregate's public delete
method) caused the expected repository call or emitted event: assert the
repository mock received a delete/mark_deleted call for the datasource IDs or
assert the domain event (e.g., DataSourceDeletedEvent) was published with those
IDs rather than checking DataSource._deleted directly; replace the two _deleted
checks with these public-facing assertions referencing ds1.id/ds2.id and the
repository or event publisher mock used in the test.

src/api/tests/unit/management/test_architecture.py (1)

250-257: Don't exempt most of the presentation layer from the IAM isolation rule.

Excluding management.presentation, both feature subpackages, and both routes* patterns means the new HTTP surface is no longer checked for accidental IAM imports. That weakens the architecture test enough that future non-auth coupling in these modules will slip through unnoticed. Prefer moving the auth bridge into a tiny dependency module and excluding only that bridge.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/management/test_architecture.py` around lines 250 - 257,
The test currently widens the IAM isolation exemption by excluding
"management.presentation", its two subpackages, and the "routes*" patterns
inside the .exclude(...) call in test_architecture.py, which disables checks for
new HTTP surface imports; update the test to remove the broad exclusions
("management.presentation", "management.presentation.knowledge_graphs",
"management.presentation.data_sources", and the "routes*" patterns) and instead
create a minimal auth bridge module (e.g., management.presentation.auth_bridge)
that houses the IAM/adaptors, then only exclude that single bridge module string
in the .exclude(...) call so the rest of the presentation layer remains subject
to the IAM isolation rule.

src/api/tests/unit/management/presentation/test_knowledge_graphs_routes.py (1)

1-5: Consolidate this with the other knowledge-graph route suite.

This file overlaps heavily with src/api/tests/unit/management/presentation/test_knowledge_graph_routes.py added in the same PR: same router, another copy of the fixture setup, and many of the same scenarios. Keeping both will make the response contract drift and doubles the maintenance cost for every route change.

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/management/presentation/test_knowledge_graphs_routes.py`
around lines 1 - 5, This file duplicates the knowledge-graph route test suite
already present in
src/api/tests/unit/management/presentation/test_knowledge_graph_routes.py;
consolidate by moving any unique tests from test_knowledge_graphs_routes.py into
test_knowledge_graph_routes.py, reuse the existing fixtures and router/test
client setup (remove duplicate fixture definitions), delete
test_knowledge_graphs_routes.py, and run test discovery to ensure all tests
still reference the same router and helper symbols (e.g., any test functions,
fixtures, or route names) so there are no duplicate or conflicting tests left.

src/api/management/presentation/knowledge_graphs/routes.py (1)

268-278: String matching on error messages is fragile.

Checking "not found" in error_msg to determine HTTP status code is brittle — it will break if the error message wording changes or is localized.

Consider having the service layer raise typed exceptions (e.g., KnowledgeGraphNotFoundError) instead of relying on message content.

♻️ Suggested approach

-    except ValueError as e:
-        error_msg = str(e)
-        if "not found" in error_msg:
-            raise HTTPException(
-                status_code=status.HTTP_404_NOT_FOUND,
-                detail=error_msg,
-            )
-        raise HTTPException(
-            status_code=status.HTTP_400_BAD_REQUEST,
-            detail=error_msg,
-        )
+    except KnowledgeGraphNotFoundError as e:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=str(e),
+        )
+    except ValueError as e:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail=str(e),
+        )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/management/presentation/knowledge_graphs/routes.py` around lines 268
- 278, Replace fragile string matching in the except block by handling a typed
exception from the service layer: have the service methods that currently raise
ValueError raise a specific KnowledgeGraphNotFoundError (or similar) and here
catch that exception explicitly (e.g., except KnowledgeGraphNotFoundError: raise
HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e))). Remove the
`"not found" in error_msg` check, keep a generic except ValueError to map to 400
for other validation errors, and add the necessary import for
KnowledgeGraphNotFoundError in routes.py so the exception type can be matched by
the except clause.

src/api/tests/unit/test_health.py (1)

236-250: Path to deployment YAML is fragile but acceptable for deployment validation.

The hardcoded relative path (Path(__file__).parents[4] / "deploy" / ...) will break if the test file moves. However, for tests that explicitly validate deployment configuration, this coupling is intentional and acceptable.

Consider adding a skip condition if the file doesn't exist to provide a clearer failure mode during refactoring.

💡 Optional: Add existence check

import pytest

_DEPLOY_YAML = (
    Path(__file__).parents[4]
    / "deploy"
    / "apps"
    / "kartograph"
    / "base"
    / "api-deployment.yaml"
)

pytestmark_deploy = pytest.mark.skipif(
    not _DEPLOY_YAML.exists(),
    reason=f"Deployment YAML not found at {_DEPLOY_YAML}"
)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/test_health.py` around lines 236 - 250, The test currently
uses a hardcoded _DEPLOY_YAML path and _load_init_containers() which will error
if the file is missing; add a pytest skip marker to the module (e.g.,
pytestmark_deploy) that uses pytest.mark.skipif(not _DEPLOY_YAML.exists(),
reason=...) so tests are skipped with a clear message when the deployment YAML
is absent; ensure you import pytest at top and reference the existing
_DEPLOY_YAML symbol and keep _load_init_containers unchanged.

src/api/management/presentation/data_sources/routes.py (1)

124-128: ValueError mapped unconditionally to 404 may hide validation errors.

All ValueError exceptions from service.create() are mapped to HTTP 404. If the service raises ValueError for validation issues (not just "KG not found"), those would incorrectly return 404 instead of 400.

Consider using a typed exception for not-found cases, or at minimum document that the service layer only raises ValueError for resource-not-found scenarios.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/management/presentation/data_sources/routes.py` around lines 124 -
128, The handler currently maps any ValueError from service.create() to HTTP 404
which can misclassify validation errors; update the code so it only converts a
dedicated "not found" exception to 404—add/use a specific exception type (e.g.
ResourceNotFoundError or DataSourceNotFound) thrown by service.create(), catch
that specific exception and raise HTTPException(status.HTTP_404_NOT_FOUND, ...)
for it, and for other ValueError instances either re-raise as HTTP 400
(validation) or let them bubble up; reference service.create(), ValueError, and
the HTTPException/status.HTTP_404_NOT_FOUND handling when making the change.

src/dev-ui/app/pages/data-sources/index.vue (2)

664-669: saveOntologyEdits is a stub that doesn't persist changes.

The function shows a success toast but doesn't actually call any API to save the ontology changes. The comment on line 657-658 acknowledges this ("In a real implementation this would load from the server"), but the save side is also unimplemented.

If this is intentional scaffolding for future work, consider adding a TODO comment or disabling the save button until the backend endpoint exists.

Do you want me to help scaffold the API call for persisting ontology changes, or open an issue to track this?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/dev-ui/app/pages/data-sources/index.vue` around lines 664 - 669, The
saveOntologyEdits function currently only shows a success toast and closes the
editor without persisting changes; update saveOntologyEdits to call the backend
persistence endpoint (e.g., use the existing API helper such as saveOntology,
updateOntology, or axios/fetch) with the current ontology payload, await the
result, handle errors by showing toast.error and not closing the editor on
failure, and only show toast.success and call closeOntologyEditor() after a
successful response; alternatively, if the backend isn't ready, add a clear TODO
comment in saveOntologyEdits and disable the save button (or short-circuit the
handler) so users can't trigger the non-persisting action.

---

377-427: Significant code duplication between wizard and editor ontology functions.

The functions startEditNode/saveEditNode/cancelEditNode (lines 377-427) are nearly identical to startEditNodeInEditor/saveEditNodeInEditor/cancelEditNodeInEditor (lines 431-473), differing only in which array they operate on (proposedNodes vs editNodes). The same applies to the edge editing functions.

Consider extracting a reusable composable or helper that accepts the target array as a parameter to reduce ~100 lines of duplication.

♻️ Example refactor approach

// Extract shared editing logic
function createOntologyEditor<T extends ProposedNodeType | ProposedEdgeType>(
  items: Ref<T[]>
) {
  return {
    startEdit(index: number) {
      const item = items.value[index]
      item.editLabel = item.label
      item.editDescription = item.description
      item.editRequired = item.required_properties.join(', ')
      item.editOptional = item.optional_properties.join(', ')
      item.editing = true
    },
    saveEdit(index: number) {
      const item = items.value[index]
      item.label = item.editLabel.trim() || item.label
      item.description = item.editDescription
      item.required_properties = item.editRequired.split(',').map(s => s.trim()).filter(Boolean)
      item.optional_properties = item.editOptional.split(',').map(s => s.trim()).filter(Boolean)
      item.editing = false
    },
    cancelEdit(index: number) {
      items.value[index].editing = false
    },
  }
}

Also applies to: 431-473

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/dev-ui/app/pages/data-sources/index.vue` around lines 377 - 427, The
node/edge editing functions (startEditNode, saveEditNode, cancelEditNode,
startEditEdge, saveEditEdge, cancelEditEdge and their InEditor counterparts)
duplicate logic; extract a reusable helper like createOntologyEditor(items:
Ref<...[]>) that returns startEdit, saveEdit, cancelEdit methods operating on
the passed Ref and replace both sets of functions to call the helper for
proposedNodes/proposedEdges and editNodes/editEdges respectively; ensure the
helper references the same properties used now (editLabel, editDescription,
editRequired, editOptional, required_properties, optional_properties, editing)
so behavior is unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 910bf73d-9d46-4ee1-80be-616dff542e1c

📥 Commits

Reviewing files that changed from the base of the PR and between 846926d and 36c9c12.

⛔ Files ignored due to path filters (2)

• src/api/uv.lock is excluded by !**/*.lock
• src/dev-ui/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (148)

• .agent-memory/spec-alignment-reviewer.md
• .gitignore
• .hyperloop/checks/check-branch-has-commits.sh
• .hyperloop/checks/check-branch-rebased-on-alpha.sh
• .hyperloop/checks/check-cross-task-deferral.sh
• .hyperloop/checks/check-domain-aggregate-mocks.sh
• .hyperloop/checks/check-empty-test-stubs.sh
• .hyperloop/checks/check-fake-success-notifications.sh
• .hyperloop/checks/check-frontend-deps-resolve.sh
• .hyperloop/checks/check-frontend-lockfile-frozen.sh
• .hyperloop/checks/check-frontend-tests-pass.sh
• .hyperloop/checks/check-graceful-shutdown-cancel.sh
• .hyperloop/checks/check-no-check-script-deletions.sh
• .hyperloop/checks/check-no-coming-soon-stubs.sh
• .hyperloop/checks/check-no-future-placeholder-comments.sh
• .hyperloop/checks/check-no-source-regressions.sh
• .hyperloop/checks/check-no-test-regressions.sh
• .hyperloop/checks/check-pages-have-tests.sh
• .hyperloop/checks/check-partial-error-assertions.sh
• .hyperloop/checks/check-process-overlays-intact.sh
• .hyperloop/checks/check-property-merge-semantics.sh
• .hyperloop/checks/check-route-handler-mock-coverage.sh
• .hyperloop/checks/check-selector-forwarding.sh
• .hyperloop/checks/check-weak-test-assertions.sh
• .hyperloop/intake/nfr-and-index-intake.md
• .hyperloop/state/reviews/task-001-round-0.md
• .hyperloop/state/reviews/task-007-round-0.md
• .hyperloop/state/reviews/task-007-round-1.md
• .hyperloop/state/reviews/task-008-round-0.md
• .hyperloop/state/reviews/task-008-round-1.md
• .hyperloop/state/reviews/task-010-round-0.md
• .hyperloop/state/reviews/task-014-round-0.md
• .hyperloop/state/reviews/task-014-round-1.md
• .hyperloop/state/reviews/task-017-round-0.md
• .hyperloop/state/reviews/task-017-round-1.md
• .hyperloop/state/reviews/task-018-round-0.md
• .hyperloop/state/reviews/task-020-round-0.md
• .hyperloop/state/tasks/task-001.md
• .hyperloop/state/tasks/task-002.md
• .hyperloop/state/tasks/task-003.md
• .hyperloop/state/tasks/task-004.md
• .hyperloop/state/tasks/task-005.md
• .hyperloop/state/tasks/task-006.md
• .hyperloop/state/tasks/task-007.md
• .hyperloop/state/tasks/task-008.md
• .hyperloop/state/tasks/task-009.md
• .hyperloop/state/tasks/task-010.md
• .hyperloop/state/tasks/task-011.md
• .hyperloop/state/tasks/task-012.md
• .hyperloop/state/tasks/task-013.md
• .hyperloop/state/tasks/task-014.md
• .hyperloop/state/tasks/task-015.md
• .hyperloop/state/tasks/task-016.md
• .hyperloop/state/tasks/task-017.md
• .hyperloop/state/tasks/task-018.md
• .hyperloop/state/tasks/task-019.md
• .hyperloop/state/tasks/task-020.md
• .hyperloop/state/tasks/task-021.md
• .hyperloop/state/tasks/task-022.md
• .hyperloop/state/tasks/task-023.md
• .hyperloop/state/tasks/task-024.md
• .hyperloop/state/tasks/task-025.md
• .hyperloop/state/tasks/task-026.md
• .hyperloop/state/tasks/task-027.md
• .hyperloop/state/tasks/task-028.md
• .hyperloop/state/tasks/task-029.md
• .hyperloop/state/tasks/task-030.md
• .hyperloop/state/tasks/task-031.md
• .hyperloop/worker-result.yaml
• .pre-commit-config.yaml
• src/api/graph/application/services/__init__.py
• src/api/graph/application/services/graph_secure_enclave.py
• src/api/graph/dependencies.py
• src/api/graph/domain/value_objects.py
• src/api/graph/infrastructure/age_bulk_loading/strategy.py
• src/api/health_routes.py
• src/api/iam/application/services/workspace_service.py
• src/api/iam/ports/exceptions.py
• src/api/iam/presentation/workspaces/routes.py
• src/api/infrastructure/settings.py
• src/api/main.py
• src/api/management/application/services/knowledge_graph_service.py
• src/api/management/dependencies/data_source.py
• src/api/management/dependencies/knowledge_graph.py
• src/api/management/presentation/__init__.py
• src/api/management/presentation/data_sources/__init__.py
• src/api/management/presentation/data_sources/models.py
• src/api/management/presentation/data_sources/routes.py
• src/api/management/presentation/knowledge_graphs/__init__.py
• src/api/management/presentation/knowledge_graphs/models.py
• src/api/management/presentation/knowledge_graphs/routes.py
• src/api/pyproject.toml
• src/api/shared_kernel/job_package/__init__.py
• src/api/shared_kernel/job_package/builder.py
• src/api/shared_kernel/job_package/checksum.py
• src/api/shared_kernel/job_package/path_safety.py
• src/api/shared_kernel/job_package/reader.py
• src/api/shared_kernel/job_package/value_objects.py
• src/api/tests/fakes/authorization.py
• src/api/tests/integration/iam/test_workspace_authorization.py
• src/api/tests/integration/management/test_data_source_authorization.py
• src/api/tests/integration/management/test_knowledge_graph_authorization.py
• src/api/tests/unit/graph/application/test_graph_secure_enclave.py
• src/api/tests/unit/graph/infrastructure/test_age_bulk_loading_strategy_partitioning.py
• src/api/tests/unit/graph/infrastructure/test_staging_table_manager.py
• src/api/tests/unit/iam/application/test_workspace_service.py
• src/api/tests/unit/iam/domain/test_exceptions.py
• src/api/tests/unit/iam/presentation/test_workspaces_routes.py
• src/api/tests/unit/infrastructure/test_cors_settings.py
• src/api/tests/unit/management/application/test_knowledge_graph_service.py
• src/api/tests/unit/management/presentation/__init__.py
• src/api/tests/unit/management/presentation/test_data_sources_routes.py
• src/api/tests/unit/management/presentation/test_knowledge_graph_routes.py
• src/api/tests/unit/management/presentation/test_knowledge_graphs_routes.py
• src/api/tests/unit/management/test_architecture.py
• src/api/tests/unit/shared_kernel/authorization/test_in_memory_provider.py
• src/api/tests/unit/shared_kernel/job_package/__init__.py
• src/api/tests/unit/shared_kernel/job_package/test_architecture.py
• src/api/tests/unit/shared_kernel/job_package/test_builder.py
• src/api/tests/unit/shared_kernel/job_package/test_checksum.py
• src/api/tests/unit/shared_kernel/job_package/test_path_safety.py
• src/api/tests/unit/shared_kernel/job_package/test_reader.py
• src/api/tests/unit/shared_kernel/job_package/test_value_objects.py
• src/api/tests/unit/test_application_lifecycle.py
• src/api/tests/unit/test_cors_middleware.py
• src/api/tests/unit/test_health.py
• src/dev-ui/app/composables/api/useQueryApi.ts
• src/dev-ui/app/layouts/default.vue
• src/dev-ui/app/pages/data-sources/index.vue
• src/dev-ui/app/pages/knowledge-graphs/index.vue
• src/dev-ui/app/tests/api-keys.test.ts
• src/dev-ui/app/tests/color-mode.test.ts
• src/dev-ui/app/tests/data-sources.test.ts
• src/dev-ui/app/tests/design-language-extended.test.ts
• src/dev-ui/app/tests/design-system.test.ts
• src/dev-ui/app/tests/graph-explorer.test.ts
• src/dev-ui/app/tests/index.test.ts
• src/dev-ui/app/tests/interaction-principles.test.ts
• src/dev-ui/app/tests/knowledge-graphs.test.ts
• src/dev-ui/app/tests/mcp-integration.test.ts
• src/dev-ui/app/tests/query-history.test.ts
• src/dev-ui/app/tests/responsive-design.test.ts
• src/dev-ui/app/tests/schema-browser.test.ts
• src/dev-ui/app/tests/sync-monitoring-extended.test.ts
• src/dev-ui/app/tests/workspace-management.test.ts
• src/dev-ui/package.json
• src/dev-ui/vitest.config.ts
• website/src/data/env-vars.json

💤 Files with no reviewable changes (43)

• .hyperloop/state/tasks/task-012.md
• .hyperloop/state/tasks/task-003.md
• .hyperloop/state/tasks/task-026.md
• .hyperloop/state/tasks/task-006.md
• .hyperloop/state/tasks/task-009.md
• .hyperloop/state/tasks/task-019.md
• .hyperloop/state/tasks/task-015.md
• .hyperloop/state/tasks/task-031.md
• .hyperloop/state/reviews/task-008-round-1.md
• .hyperloop/state/tasks/task-001.md
• .hyperloop/state/reviews/task-018-round-0.md
• .hyperloop/state/reviews/task-010-round-0.md
• .hyperloop/state/tasks/task-010.md
• .hyperloop/state/tasks/task-014.md
• .hyperloop/state/tasks/task-020.md
• .hyperloop/state/tasks/task-002.md
• .hyperloop/state/tasks/task-013.md
• .hyperloop/state/tasks/task-007.md
• .hyperloop/state/tasks/task-021.md
• .hyperloop/state/tasks/task-016.md
• .hyperloop/state/tasks/task-030.md
• .hyperloop/state/tasks/task-022.md
• .hyperloop/state/tasks/task-011.md
• .hyperloop/state/reviews/task-017-round-1.md
• .hyperloop/state/tasks/task-029.md
• .hyperloop/state/tasks/task-017.md
• .hyperloop/state/reviews/task-008-round-0.md
• .hyperloop/state/tasks/task-018.md
• .hyperloop/state/tasks/task-005.md
• .hyperloop/state/tasks/task-004.md
• .hyperloop/state/tasks/task-025.md
• .hyperloop/state/tasks/task-024.md
• .hyperloop/state/tasks/task-023.md
• .hyperloop/state/reviews/task-001-round-0.md
• .hyperloop/state/reviews/task-007-round-0.md
• .hyperloop/state/reviews/task-014-round-1.md
• .hyperloop/state/reviews/task-017-round-0.md
• .hyperloop/state/reviews/task-020-round-0.md
• .hyperloop/state/tasks/task-027.md
• .hyperloop/state/tasks/task-028.md
• .hyperloop/state/tasks/task-008.md
• .hyperloop/state/reviews/task-014-round-0.md
• .hyperloop/state/reviews/task-007-round-1.md

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Prevent post-commit observability errors from re-running the batch

If probe.apply_batch_completed(...) at Line 179 throws after conn.commit() at Line 176, the except block retries the full batch, which can duplicate already committed mutations.

Proposed fix

-                duration_ms = (time.perf_counter() - start_time) * 1000
-                probe.apply_batch_completed(
-                    total_operations=len(operations),
-                    total_batches=total_batches,
-                    duration_ms=duration_ms,
-                    success=True,
-                )
+                duration_ms = (time.perf_counter() - start_time) * 1000
+                try:
+                    probe.apply_batch_completed(
+                        total_operations=len(operations),
+                        total_batches=total_batches,
+                        duration_ms=duration_ms,
+                        success=True,
+                    )
+                except Exception:
+                    # Observability must not trigger mutation retries after commit.
+                    pass
 
                 return MutationResult(success=True, operations_applied=len(operations))

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

Also applies to: 188-198

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/graph/infrastructure/age_bulk_loading/strategy.py` around lines 176 -
184, After committing the DB transaction (conn.commit()) wrap the post-commit
observability call probe.apply_batch_completed(...) in its own try/except that
catches and logs any exceptions but does not re-raise them so the outer
retry/except path will not re-run an already-committed batch; apply the same
pattern to the second occurrence around lines 188-198 (i.e., isolate
probe.apply_batch_completed failures from transaction control), and ensure the
log includes enough context (batch id/total_operations/duration_ms) while
leaving the commit outcome unchanged.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Real-path dedup makes checksum non-deterministic across traversal orders

This can drop valid symlink entries depending on scan order, so identical content trees may hash differently.

Suggested fix

-    visited_real_paths: set[Path] = set()
+    seen_normalized_paths: set[str] = set()
@@
-            # Cycle detection via resolved paths
-            if resolved in visited_real_paths:
-                continue
-
             if not resolved.is_file():
                 continue
@@
-        visited_real_paths.add(actual_path)
-        entries.append((normalized, actual_path))
+        if normalized in seen_normalized_paths:
+            continue
+        seen_normalized_paths.add(normalized)
+        entries.append((normalized, actual_path))

Also applies to: 92-93

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/shared_kernel/job_package/checksum.py` around lines 56 - 70, The
current logic skips symlink entries entirely when their resolved path is already
in visited_real_paths, causing non-deterministic checksum differences based on
traversal order; change it so symlink directory entries are not dropped simply
because their resolved path was seen—only use visited_real_paths to prevent
recursive traversal into targets (i.e., before recursing into a resolved
directory, check/update visited_real_paths), but always include the symlink
entry itself in the checksum list; apply the same change to the analogous check
at the other site that references visited_real_paths (lines around the second
occurrence).

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python - <<'PY'
try:
    from fastapi import status
    print("HTTP_422_UNPROCESSABLE_CONTENT =", hasattr(status, "HTTP_422_UNPROCESSABLE_CONTENT"))
    print("HTTP_422_UNPROCESSABLE_ENTITY  =", hasattr(status, "HTTP_422_UNPROCESSABLE_ENTITY"))
except Exception as exc:
    print(f"IMPORT_ERROR: {exc}")
PY

rg -n "HTTP_422_UNPROCESSABLE_(CONTENT|ENTITY)" src/api/tests/unit/management/presentation/test_knowledge_graph_routes.py src/api/tests/unit/management/presentation/test_knowledge_graphs_routes.py

Repository: openshift-hyperfleet/kartograph

Length of output: 890

---

🏁 Script executed:

#!/bin/bash
# Find all dependency files
find . -maxdepth 3 -type f \( -name "requirements*.txt" -o -name "pyproject.toml" -o -name "setup.py" -o -name "poetry.lock" -o -name "Pipfile" \) | head -20

Repository: openshift-hyperfleet/kartograph

Length of output: 100

---

🏁 Script executed:

#!/bin/bash
# Search for any custom status constant definitions or imports in the codebase
rg -n "HTTP_422_UNPROCESSABLE" --type py -B 2 -A 2 | head -100

Repository: openshift-hyperfleet/kartograph

Length of output: 8610

---

🏁 Script executed:

#!/bin/bash
# Check if status constants are imported or defined anywhere custom
rg -n "from fastapi|from starlette" src/api/tests/unit/management/presentation/ | grep status

Repository: openshift-hyperfleet/kartograph

Length of output: 412

---

🏁 Script executed:

#!/bin/bash
cat src/api/pyproject.toml | head -50

Repository: openshift-hyperfleet/kartograph

Length of output: 1302

---

Replace HTTP_422_UNPROCESSABLE_CONTENT with HTTP_422_UNPROCESSABLE_ENTITY — constant does not exist in FastAPI.

The constant HTTP_422_UNPROCESSABLE_CONTENT does not exist in FastAPI/Starlette. The pinned version (>=0.123.9) only exposes HTTP_422_UNPROCESSABLE_ENTITY. Other test files in the same directory correctly use HTTP_422_UNPROCESSABLE_ENTITY (e.g., test_knowledge_graphs_routes.py:287). These assertions (lines 230, 246, 643, 659) will fail with AttributeError at runtime.

Fix all four occurrences

- assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT
+ assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY

📝 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

	def test_create_knowledge_graph_returns_422_for_empty_name(
	self,
	test_client: TestClient,
	mock_kg_service: AsyncMock,
	) -> None:
	"""Returns 422 Unprocessable Entity when name is empty (Pydantic validation)."""
	workspace_id = "01JT0000000000000000000002"
	response = test_client.post(
	f"/management/workspaces/{workspace_id}/knowledge-graphs",
	json={"name": "", "description": "desc"},
	)
	assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT
	def test_create_knowledge_graph_returns_422_for_oversized_name(
	self,
	test_client: TestClient,
	mock_kg_service: AsyncMock,
	) -> None:
	"""Returns 422 Unprocessable Entity when name exceeds 100 characters."""
	workspace_id = "01JT0000000000000000000002"
	long_name = "x" * 101
	response = test_client.post(
	f"/management/workspaces/{workspace_id}/knowledge-graphs",
	json={"name": long_name, "description": "desc"},
	)
	assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT
	def test_create_knowledge_graph_returns_422_for_empty_name(
	self,
	test_client: TestClient,
	mock_kg_service: AsyncMock,
	) -> None:
	"""Returns 422 Unprocessable Entity when name is empty (Pydantic validation)."""
	workspace_id = "01JT0000000000000000000002"
	response = test_client.post(
	f"/management/workspaces/{workspace_id}/knowledge-graphs",
	json={"name": "", "description": "desc"},
	)
	assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
	def test_create_knowledge_graph_returns_422_for_oversized_name(
	self,
	test_client: TestClient,
	mock_kg_service: AsyncMock,
	) -> None:
	"""Returns 422 Unprocessable Entity when name exceeds 100 characters."""
	workspace_id = "01JT0000000000000000000002"
	long_name = "x" * 101
	response = test_client.post(
	f"/management/workspaces/{workspace_id}/knowledge-graphs",
	json={"name": long_name, "description": "desc"},
	)
	assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/tests/unit/management/presentation/test_knowledge_graph_routes.py`
around lines 217 - 246, The tests
test_create_knowledge_graph_returns_422_for_empty_name and
test_create_knowledge_graph_returns_422_for_oversized_name (and the other two
failing assertions you noted) are using the incorrect constant
HTTP_422_UNPROCESSABLE_CONTENT; replace each occurrence with the correct
FastAPI/Starlette constant HTTP_422_UNPROCESSABLE_ENTITY in those assertions so
they reference the existing status constant (e.g., change assert
response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT to assert
response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY in the test
functions named test_create_knowledge_graph_returns_422_for_empty_name,
test_create_knowledge_graph_returns_422_for_oversized_name and the two other
occurrences referenced).

[jsell-rh]

Hyperloop: Worker Crashed

Worker implementer crashed on branch hyperloop/task-008.

The orchestrator will handle recovery.