Base Event, metadata description updates#1616
Conversation
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Schema Description ReviewAutomated suggestions for improving description clarity for LLM consumption. These are advisory — not required changes. Looking at the compiled schema changes, I can see significant improvements have been made since the previous review. Let me check which issues have been addressed and identify any remaining concerns. Previous Issue Status✅ Fixed: The anti-pattern about missing enum descriptions in Suggestions
CHANGELOG Issues
SummaryThe schema changes represent excellent improvements in description quality and LLM comprehension. The previous anti-pattern finding has been resolved with proper enum descriptions. The remaining suggestions are minor refinements to clarify that duration-based attributes can apply to both aggregate and single events with measurable timespan, plus a small formatting consistency issue in the CHANGELOG. |
Schema Anti-Pattern CheckAutomated structural analysis for common schema design anti-patterns. These are advisory — not required changes. Informational
Summary: 0 warning(s), 1 informational finding(s) |
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
|
@zschmerber-atlassian the suggestion bot seems to be goofing up. I suspect something's off in the end state that is used for analysis, because what it is calling "current" is not really current |
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
|
We should also improve the descriptions for the |
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
|
@mikeradka I just pushed a commit that addresses Paul's comments. Take a look. |
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
related PR - #1616 ## Summary The profile metaschema (`profile.schema.json`) is missing `references` and `@deprecated` properties that are supported by the event class and object metaschemas (via `common-event-object.schema.json`). Since the profile schema sets `additionalProperties: false`, any profile that uses these fields (e.g. `datetime.json` uses `references`) would fail strict metaschema validation. ## Changes - Added `references` property (`$ref` to `references.schema.json`) to the profile metaschema - Added `@deprecated` property (`$ref` to `deprecated.schema.json`) to the profile metaschema - Updated CHANGELOG ## Testing - Verified `datetime.json` (which uses `references`) passes validation against the updated metaschema - No existing profiles use `@deprecated` at the profile level yet, but the field is now available for future use --------- Signed-off-by: Rajas Panat <rajaspa@amazon.com>
The review bot was sending entire compiled objects/classes to Claude when any attribute in the file changed. This caused Claude to review and flag inherited/generated attributes (like type_uid enums) that the PR never touched. Now parses the PR diff to extract which specific attributes were modified in each file, and filters the compiled output to only include those attributes before sending to Claude. Fixes ocsf#1616 Co-authored-by: Cursor <cursoragent@cursor.com>
Problem The description review bot flags attributes the PR didn't touch. In #1616 (base_event/metadata description updates), the bot flagged incident_finding.type_uid enum values — a compiler-generated attribute unrelated to the PR's changes. Root Cause When any attribute in a file is modified, review_descriptions.py sends the entire compiled object (all inherited + generated attributes) to Claude. Claude can't reliably distinguish what the PR changed from what was already there, so it reviews and suggests changes to untouched content. Fix Added diff-aware attribute scoping to the prepare phase: extract_changed_attrs_in_file() — parses the PR diff to identify which specific attributes were modified in each changed file _filter_to_changed_attrs() — filters the compiled output to only include those attributes Claude now only sees the attributes the PR actually changed. Co-authored-by: zschmerber <zschmerber@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Seems like there is one more place where |
|
All the tests pass now. The ocsf-validator pypi package needed to be published for the latest changes (ocsf/ocsf-validator#38) to be picked up by the github action. That's complete now - https://pypi.org/project/ocsf-validator/ (thanks to @query-jeremy) |
davemcatcisco
left a comment
There was a problem hiding this comment.
As well as the specific comments below, I've a general comment about this PR.
Our convention until now is that:
- "foo_id" enum attributes (which are often recommended or required) have a detailed description
- "foo" sibling attributes (always optional) have cookie-cutter descriptions that reference the "foo_id" attribute
A lot of the updates to enum attributes and their sibling in this PR seem to flip this around. I think it's important that the primary attribute gets the most fullsome description.
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
…tion Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Yeah, this is should be addressed in the latest commit. It was primarily the case for status attributes in finding classes. |
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Signed-off-by: Rajas Panat <rajaspa@amazon.com>
Aniak5
left a comment
There was a problem hiding this comment.
Great improvements here! Thanks for taking this on!
Related Issue: General lack of depth in our base event descriptions | Also addresses #1611 & #1614
Description of changes:
dictionary.json
Updated 17 attributes to be more descriptive while remaining generic
with "See specific usage" where appropriate:
activity_name,category_name,class_name,type_name,severity) clarify the relationshipto their
_idcounterpart and when the sibling string must bepopulated (i.e.
_id= 99)activity_id,category_uid,class_uid,type_uidaddproducer guidance (compute formula, examples)
metadata,observablesexplain purpose and key sub-attributesraw_data,raw_data_hash,raw_data_size,unmappedexplainpurpose and when to populate
timezone_offsetadds local-time reconstruction contexttimegeneralized to "See specific usage" — event-specific andfinding-specific semantics live in their respective classes
count,messagegeneralized to "See specific usage"status,status_code,status_detail,status_details,status_id) generalized to "See specific usage"status_idenum trimmed to only0(Unknown) and99(Other);Success/Failure moved to base_event where it belongs
events/base_event.json
Attributes that now inherit from dictionary have overrides removed.
Remaining overrides provide event-specific guidance:
timedocuments the aggregate convention (time=start_timewhen
count> 1) and clarifies this is not when the event recordwas created/serialized (Clarify Event Record Provenance Across Lifecycle Stages #1614). Points to
metadata.logged_timeand
metadata.processed_timefor lifecycle timestampsstart_time,end_time,durationscoped to aggregate events(
count> 1), with note that subclasses may redefine for theirown time-range semantics
countscoped to aggregate events onlystatus_idcarries the Success/Failure enum with descriptionsstatus,status_code,status_detailprovide event-outcomeexamples and explain the layered relationship
messageadds UI/alert-friendly guidance with exampleevents/findings/finding.json
activity_idadds Create/Update/Close guidanceactivity_nameadds Other (99) must-populate rulestatus/status_iddistinguishes finding lifecycle from eventoutcome, with workflow state descriptions
start_time/end_timeclarified for finding contexttimeoverridden: finding creation time, not activity occurrenceevents/findings/incident_finding.json
timeoverridden: incident creation time, not activity occurrenceprofiles/host.json
actoranddeviceget role-specific descriptionsprofiles/datetime.json
_dtsiblingattributes of type
datetime_twherevertimestamp_tappearsobjects/http_response.json, objects/response.json
http_response.messagemessageattribute gets context-specific descriptions to resolvecompiler "See specific usage" warnings