docs: update for 1.1 - #421
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR reorganizes Osprey’s documentation into user, development, integration, concepts, and rules guides; adds local setup and CLI references; refreshes examples and UI documentation; updates navigation and redirects; and introduces a custom Roost documentation theme. ChangesDocumentation entry points and navigation
Concepts and rule-writing guides
Development and local setup guides
Integration and data-flow guides
User and investigation guides
Documentation theme implementation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 user research link pointed to docs/user_personas.md, which doesn't exist; the actual file is docs/research-personas.md.
docs/DEVELOPMENT.md doesn't exist; the development setup guide actually lives at docs/development/README.md.
The Features and Rules pages in the UI are titled "Features Registry" and "Rules Registry" (added alongside the Registry pages), but the docs still called them "Features" and "Rules". Updated the headers and the anchor links pointing to them from the User Guide overview.
None of Osprey's three console-script CLIs were documented anywhere. Flags and subcommands here are verified against the actual click/argparse definitions, and cross-checked against real --help output where the sandboxed environment allowed running them.
Bulk labeling from the command line (apply_label/bulk_apply_label) was otherwise undiscoverable from the UI-focused Operate docs.
…#11) Answers a recurring adopter question: SML's restrictions (no loops, assign-once names, static typing) aren't arbitrary — they're what let the engine run untrusted rule logic safely and evaluate it in parallel. Also honest about the current gap: there's no dedicated SML tooling yet.
) New adopters had no single page explaining how events actually reach Osprey (Kafka/PubSub/coordinator gRPC), how results come back out (output sinks + execution result stores), or how to run against a custom platform via the register_input_stream/ register_action_proto_deserializer plugin hooks instead of forking.
register_validation_exporter and register_label_output_sink exist in osprey_hooks.py but weren't listed alongside the other hookspecs.
… ML models, and known gaps (closes #383) Consolidates the plugin extension points adopters ask about most: UDFs, input/output sinks, the labels service, and three previously undocumented ones (hash-based lookups, ML model integration, review tool integration). The latter three don't map to dedicated features — documented honestly as compositions of existing primitives (or, for review tools, as an acknowledged gap) rather than inventing hooks that don't exist.
Replaced with colons, semicolons, commas, or restructured sentences depending on context, and varied a repeated "— this is..." construction that had crept into several pages.
cassidyjames
force-pushed
the
cassidyjames/docs-1.1
branch
from
July 14, 2026 16:15
1e9929c to
106e80d
Compare
example_docker_compose/ lives outside the mdBook src root, so relative links to it 404 on the deployed site. Link to the GitHub directory instead, which renders the README.
mdBook slugs the heading as #osprey-cli-apply_label, so the bare #apply_label anchor silently landed at the top of the page.
"Tools Overview" vs "Development Tools" and "User Research Overview" vs "User Research & Personas" made the sidebar and pages disagree about what a page is called.
The docs said "autoupgrade", which isn't a pre-commit subcommand.
The per-tool "Key rules enabled:"/"Key features:" bullet lists read like generated filler; one sentence each carries the same facts. Claims verified against pyproject.toml.
This was the only British-spelled page in the book, and the icon column was decorative rather than informative.
"Yeet" is the literal button label in the UI (Chart.tsx, TopNFooter.tsx), so quote it as such instead of leaving it looking like slang, and document the Top N table's Yeet Table button, which was missing from the list.
The page was mostly a generic git tutorial with invented examples, and its pre-commit hook list didn't match .pre-commit-config.yaml (no trailing-whitespace, end-of-file, or YAML hooks exist). Keep branch naming, what the hooks actually run, the manual check commands, and two real commit messages from history.
The ready banner listed LazyPostRule, QuickPostRule, and FoxPostRule, none of which are in example_rules/, and suggested a query that matches nothing. ContainsHello is the only demo rule; describe it accurately (ban + meow label) and fix the query hint.
One-command demo path for the 1.1 analyst audience: prerequisites, what demo.sh actually does (including that it wipes prior demo volumes), what to try against the real ContainsHello ruleset, and how to stop it. Front of the book, before the guides.
The README never mentioned demo.sh or the published documentation — the two best entry points for someone evaluating Osprey. Also swap marketing adjectives for plain description.
PR #409 and this branch both added an integrations page closing #383. Keep #409's fuller walkthrough as the base at development/integrations.md, fold in the branch page's unique content (hash-lookup and review-tool extension points, UdfCategories tip, data-flow cross-links, extra hooks), and redirect the old top-level URL. Also set create-missing = false so bad SUMMARY paths fail the build instead of silently creating stub pages.
The Development Guide now opens with the Getting Started quickstart (development/README.md), and the detailed setup reference moves to development/local.md; the two openers point at each other. Rename the setup page's "Getting Started" step to "Start the Services" to avoid colliding with the new page, and point rules.md's editor-setup claim at IDE Setup, which is where editor docs actually live.
Open with what Osprey is and a screenshot instead of contributing boilerplate; route readers by role (analyst, rule author, developer); note how docs versioning works. Follows the style from Coop.
Concepts moved from user/ to the docs root, but links in and to it still assumed the old location; also fix a typo'd #effects anchor.
rules.md was over a quarter of all doc text: tutorial, reference, rationale, and a worked example in one page. Move it to rules/ with Examples and Why a Bespoke Language? as subpages, slim the UDF section to the rule-author view (naming the new 1.1 stdlib UDFs, with the in-app UDF Registry as the authoritative list), and link Python implementation topics to Integrations & Plugins instead of duplicating them. Redirect the old rules.html URL. The Examples page walks the demo ruleset file by file and adds labels-as-state and multi-signal patterns. Fixes #382
Screenshots with transparency were hard to read against the dark theme. Reuse the frame color as a background-color, and add the required colorspace to color-mix() so both declarations actually apply; browsers drop color-mix() without it.
Data Flow and Integrations & Plugins are for people connecting Osprey to their platform, not people changing Osprey itself—the same audience split Coop's docs draw. Give them their own guide with a short landing page, and retarget the /integrations.html redirect.
The intro had three overlapping "Osprey is..." paragraphs plus a list of vague bullets accumulated over time; merge them into one description of the actual flow from events in to verdicts out. Also give the screenshot a real alt text, Americanize "labelling", and drop boilerplate that doesn't apply (mailing lists) or that the dev guide already covers.
Verified the user guide against osprey_ui/src and the screenshots:
labels have connotations (LabelConnotation), not "polarities"; the
Create New Job modal creates jobs from an uploaded file of entities
rather than collecting label fields; the Saved Queries menu options and
the Rules Visualizer's action/label search now match the UI; and PoP
and Precision wording follows the actual buttons. Documents the
100,000-entity default limit on bulk jobs (and its opt-out) from
LabelForm.
Also replaces every placeholder image alt text ("Osprey Home", "Query
Box") with a full description of what each screenshot shows, and trims
Query Syntax's opener that repeated the next paragraph.
The Name/Logic/Description blocks and the effects, labels, nulls, and
Import/Require sections kept a stiff, formal register inherited from
the original rules.md ("all encompassed as single comma-delimited
list", "operators and designers may utilize"). Rewrite them in the
page's tutorial voice without changing technical meaning, link the Null
gotcha from where it first bites, and fix the "verify and email" typo.
Normalize dash spacing, drop "Note that"/"Keep in mind" hedges, align the sidebar Workflow entry with its H1, and fix comma splices and a typo in Local Development and the personas page. Corrects the test-data producer description (it sends create_post events, not user logins, per example_data/template.json) and gives IDE Setup the SML file-association note that Why a Bespoke Language? already points readers to.
cassidyjames
marked this pull request as ready for review
July 15, 2026 12:54
cassidyjames
requested review from
a team,
EXBreder,
ayubun,
haileyok and
vinaysrao1
as code owners
July 15, 2026 12:54
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
docs/theme/css/content.css (1)
30-32: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider alternatives for
text-wrap: balanceon table cells.Applying
text-wrap: balanceto table data cells can be computationally expensive. Browsers typically limit balancing to elements with 4–6 lines, meaning it will likely be ignored on longer cell content and waste calculation time on shorter ones. It is generally recommended to reservebalancefor headings. Consider usingtext-wrap: prettyor the default wrapping behavior fortdelements.♻️ Proposed alternative
table td { - text-wrap: balance; + text-wrap: pretty; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/theme/css/content.css` around lines 30 - 32, Update the table cell styling around the td selector to remove text-wrap: balance and use text-wrap: pretty or the default wrapping behavior instead; keep balance reserved for heading elements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/concepts.md`:
- Line 28: Update the rule description near the definition of “rule” to state
that rule evaluation can return True, False, or Null, while preserving the
existing explanation that the result is queryable and rules can be wired to
effects.
In `@docs/development/local.md`:
- Around line 115-128: Update the register_ast_validators example signature to
return Sequence[Type[BaseValidator]] instead of None, matching the documented
hook contract and validator iteration behavior. Leave the surrounding hook
documentation unchanged.
- Around line 87-91: Update the networking guidance around the docker-compose
port examples to remove or clearly discourage the unauthenticated public bind
`5002:5002`. Retain the Tailscale/SSH access approach, and require authenticated
TLS or reverse-proxy protection before documenting any public binding; ensure
the guidance addresses both ports 5002 and 5004.
In `@docs/development/README.md`:
- Around line 21-24: Update the demo.sh invocation in the development README to
avoid piping the mutable main branch directly into bash. Pin the download to an
immutable release or commit, or change the instructions to download the script
for inspection before execution while preserving the documented repository setup
flow.
In `@docs/development/troubleshooting.md`:
- Line 34: Update the Kafka offset explanation in the troubleshooting
documentation to state that latest applies only when the supervisor starts
without committed offsets, while existing checkpoints determine resume position
and resetting the supervisor can change the cutoff. Remove the specific “after
docker compose up first ran” wording.
In `@docs/integration/integrations.md`:
- Line 124: Update the guidance around InputStreamSource.KAFKA and
InputStreamSource.PUBSUB to state that Kafka events use KAFKA, while Google
Pub/Sub events use PUBSUB. Keep the custom InputStreamSource.PLUGIN and protobuf
deserializer guidance unchanged.
In `@docs/rules/examples.md`:
- Around line 124-130: Update the DidAddLabel example in the rule documentation
to use entity_type="User", matching the declared type of UserId and the label
configuration. Leave the label_name and surrounding query guidance unchanged.
- Around line 31-60: Update the documentation examples so only the stable UserId
identifier remains declared with EntityJson. Change EventType in models/base.sml
and PostText in models/post.sml to use JsonData while preserving their existing
JSON paths and type behavior, and revise the surrounding explanation to describe
them as event attributes rather than entities.
In `@docs/rules/README.md`:
- Around line 132-145: Standardize optional-feature examples on SML’s Null
sentinel: in docs/rules/README.md lines 132-145, describe missing optional
values as Null instead of Python None; in docs/rules/examples.md lines 153-155,
update the EmbedLink comparison from None to Null.
In `@docs/user/investigate/query-syntax.md`:
- Line 44: Update the sentence describing DidAddLabel() in the query syntax
documentation to state that it matches events attempting to add a label, without
implying the label was successfully added or remains on the entity; preserve the
existing distinction from HasLabel().
---
Nitpick comments:
In `@docs/theme/css/content.css`:
- Around line 30-32: Update the table cell styling around the td selector to
remove text-wrap: balance and use text-wrap: pretty or the default wrapping
behavior instead; keep balance reserved for heading elements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 318f2fba-d654-4ed7-b485-0a90682244a2
📒 Files selected for processing (35)
AGENTS.mdCHANGELOG.mdREADME.mddemo.shdocs/README.mddocs/SUMMARY.mddocs/book.tomldocs/concepts.mddocs/development/README.mddocs/development/cli-reference.mddocs/development/ide.mddocs/development/local.mddocs/development/tools.mddocs/development/troubleshooting.mddocs/development/workflow.mddocs/integration/README.mddocs/integration/data-flow.mddocs/integration/integrations.mddocs/research-personas.mddocs/rules.mddocs/rules/README.mddocs/rules/examples.mddocs/rules/why-sml.mddocs/theme/css/content.cssdocs/theme/css/roost-hljs.cssdocs/theme/css/roost.cssdocs/theme/fonts/fonts.cssdocs/theme/index.hbsdocs/user/README.mddocs/user/appearance.mddocs/user/investigate/README.mddocs/user/investigate/labels.mddocs/user/investigate/query-syntax.mddocs/user/manage.mddocs/user/operate.md
💤 Files with no reviewable changes (2)
- docs/rules.md
- docs/user/appearance.md
Comment on lines
+87
to
+91
| By default, the `docker-compose.yaml` binds running services to `127.0.0.1`. If you are running the docker compose on a headless machine, you may need to modify this configuration and/or make changes to your firewall, specifically for ports `5002` and `5004`. | ||
|
|
||
| For example, if you use Tailscale to access your Osprey instance, you may change `127.0.0.1:5002:5002` to `<Tailscale IP>:5002:5002`. Alternatively, if you wish for your instance to be accessible from the public internet, you may set it simply to `5002:5002` to bind to `0.0.0.0`. | ||
|
|
||
| Be aware that some firewalls like iptables/UFW do _not_ prevent access to ports being used by Docker networking. Not explicitly setting a bind address with only UFW as a firewall will not prevent access from the public internet unless [properly configured](https://github.com/chaifeng/ufw-docker). |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## docs/development/local.md (around lines 80-100)\n'
nl -ba docs/development/local.md | sed -n '80,100p'
printf '\n## Search for 5002/5004 port mappings in docker-compose.yaml\n'
rg -n '5002|5004|127\.0\.0\.1|0\.0\.0\.0' docker-compose.yamlRepository: roostorg/osprey
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba docs/development/local.md | sed -n '80,100p'
printf '\n---\n'
rg -n '5002|5004|127\.0\.0\.1|0\.0\.0\.0' docker-compose.yamlRepository: roostorg/osprey
Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## docs/development/local.md (around lines 80-100)\n'
sed -n '80,100p' docs/development/local.md | cat -n
printf '\n## Search for 5002/5004 port mappings in docker-compose.yaml\n'
grep -nE '5002|5004|127\.0\.0\.1|0\.0\.0\.0' docker-compose.yaml || trueRepository: roostorg/osprey
Length of output: 2787
Avoid recommending public exposure for the dev UI/API docs/development/local.md:87-91
The 5002:5002 example binds the UI to 0.0.0.0 and can expose both 5002 and 5004 to the internet. Keep the Tailscale/SSH example, or add a clear requirement for authenticated TLS/reverse-proxy protection before any public bind.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/development/local.md` around lines 87 - 91, Update the networking
guidance around the docker-compose port examples to remove or clearly discourage
the unauthenticated public bind `5002:5002`. Retain the Tailscale/SSH access
approach, and require authenticated TLS or reverse-proxy protection before
documenting any public binding; ensure the guidance addresses both ports 5002
and 5004.
Comment on lines
+21
to
+24
| Or without cloning anything first—this clones the repo into `./osprey-demo` for you: | ||
|
|
||
| ### 6. (Optional) Open ports for the UI/UI API | ||
| ```sh | ||
| curl -sSL https://raw.githubusercontent.com/roostorg/osprey/main/demo.sh | bash |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid piping a mutable branch directly into bash.
This executes whatever changes are currently on main without review or integrity verification. Pin the script to an immutable release/commit, or instruct users to download and inspect it before execution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/development/README.md` around lines 21 - 24, Update the demo.sh
invocation in the development README to avoid piping the mutable main branch
directly into bash. Pin the download to an immutable release or commit, or
change the instructions to download the script for inspection before execution
while preserving the documented repository setup flow.
| - Narrow the time range to 1–4 hours centered on when you generated test data | ||
| - Click the edit icon next to the displayed time range to switch to a custom date/time picker | ||
| - Note that Druid's Kafka consumer uses `auto.offset.reset: latest`; it only picks up events produced after `docker compose up` first ran, so events from before that point will not appear regardless of the time range | ||
| - Druid's Kafka consumer uses `auto.offset.reset: latest`; it only picks up events produced after `docker compose up` first ran, so events from before that point will not appear regardless of the time range |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the troubleshooting doc and any relevant Kafka offset references.
git ls-files | rg '(^|/)docs/development/troubleshooting\.md$|kafka|druid|offset.reset|supervisor|committed offset|docker compose up' || true
echo
echo '--- docs/development/troubleshooting.md ---'
cat -n docs/development/troubleshooting.md | sed -n '1,120p'
echo
echo '--- search for auto.offset.reset references ---'
rg -n "auto\.offset\.reset|committed offset|supervisor reset|docker compose up" . || trueRepository: roostorg/osprey
Length of output: 6648
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg '(^|/)docs/development/troubleshooting\.md$|kafka|druid|offset.reset|supervisor|committed offset|docker compose up' || true
echo
echo '--- docs/development/troubleshooting.md ---'
cat -n docs/development/troubleshooting.md | sed -n '1,120p'
echo
echo '--- search for auto.offset.reset references ---'
rg -n "auto\.offset\.reset|committed offset|supervisor reset|docker compose up" . || trueRepository: roostorg/osprey
Length of output: 6648
🌐 Web query:
Druid Kafka indexing service auto.offset.reset latest committed offsets supervisor checkpoints
💡 Result:
In Apache Druid, the Kafka indexing service manages offsets through a supervisor that coordinates indexing tasks. The behavior regarding offset resets and checkpoints is as follows: 1. ResetOffsetAutomatically: The resetOffsetAutomatically configuration (defaulting to false) controls how Druid handles OffsetOutOfRangeException, which occurs when the required Kafka offsets are no longer available (e.g., due to data retention policies) [1][2]. When set to true, Druid automatically attempts to reset the partition offset to a valid position [1][2]. Note that this can lead to data loss or duplication without explicit notification [1]. When set to false, the ingestion tasks fail, and the issue must be resolved via manual intervention, such as using the Reset Supervisor API [1][2]. 2. Relationship to Kafka's auto.offset.reset: While the behavior is conceptually similar to Kafka's own auto.offset.reset property, Druid's implementation is specific to the supervisor [1]. Specifically, when auto-reset is triggered in Druid, it does not strictly mirror the standard Kafka client property [3]. Historically, there have been nuances in how this reset is performed—specifically aiming to use the least available offset to avoid infinite loops or unnecessary data gaps [4][3]. 3. Supervisor Checkpoints and Offsets: Under normal operation, Druid indexing tasks commit checkpoints (offsets) to the metadata store [5]. Subsequent tasks start from these committed offsets, ensuring exactly-once processing [5]. The supervisor maintains these offsets and uses them to coordinate tasks [5]. If a supervisor is running for the first time, it uses the useEarliestOffset setting to determine whether to start from the earliest or latest available offset in Kafka [1][5]. 4. Manual Intervention: If ingestion halts due to unavailable offsets (and resetOffsetAutomatically is false), the Reset Supervisor API is the standard mechanism to clear stored offsets [2][6]. This action forces the supervisor to reconcile its state and resume from a valid, current position in Kafka [2][6]. Top results: [7][1][5][2]
Citations:
- 1: https://druid.apache.org/docs/27.0.0/development/extensions-core/kafka-supervisor-reference
- 2: https://github.com/apache/druid/blob/master/docs/ingestion/supervisor.md
- 3: Fixes a bug in Kafka auto reset apache/druid#12008
- 4: Infinite automatic Kafka offset resetting apache/druid#11658
- 5: https://druid.apache.org/docs/29.0.0/ingestion/kafka-ingestion/
- 6: https://druid.apache.org/docs/latest/api-reference/supervisor-api/
- 7: https://druid.apache.org/docs/29.0.1/ingestion/supervisor
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the doc exists, print only the relevant area.
if [ -f docs/development/troubleshooting.md ]; then
sed -n '24,44p' docs/development/troubleshooting.md | cat -n
fi
# Search for any mention of Kafka supervisor offset handling.
rg -n "committed offset|checkpoint|supervisor|latest|earliest|auto\.offset\.reset" docs . || trueRepository: roostorg/osprey
Length of output: 12025
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- druid/specs/execution_results.json ---'
cat -n druid/specs/execution_results.json | sed -n '1,220p'
echo
echo '--- druid/specs/submit-specs.sh ---'
cat -n druid/specs/submit-specs.sh | sed -n '1,220p'
echo
echo '--- search for useEarliestOffset / reset supervisor in Druid files ---'
rg -n "useEarliestOffset|auto\.offset\.reset|reset supervisor|terminate|checkpoint|offset" druid osprey_worker/src/osprey/worker/ui_api/osprey/lib/druid.py osprey_worker/src/osprey/worker/ui_api/druid_osprey_supervisor_spec.json || trueRepository: roostorg/osprey
Length of output: 2535
Clarify the offset boundary. auto.offset.reset: latest only applies when the supervisor starts without committed offsets; once Druid has checkpoints, it resumes from them, and a supervisor reset can change that cutoff. Saying “after docker compose up first ran” is too specific.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/development/troubleshooting.md` at line 34, Update the Kafka offset
explanation in the troubleshooting documentation to state that latest applies
only when the supervisor starts without committed offsets, while existing
checkpoints determine resume position and resetting the supervisor can change
the cutoff. Remove the specific “after docker compose up first ran” wording.
Comment on lines
+31
to
+60
| **`models/base.sml`** defines the features present on every event. `UserId` and `EventType` are declared as entities, so Osprey tracks them as _things_ that persist across events—that's what lets a label stick to a user later. `coerce_type=True` (the default) converts a mismatched value, like a numeric user ID, to the declared type instead of erroring. | ||
|
|
||
| ```python | ||
| UserId: Entity[str] = EntityJson( | ||
| type='User', | ||
| path='$.user_id', | ||
| coerce_type=True | ||
| ) | ||
|
|
||
| EventType: Entity[str] = EntityJson( | ||
| type='EventType', | ||
| path='$.event_type', | ||
| coerce_type=True | ||
| ) | ||
|
|
||
| ActionName=GetActionName() | ||
|
|
||
| ActionId=GetActionId() | ||
| ``` | ||
|
|
||
| The last two lines pull the event's name and ID from Osprey itself rather than the JSON payload: `GetActionName()` and `GetActionId()` are stdlib UDFs, and exposing them as features makes them queryable in the UI. | ||
|
|
||
| **`models/post.sml`** adds the one feature specific to posts: | ||
|
|
||
| ```python | ||
| PostText: Entity[str] = EntityJson( | ||
| type='PostText', | ||
| path='$.post.text', | ||
| coerce_type=True | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Model only stable identifiers as entities.
docs/concepts.md Line 22 defines entities as values that uniquely identify platform objects. EventType and PostText are repeated event attributes, not identities; declaring them with EntityJson gives them incorrect entity semantics and may create misleading label/entity views. Use JsonData for these fields and retain EntityJson for UserId.
Proposed correction
-EventType: Entity[str] = EntityJson(
- type='EventType',
+EventType: str = JsonData(
path='$.event_type',
- coerce_type=True
)
-PostText: Entity[str] = EntityJson(
- type='PostText',
+PostText: str = JsonData(
path='$.post.text',
- coerce_type=True
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/rules/examples.md` around lines 31 - 60, Update the documentation
examples so only the stable UserId identifier remains declared with EntityJson.
Change EventType in models/base.sml and PostText in models/post.sml to use
JsonData while preserving their existing JSON paths and type behavior, and
revise the surrounding explanation to describe them as event attributes rather
than entities.
Comment on lines
+132
to
+145
| EmbedLink: Optional[str] = JsonData( | ||
| path='$.embedLink', | ||
| required=False, | ||
| ) | ||
|
|
||
| ReplyId: Entity[str] = EntityJson( | ||
| type='PostId', | ||
| path='$.replyId', | ||
| required=False, | ||
| ) | ||
| ``` | ||
|
|
||
| The [`JsonData` UDF](#user-defined-functions-udfs) lets us take the event's JSON and define features based on the contents of that JSON. These features can then be referenced in other rules that we import the `models/record/post.sml` model into. If you have any values inside your JSON object that may not always be present, you can set `required` to `False`, and these features will be `None` whenever the feature is not present. | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Standardize on SML’s Null sentinel.
The rule guide and runnable example use Python’s None, while the documented SML semantics use Null. Align both locations so users do not copy invalid or inconsistent optional-feature checks.
docs/rules/README.md#L132-L145: describe absent optional features asNull.docs/rules/examples.md#L153-L155: changeEmbedLink != NonetoEmbedLink != Null.
📍 Affects 2 files
docs/rules/README.md#L132-L145(this comment)docs/rules/examples.md#L153-L155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/rules/README.md` around lines 132 - 145, Standardize optional-feature
examples on SML’s Null sentinel: in docs/rules/README.md lines 132-145, describe
missing optional values as Null instead of Python None; in
docs/rules/examples.md lines 153-155, update the EmbedLink comparison from None
to Null.
Member
There was a problem hiding this comment.
+1 @cassidyjames I think it's supposed to be None but worth confirming
| ## Label queries | ||
|
|
||
| The query UI searches across actions (events), not entity state, so `HasLabel()` won't work here. Use `DidAddLabel()` instead, which matches events where a label was added: | ||
| The query UI searches across events, not entity state, so `HasLabel()` won't work here. Use `DidAddLabel()` instead, which matches events where a label was added: |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify DidAddLabel() semantics.
The implementation in osprey_worker/src/osprey/engine/query_language/udfs/did_mutate_label.py:21-43 matches events that attempted to add a label; it does not guarantee that the label was added or remains on the entity. Update this sentence to avoid implying successful state mutation.
Proposed wording
-The query UI searches across events, not entity state, so `HasLabel()` won't work here. Use `DidAddLabel()` instead, which matches events where a label was added:
+The query UI searches across events, not entity state, so `HasLabel()` won't work here. Use `DidAddLabel()` instead, which matches events that attempted to add a label; it does not guarantee that the label was added:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The query UI searches across events, not entity state, so `HasLabel()` won't work here. Use `DidAddLabel()` instead, which matches events where a label was added: | |
| The query UI searches across events, not entity state, so `HasLabel()` won't work here. Use `DidAddLabel()` instead, which matches events that attempted to add a label; it does not guarantee that the label was added: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user/investigate/query-syntax.md` at line 44, Update the sentence
describing DidAddLabel() in the query syntax documentation to state that it
matches events attempting to add a label, without implying the label was
successfully added or remains on the entity; preserve the existing distinction
from HasLabel().
Cross-checked the docs against the engine and CLI and fixed several things that were wrong or would not actually work: - cli-reference: osprey-cli subcommands use dashes, not underscores (Click derives the command name from the function and converts _ to -), so `push_rules` etc. failed with "No such command". Also refreshed the osprey-stress `measure` wording now that #236 has merged. - query-syntax: SML uses `None`, not `Null`; the query box supports only four built-in functions (RegexMatch, DidAddLabel, DidRemoveLabel, DidDeclareVerdict), so the TextContains/ListLength query examples (both rules UDFs) would 500. Reworked the "Using UDFs" section and the note. - manage: clarified that the UDF Registry is mainly a rules reference; only those four functions work in the query box. - rules/README and rules/examples: Rule(...) requires a `description` argument (validation rejects it otherwise) and the null literal is `None`, not `Null`. Verified both against the real validator (validate_and_push). - local: use `uv run python` instead of a hardcoded python3.11 binary. Co-Authored-By: cassidyjames <611168+cassidyjames@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
julietshen
reviewed
Jul 20, 2026
julietshen
left a comment
julietshen
left a comment
Member
There was a problem hiding this comment.
found a few issues, and i'm happy to help with changes!
Comment on lines
+132
to
+145
| EmbedLink: Optional[str] = JsonData( | ||
| path='$.embedLink', | ||
| required=False, | ||
| ) | ||
|
|
||
| ReplyId: Entity[str] = EntityJson( | ||
| type='PostId', | ||
| path='$.replyId', | ||
| required=False, | ||
| ) | ||
| ``` | ||
|
|
||
| The [`JsonData` UDF](#user-defined-functions-udfs) lets us take the event's JSON and define features based on the contents of that JSON. These features can then be referenced in other rules that we import the `models/record/post.sml` model into. If you have any values inside your JSON object that may not always be present, you can set `required` to `False`, and these features will be `None` whenever the feature is not present. | ||
|
|
Member
There was a problem hiding this comment.
+1 @cassidyjames I think it's supposed to be None but worth confirming
- Fix three cross-links to rules/README.md#... : mdBook renders that page as
rules/index.html, so the inline links became rules/README.html and 404'd.
Point them at the directory (rules/#anchor) instead. Affects labels.md,
integrations.md (x2).
- DidAddLabel entity_type should be the entity's declared type ("User"), not
the feature name "UserId" (examples.md, query-syntax.md).
- concepts.md: rules also evaluate to None (missing signal), not just
True/False.
- local.md: register_ast_validators returns Sequence[Type[BaseValidator]],
not None (matches the hook spec and the hooks table).
- integrations.md: use InputStreamSource.PUBSUB for Google Pub/Sub, not
KAFKA (the surrounding table already maps it to PUBSUB).
Co-Authored-By: cassidyjames <611168+cassidyjames@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
julietshen
self-requested a review
July 21, 2026 03:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docs pass for the 1.1 release
Summary by CodeRabbit
ContainsHellorules/query examples.