fix: align narrative state outputs with schema#19500
Conversation
BrianCLong
added
the
codex
ChatGPT Codex Connector
|
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 establishes a new Narrative CI pipeline designed to produce schema-valid and deterministic outputs. It introduces formal JSON schemas for various narrative-related data structures, implements mechanisms for consistent output generation through configuration hashing and stable JSON serialization, and integrates Open Policy Agent (OPA) policies to enforce data quality and traceability. The changes also include comprehensive updates to documentation and evidence bundling to align with the new pipeline's structure and outputs, ensuring reliable operation for downstream validation and policy gates. 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
|
|
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 (26)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new narrative CI pipeline with a strong focus on creating deterministic and schema-valid outputs. The changes are comprehensive, adding new schemas, OPA policies for validation, and several pipeline steps. My review focuses on ensuring the implementation aligns with the goal of determinism and follows good software engineering practices. I've identified a few areas for improvement, including some code duplication that could be refactored, a potential source of non-determinism in the evidence bundling step, and a minor data inconsistency. Overall, this is a solid contribution that significantly enhances the CI process.
| }; | ||
|
|
||
| const stamp = { | ||
| generated_at: new Date().toISOString(), |
There was a problem hiding this comment.
Using new Date().toISOString() introduces a non-deterministic value into the stamp.json file. This seems to contradict the pull request's primary goal of ensuring deterministic outputs. Every run will produce a different stamp.json, and thus a different evidence bundle hash. If full determinism is desired for fixture runs, consider using a fixed timestamp or sourcing the timestamp from a stable source (e.g., commit timestamp).
| "incomplete": 0, | ||
| "not_started": 5, | ||
| "total": 17, | ||
| "total": 21, |
There was a problem hiding this comment.
The total in the summary appears to be inconsistent. It was increased from 17 to 21 (a jump of 4), but only one new item was added to the initiatives array. Furthermore, the sum of rc_ready (8), partial (2), incomplete (0), and not_started (5) is 15, which does not match either the old or the new total. Please verify and correct the summary values to ensure data consistency.
| export function stableHash(input: string): string { | ||
| return createHash('sha256').update(input).digest('hex'); | ||
| } |
There was a problem hiding this comment.
This stableHash function duplicates the functionality of the sha256 function already defined in intelgraph/pipelines/narrative_ci/lib/hash.ts. To avoid code duplication and improve maintainability, this function should be removed. Please use the sha256 function from hash.ts instead.
| process.exit(1); | ||
| } | ||
|
|
||
| const ajv = new Ajv({ allErrors: true, strict: false }); |
There was a problem hiding this comment.
Using strict: false with AJV is not recommended as it can miss potential issues in schemas. Please consider setting strict: true to enable stricter validation and catch a wider range of schema errors. If strict: false is required for a specific reason, please add a comment explaining why.
const ajv = new Ajv({ allErrors: true, strict: true });| import { mkdir, readFile, writeFile } from 'node:fs/promises'; | ||
| import path from 'node:path'; | ||
| import { sha256 } from '../lib/hash.js'; | ||
| import { stableStringify } from '../lib/json_stable.js'; | ||
|
|
||
| const outDir = path.resolve('out/metrics'); | ||
| await mkdir(outDir, { recursive: true }); | ||
|
|
||
| const configPath = path.resolve('intelgraph/pipelines/narrative_ci/config/defaults.yml'); | ||
| const configContents = await readFile(configPath, 'utf-8'); | ||
| const payload = { | ||
| run_id: 'fixture-run', | ||
| config_hash: sha256(configContents), | ||
| scores: [], | ||
| }; | ||
|
|
||
| await writeFile( | ||
| path.join(outDir, 'seeding_density.json'), | ||
| `${stableStringify(payload)}\n`, | ||
| 'utf-8', | ||
| ); | ||
|
|
||
| console.log('Seeding density scores written'); |
There was a problem hiding this comment.
This script, along with 31_score_handoff.ts, 32_score_compression.ts, and 40_state_machine.ts, contains a significant amount of duplicated code for reading the configuration, hashing it, and writing a fixture file. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider refactoring this logic into a shared utility function.
For example, you could create a writeFixture function in a new lib/fixture_writer.ts file that encapsulates the common logic. The individual step files would then become much simpler, just calling this function with their specific data.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
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. |
1 participant
Motivation
config_hashand scoping fixture state transition files to a stable path.Description
schemas/narrative/narrative_state.schema.jsonand updated the pipeline to emit aconfig_hashand writestate_transitions.jsontoout/narratives/fixture/for deterministic fixture runs.intelgraph/pipelines/narrative_ci/steps/40_state_machine.tsto readconfig/defaults.yml, compute a SHA-256config_hashvialib/hash.ts, and emit stable JSON usinglib/json_stable.ts.steps/50_bundle_evidence.ts, README, andevidence/index.jsonto reference the scoped fixture output path and new evidence ID.config/defaults.yml,lib/{hash,ids,json_stable,schema_validate}.ts, fixture step implementations (30_*/31_*/32_*), schema artifacts (schemas/narrative/*), OPA policies and fixtures, and anarrative-ciGitHub Actions workflow to validate the full fixture pipeline in CI.docs/roadmap/STATUS.jsonwith a timestamped revision note describing the refined narrative CI lane-1 scaffold.Testing
narrative-ciworkflow is configured to run the fixture pipeline (npx tsx intelgraph/pipelines/narrative_ci/steps/50_bundle_evidence.ts --fixture), run schema validation (npx tsx intelgraph/pipelines/narrative_ci/lib/schema_validate.ts out schemas/narrative), execute OPA policy tests (opa test .github/policies/narrative_ci -v), and enforce a determinism grep gate for timestamp-like keys.Codex Task