Knowledge graphs

[lumburovskalina]

Main PR

Related PRs:

• Knowledge graphs #88
• Knowledge graphs submodule-react-components#66
• Knowledge graphs refinery-gateway#371
• Knowledge graphs refinery-submodule-model#209
• https://github.com/code-kern-ai/cognition-ui/pull/188
• Knowledge graphs submodule-javascript-functions#26
• https://github.com/code-kern-ai/cognition-gateway/pull/289
• Knowledge graphs refinery-ac-exec-env#90
• https://github.com/code-kern-ai/cognition-task-master/pull/42

How to test: https://www.notion.so/kern-ai/Test-Findings-refinery-1-23-0-cognition-1-11-0-2f6007cbb4c080788da2c470a12584ad?source=copy_link#2f6007cbb4c0810db1b0f7abbe2f5723

[JWittmeyer]

Hitting Select / Unselect actions on the overview cause the blocks to disappear

• resolved

[JWittmeyer]

When i click on execute on a stable query nothing happens

• resolved

[JWittmeyer]

(cognition)
Datablock is_ready_check is not working

• resolved

[JWittmeyer]

(cognition)
Fact display doesn't work

• resolved
• discussed

@andhreljaKern BE changes right?

Comment by @andhreljaKern :

Discussed and postponed for Testing session improvements.

[JWittmeyer]

(cognition)
Data block progress message isn't set (not visible in widget & edit only shows the enum)

• resolved

[JWittmeyer]

Having clause is currently not supported for complex analysis (e.g. count(*) > 1) though it might be good enough with sort 🤔

• discussed

[andhreljaKern]

Code Review - refinery-ui #88 (Knowledge Graphs)

Fresh review against code-kern-ai UI standards.

---

🔴 Critical Issues

1. Error Messages Concatenated Without Separators

File: DataBlocksDetailsOverview.tsx L1164-1184

let error = '';
if (res.denyReason.select) {
    error += 'SELECT clause is NOT valid: ' + res.denyReason.select;
}
if (res.denyReason.where) {
    error += 'WHERE clause is NOT valid: ' + res.denyReason.where;  // No separator!
}
// ... more concatenations
alert(error);  // Displays as one unreadable string

Result: "SELECT clause is NOT valid: ...WHERE clause is NOT valid: ..." (unreadable)

Fix: Add \n between messages:

if (res.denyReason.select) {
    error += 'SELECT clause is NOT valid: ' + res.denyReason.select + '\n';
}

• Add newlines between error messages

---

2. Using alert() - Poor UX

File: DataBlocksDetailsOverview.tsx L1161, L1184

alert('Everything is valid');  // ❌ Blocks UI
alert(error);                   // ❌ Blocks UI

Problem: Per UI standards, should use toast notifications, not alert().

Fix:

toast.success('Everything is valid');
toast.error(error);

• Replace alert() with toast

---

3. Missing Dependencies in useCallback

File: ConfirmExecutionModal.tsx L35-37

const calculateUserAttributeAllRecords = useCallback(() => {
    const attributeId = props.dataBlockId ? router.query.columnId as string : props.currentAttributeId;
    calculateUserAttributeAllRecordsPost(projectId, { attributeId, dataBlockId: props.dataBlockId }, (res) => { });
}, [modalExecuteAll, props.currentAttributeId, props.dataBlockId]);  // ❌ Missing router

Problem: Uses router.query but router not in dependencies.

Fix:

}, [modalExecuteAll, props.currentAttributeId, props.dataBlockId, router]);

• Add router to dependencies

---

File: DataBlocksDetailsOverview.tsx L1298-1302

const handleWebsocketNotification = useCallback((msgParts: string[]) => {
    if (msgParts[1] == 'calculate_attribute') {
        refetchDataBlockById();
    }
}, [currentDataBlock]);  // ❌ Uses refetchDataBlockById but not in deps

Fix:

}, [refetchDataBlockById]);

• Fix dependency array

---

4. JSON.parse Without try/catch

File: ExecutionContainer.tsx L89

sampleRecordsFinal.calculatedAttributesList = sampleRecordsFinal.calculatedAttributes.map((record: string) => JSON.parse(record));  // ❌ Can throw

Fix:

try {
    sampleRecordsFinal.calculatedAttributesList = sampleRecordsFinal.calculatedAttributes.map(
        (record: string) => JSON.parse(record)
    );
} catch (e) {
    console.error('Failed to parse attributes:', e);
    toast.error('Error parsing data');
}

• Add error handling

---

✅ Good Practices

1. Proper Redux Usage

const projectId = useSelector(selectProjectId);
const orgId = useSelector(selectOrganizationId);
const dispatch = useDispatch();

---

2. Optional Chaining

dataBlock?.sqlData?.length ?? 100
getRecordByRecordIdDataBlockColumn(projectId, dataBlock?.id, recordId, ...)

---

3. WebSocket Integration

useWebsocket(orgId, Application.REFINERY, CurrentPage.DATA_BLOCKS_DETAILS, handleWebsocketNotification, projectId);

---

📊 Review Summary

Critical Issues: 4
Good Practices: 3

---

Priority Fixes

Must Fix:

1. Error message concatenation (Issue #1)
2. Dependency arrays (Issue #3)
3. JSON.parse error handling (Issue #4)

Should Fix:
4. Replace alert() with toast (Issue #2)

---

Status: ⚠️ Changes Requested - 4 critical fixes needed

Estimated Fix: 2 hours

---

Reviewed against code-kern-ai UI standards

[JWittmeyer]

Let's run the names through a capitalization (or enumtoArray should also work)

• resolved

[JWittmeyer]

After switching to a different project the values where still on the ones from the old project. maybe we can clear on either desction or project id change

• resolved