chore(schema): add field descriptions for firefox_ios_derived#9692
Conversation
…e_clients_yearly_v1
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds field descriptions to three firefox_ios_derived table schema.yaml files (baseline_clients_yearly_v1, feature_usage_events_v1, ltv_ios_aggregates_v1) and introduces a new dataset-level firefox_ios_derived.yaml base schema, generated by a metadata agent pipeline.
The bulk of the descriptions are clear and consistent, and YAML structure/field names look valid with no duplicates. My comments focus on a couple of descriptions whose accuracy I could verify against the queries and the repo's canonical wording, plus a note on how the new dataset-level file relates to the inlined descriptions. Since the PR's own checklist notes descriptions still need an accuracy review, the maintainers should give the _users/event-count and autofill descriptions a domain pass as well — I only flagged the ones I could confirm.
| type: BOOLEAN | ||
| mode: NULLABLE | ||
| description: Indicates whether this is the client's first year of activity. True if the client's first_seen_date | ||
| falls within the current year window. |
There was a problem hiding this comment.
issue: This description of is_new_profile is inaccurate. is_new_profile is a passthrough from firefox_ios.baseline_clients_daily and marks whether the profile was newly created (the canonical description used elsewhere in this repo is "Indicates whether the Firefox profile associated with the client was newly created. True means the profile was created recently"). It has nothing to do with a "first year of activity" or a "current year window" — that interpretation appears to be inferred from the table name. Replace it with the profile-creation meaning. The same incorrect text is duplicated in firefox_ios_derived.yaml (line 98) and should be fixed there too.
There was a problem hiding this comment.
updated these to correct definition
| - name: retained_week_2 | ||
| type: BOOLEAN | ||
| description: Whether the client was retained at week 2 (i.e., sent a ping in the second week after first seen). Null in the current sample, | ||
| likely populated for older cohorts. |
There was a problem hiding this comment.
issue: Schema descriptions should document a field's durable semantics, not a transient observation of the current data or a guess. "Null in the current sample, likely populated for older cohorts" states an assumption ("likely") about data that isn't part of the field definition and will become stale/misleading. paid_vs_organic_gclid (line 168) has the same problem with "Null for all clients in this sample." Describe what the field represents and, if the null behavior is a real invariant, state the actual reason rather than an observation about the sample.
There was a problem hiding this comment.
'null' language removed from descriptions
| @@ -0,0 +1,206 @@ | |||
| fields: | |||
There was a problem hiding this comment.
suggestion: This new dataset-level base schema is added, but the per-table schema.yaml files in this PR inline their descriptions directly rather than referencing it. The established pattern for these <dataset>_derived.yaml files (e.g. messaging_system_derived.yaml) is for the table schema.yaml files to pull canonical text via !include-field-description with a file: path (see docs/reference/schema_includes.md). As written, the same descriptions now live in two places and can drift. Consider either referencing the shared file from the table schemas via !include-field-description, or dropping the shared file if inline descriptions are the intended source of truth.
There was a problem hiding this comment.
implemented changes so that repeated field descriptions are replaced with the base schema definitions
also, the upstream fix to the agent is applied in the PR: https://github.com/mozilla/data-shared-llm-agents/pull/157/changes
…escriptions - Replace duplicated inline field descriptions with !include-field-description references to firefox_ios_derived.yaml (baseline_clients_yearly_v1, feature_usage_events_v1). - Correct is_new_profile to describe profile creation, not yearly activity. - Remove transient sample observations from retained_week_2 and paid_vs_organic_gclid descriptions. Addresses review feedback on PR #9692.
This comment has been minimized.
This comment has been minimized.
Integration report
|
3 participants
Schema Update:
moz-fx-data-shared-prod.firefox_ios_derivedSummary
Fields Updated by Table
Contradictions Found
None
Checklist