Define SIP Observability Profile v1.0.0#42
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request introduces the SIP Observability Profile, establishing standardized event and scorecard schemas for the Starlight Intelligence System. The review identified several areas to strengthen the data contracts: the 'risks' array in the scorecard schema is currently too permissive, several objects in the event schema allow 'additionalProperties' which may lead to inconsistency, and the 'scorecard_id' field lacks a minimum length constraint to prevent empty identifiers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "source_event_ids": {"type":"array","items":{"type":"string"},"uniqueItems":true}, | ||
| "metrics": {"type":"array","items":{"type":"object","required":["name","value","unit","definition"],"properties":{"name":{"type":"string"},"value":{"type":["number","integer","string","boolean","null"]},"unit":{"type":"string"},"definition":{"type":"string"},"numerator":{"type":["number","integer"]},"denominator":{"type":["number","integer"]}},"additionalProperties":false}}, | ||
| "signals": {"type":"object","additionalProperties":{"type":["string","number","integer","boolean","null"]}}, | ||
| "risks": {"type":"array","items":{"type":"object"}}, |
There was a problem hiding this comment.
The items in the risks array are defined as a generic {"type":"object"}, which allows any object structure. This is too permissive and undermines the purpose of the schema by leaving a key part of the data contract undefined. Please define the structure of a risk object with specific properties (e.g., id, description, severity, status) and set additionalProperties: false.
| "event_id": {"type":"string","minLength":1}, | ||
| "event_type": {"enum":["objective.created","decision.recorded","run.started","run.completed","run.failed","artifact.produced","artifact.verified","human.intervention","cost.observed","metric.observed","repo.snapshot","incident.opened","incident.resolved"]}, | ||
| "observed_at": {"type":"string","format":"date-time"}, | ||
| "source": {"type":"object","required":["system"],"properties":{"system":{"type":"string","minLength":1},"connector":{"type":"string"},"machine":{"type":"string"},"conversation_id":{"type":"string"},"ref":{"type":"string"}},"additionalProperties":true}, |
There was a problem hiding this comment.
The source object, as well as links (line 13), execution (line 14), quality (line 15), economics (line 16), and items in the evidence array (line 18), are defined with "additionalProperties": true. This allows any extra properties to be added, which can lead to inconsistent data and makes it harder for consumers to reliably parse these events.
For a stricter data contract, it's recommended to set "additionalProperties": false and explicitly define all allowed properties in the schema. If extensibility is truly needed, consider adding a dedicated custom property to these objects to hold arbitrary key-value pairs.
| "required": ["schema_version","scorecard_id","generated_at","as_of","source_event_count","source_event_ids","metrics","signals","risks","anti_goodhart","attestation"], | ||
| "properties": { | ||
| "schema_version": {"const":"1.0.0"}, | ||
| "scorecard_id": {"type":"string"}, |
CTO/CDO council: refine before readyThe profile is a strong vendor-neutral substrate, and the required join spine is the right direction. I independently verified the current head against
Industry note: freeze stable identity/join keys while allowing additive attribute evolution; avoid forcing every emitter into lockstep for harmless schema additions. No code changes made to this owned Codex lane. Current checks are green and GitHub reports the draft merge state as clean. |
What changed
Defines the substrate-neutral SIP Observability Profile:
Why
The estate already measures models, memory, retrieval, safety, content, cost, and fleet state, but those observations do not share a neutral evidence contract. This profile standardizes semantics without mandating Supabase, Langfuse, Hermes, or any other vendor/runtime.
Governance
This is substrate-class. The included Board verdict authorizes a review branch and operational dog-food run, not automatic merge.
Required before merge:
Verification