Feat/knowledge graphs#9
Conversation
Summary of ChangesHello @robfrank, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new, self-contained Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace SEARCH_CLASS() with SEARCH_INDEX('Paper[abstract]', ...) for
full-text search (Q3): SEARCH_CLASS is not a known ArcadeDB function
- Replace Q5 MATCH subquery with hardcoded IDs ['p2','p8','p4'] as
ArcadeDB does not support subqueries inside MATCH where clauses
- Fix docker-compose.yml healthcheck to use wget instead of curl since
the arcadedb container image does not include curl
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-engine Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- docker-compose.yml: switch healthcheck from wget/CMD-SHELL to curl/CMD, matching recommendation-engine and avoiding wget dependency in the image - sql/02-data.sql: replace date() with fixed ISO 8601 datetime strings to correct the DATE/DATETIME type mismatch and make test data reproducible - setup.sh: add retry counter (30 × 2s = 60s max) to the startup loop so ArcadeDB failures surface quickly instead of hanging until CI timeout - queries/queries.sh: document that Query 5 paper IDs are hardcoded due to ArcadeDB not supporting SELECT subqueries inside MATCH where clauses - remove redundant .gitkeep files from queries/ and java/examples/ dirs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
521cf97 to
a74cf18
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a new knowledge-graphs use case, including the application code in Java, query scripts, SQL schema and data, and associated documentation and implementation plans. The implementation is well-structured, mirroring the existing recommendation-engine use case, and the code is clean and correct. The use of fixed versions, robust shell scripting, and clear documentation is commendable.
My review focuses on improving the implementation plan documents to align them with the superior implementation, ensuring that the documentation remains accurate and maintainable. I've identified a few areas in docs/plans/2026-02-25-knowledge-graphs.md where the plan could be updated to reflect the more robust and reproducible code that was actually implemented.
I am having trouble creating individual review comments. Click here to see my feedback.
docs/plans/2026-02-25-knowledge-graphs.md (136-139)
The setup.sh script in the implementation includes a timeout for waiting for ArcadeDB to start, which is more robust than the infinite loop in this plan. Consider updating the plan to reflect this improvement. The implemented version is:
retries=0
until curl -sf -u "${ARCADEDB_USER}:${ARCADEDB_PASS}" \
"${ARCADEDB_URL}/api/v1/ready" > /dev/null 2>&1; do
(( retries++ >= 30 )) && { echo "ArcadeDB did not start after 60s"; exit 1; }
sleep 2
donedocs/plans/2026-02-25-knowledge-graphs.md (340-349)
The plan uses date() for timestamps in PaperActivity records. This makes time-series queries non-deterministic because the results would depend on when the setup script is run. The implementation correctly uses fixed timestamp strings (e.g., '2024-01-15T10:00:00'). The plan should be updated to use fixed timestamps for reproducibility.
docs/plans/2026-02-25-knowledge-graphs.md (441)
The plan for Query 3 (Full-Text Search) uses SEARCH_CLASS. However, the implementation correctly uses SEARCH_INDEX('Paper[abstract]', '...') which is more specific and aligns with the created FULL_TEXT index on the abstract property. The plan (including the queries.sh, KnowledgeGraph.java, and README.md sections) should be updated to use SEARCH_INDEX for consistency and correctness.
WHERE SEARCH_INDEX('Paper[abstract]', 'distributed AND consensus') = true
PR Review: knowledge-graphs use caseOverall this is a clean, well-structured addition that follows the What's working well
Issues worth addressing1. The script runs under query "cypher" "..." || echo "[Query 1 FAILED]"2.
3. Hardcoded Query 5 seed IDs are difficult to verify The comment says the IDs 4.
# NOTE: each SQL statement must occupy exactly one line in the file.Nits
SummaryThe blocking issues are minor — the most actionable ones are the |
No description provided.