Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary of ChangesHello, 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 refines the narrative schema pack to ensure a deterministic, enforceable data model across both graph and relational stores. The changes aim to eliminate schema drift, improve validation, and provide a canonical, machine-validateable provenance receipt for downstream pipelines to assert and audit evidence lineage and run metadata. This enhancement provides a more robust and consistent foundation for narrative data management. 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
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces several new schema files to define a narrative data model across different data stores. The changes are well-structured and provide a solid foundation. My review includes a few suggestions to enhance data integrity, validation, and consistency. Specifically, I've recommended adding a pattern for span in the JSON schema, ensuring all properties are required where appropriate in the YAML schema, completing the index definitions, and adding ON DELETE CASCADE to foreign keys in the SQL schema for better referential integrity.
| "type": "string", | ||
| "pattern": "^sha256:[a-f0-9]{64}$" | ||
| }, | ||
| "span": {"type": "string"} |
There was a problem hiding this comment.
The span property is currently defined as a generic string. To improve data validation and clarity, consider adding a pattern to enforce a specific format, such as character offsets (e.g., '120:184'). A description would also be helpful for consumers of the schema.
| "span": {"type": "string"} | |
| "span": {"type": "string", "pattern": "^\\d+:\\d+$", "description": "A pointer to a segment of the input artifact, e.g., character offsets '120:184'."} |
| hash_algo: {type: string} | ||
| hash: {type: string} | ||
| size_bytes: {type: integer} |
There was a problem hiding this comment.
The properties hash_algo, hash, and size_bytes within the integrity object are not marked as required. If an integrity object is provided, it's likely that all its properties should be present to be meaningful. Consider making them required to ensure data completeness.
hash_algo: {type: string, required: true}
hash: {type: string, required: true}
size_bytes: {type: integer, required: true}| indexes: | ||
| - label: Artifact | ||
| props: [artifact_id] | ||
| - label: Actor | ||
| props: [actor_id, handle] | ||
| - label: Narrative | ||
| props: [narrative_id, state] | ||
| - label: Frame | ||
| props: [frame_id, narrative_id] | ||
| - label: Assumption | ||
| props: [assumption_id] | ||
| - label: Claim | ||
| props: [claim_id] |
There was a problem hiding this comment.
The indexes section appears to be incomplete. To maintain consistency with the defined constraints (which imply indexes on primary IDs) and the explicit indexing of other properties, it would be clearer to list all intended indexed properties. The current list is missing indexes for Community, Event, and Evidence nodes on their respective ID properties.
indexes:
- label: Artifact
props: [artifact_id]
- label: Actor
props: [actor_id, handle]
- label: Narrative
props: [narrative_id, state]
- label: Frame
props: [frame_id, narrative_id]
- label: Assumption
props: [assumption_id]
- label: Claim
props: [claim_id]
- label: Community
props: [community_id]
- label: Event
props: [event_id]
- label: Evidence
props: [evidence_id]| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS narrative_metric ( | ||
| run_id TEXT NOT NULL REFERENCES narrative_run(run_id), |
There was a problem hiding this comment.
To improve referential integrity, consider adding ON DELETE CASCADE to the foreign key constraint on run_id. This ensures that when a record in narrative_run is deleted, all corresponding records in narrative_metric are automatically removed, preventing orphaned rows.
run_id TEXT NOT NULL REFERENCES narrative_run(run_id) ON DELETE CASCADE,| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS narrative_transition ( | ||
| run_id TEXT NOT NULL REFERENCES narrative_run(run_id), |
There was a problem hiding this comment.
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS narrative_handoff ( | ||
| run_id TEXT NOT NULL REFERENCES narrative_run(run_id), |
There was a problem hiding this comment.
|
Temporarily closing to reduce Actions queue saturation and unblock #22241. Reopen after the golden-main convergence PR merges. |
1 similar comment
|
Temporarily closing to reduce Actions queue saturation and unblock #22241. Reopen after the golden-main convergence PR merges. |
Motivation
Description
schemas/narrative_graph.schema.ymlto include full node coverage (Artifact, Actor, Narrative, Frame, Claim, Assumption, Community, Event, Evidence), relationships, indexes, and uniqueness constraints for all primary IDs.schemas/neo4j/narrative_constraints.cypherto create UNIQUE constraints for id properties and indexes foractor.handle,narrative.state, andframe.narrative_idto match YAML schema expectations.schemas/postgres/narrative_metrics.sqldefiningnarrative_run,narrative_metric,narrative_transition, andnarrative_handofftables and narrative-scoped indexes for performant metrics and transition queries.docs/provenance/receipt.schema.jsonas a formal JSON Schema for deterministic provenance receipts and updatedocs/roadmap/STATUS.jsonto record the schema pack refinement delivery.Testing
node scripts/check-boundaries.cjs, which completed successfully with no boundary violations.pnpm lint,pnpm test, andmake smoke) in CI prior to merge for full verification.Codex Task