Knowledge graphs

[lumburovskalina]

Main PR: code-kern-ai/refinery-ui#88

[andhreljaKern]

Code Review - refinery-gateway #371 (Knowledge Graphs)

Fresh review against code-kern-ai backend standards.

---

🔴 Critical Issues

1. Loop Without Proper Transaction Management

File: controller/data_block/manager.py L1240-1247

def delete_many(org_id: str, project_id: str, ids: Optional[List[str]] = None) -> None:
    for id in ids:
        data_block_attribute_manager.delete_attributes(id)  # ❌ May commit inside loop
        s3.delete_object(str(org_id), str(project_id) + "/data-blocks/" + id + "/docbin_full")
    
    data_block_db_bo.delete_many(org_id, project_id, ids, with_commit=True)

Problem: Loop operations without with_commit=False. If delete_attributes() commits, partial state persists on failure.

Fix:

def delete_many(org_id: str, project_id: str, ids: Optional[List[str]] = None) -> None:
    try:
        for id in ids:
            data_block_attribute_manager.delete_attributes(id, with_commit=False)
            s3.delete_object(...)
        data_block_db_bo.delete_many(org_id, project_id, ids, with_commit=False)
        general.flush_or_commit()
    except Exception as e:
        general.rollback()
        raise

• Add proper transaction management

---

2. FastAPI Schema Violation

File: controller/data_block/attribute.py L732

def create(
    data_block_id: str,
    name: str,
    data_type: str = DataTypes.TEXT.value,
    user_created: bool = False,
    relative_position: Optional[int] = None,
    source_code: Optional[str] = "",  # ❌ WRONG
    state: Optional[str] = None,
    additional_config: Optional[Dict[str, Any]] = None,
) -> DataBlockAttribute:

Problem: Optional[str] = "" violates standard. Per code-kern-ai standards:

• Optional parameters MUST default to None
• Non-None defaults should NEVER use Optional

Fix:

source_code: str = "",  # Remove Optional
# OR
source_code: Optional[str] = None,  # Change default to None

• Fix FastAPI schema pattern

---

✅ Good Practices

1. Perfect Transaction Management in Loops

File: controller/data_block/attribute.py L825-849

for attribute_item in attribute_items:
    attribute_id = str(attribute_item.id)
    if is_usable:
        data_block_manager.delete_user_created_attribute(
            data_block_id=data_block_id,
            attribute_id=attribute_id,
            with_commit=False,  # ✓ Perfect!
        )
    data_block_attributes_db_bo.delete(
        data_block_id, attribute_id, with_commit=False  # ✓ Perfect!
    )

general.commit()  # ✓ Single commit after loop!

Excellent: This is the correct pattern from standards.

---

2. Proper Use of general Module

All DB operations use general.rollback(), general.commit(), general.flush_or_commit() - not session.

---

3. Project Access Control

File: fast_api/routes/data_block.py L1709-1713

@router.get(
    "/single/{project_id}/{data_block_id}",
    dependencies=[Depends(auth_manager.check_project_access_dep)],  # ✓ Correct!
)
def get(request: Request, project_id: str, data_block_id: str):

All endpoints with project_id have proper access control.

---

4. Consistent with_commit Pattern

Functions properly support with_commit parameter:

def update_add_user_created_attribute(..., with_commit: bool = False):
    # ... operations ...
    general.flush_or_commit(with_commit)  # ✓ Correct!

---

📊 Review Summary

Critical Issues: 2
Good Practices: 4

---

Priority Fixes

1. Fix delete_many() transaction management (Issue #1)
2. Fix FastAPI schema for source_code parameter (Issue #2)

---

Status: ⚠️ Changes Requested - 2 critical fixes needed

Estimated Fix: 30 minutes

---

Reviewed against code-kern-ai backend standards

[andhreljaKern]

requires an alembic update before merge to include sound notifications

• resolved