-
Notifications
You must be signed in to change notification settings - Fork 0
Add basic DDlog schema and fix build #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add basic DDlog schema and fix build #17
Conversation
Reviewer's GuideThis PR refactors the server and middleware modules for cleaner builds by streamlining documentation, applying File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 32 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (3)
WalkthroughA new DDlog module for an experimental physics engine was introduced, defining data types and relations for positions, blocks, and slopes, along with logic for occupied positions. In Rust source files, documentation and example comments were removed or condensed, and test modules were reorganised without altering any code logic or public APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DDlogEngine
User->>DDlogEngine: Provide Blocks and Slopes input
DDlogEngine->>DDlogEngine: Apply Occupied rules
DDlogEngine-->>User: Output Occupied positions
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai review |
✅ Actions performedReview triggered.
|
…and-slopes` Docstrings generation was requested by @leynos. * #17 (comment) The following files were modified: * `src/extractor.rs` * `src/middleware.rs` * `src/server.rs`
Note Generated docstrings for this pull request at #18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
physics/physics.dl (1)
36-38
: Avoid generating superfluous duplicates inOccupied
Occupied
is derived three times and DDlog’s set semantics deduplicate identical facts, so this is not wrong.
If you ever add aggregation onOccupied
, however, emitting three copies of the same position will inflate counts.
A single rule that unions the three sources is marginally clearer:-output relation Occupied(p: Position) -Occupied(p) :- Blocks(Block{pos=p, ..}). -Occupied(p) :- Slopes(Slope{from=p, ..}). -Occupied(p) :- Slopes(Slope{to=p, ..}). +output relation Occupied(p: Position) +Occupied(p) :- + Blocks(Block{pos=p, ..}); + Slopes(Slope{from=p, ..}); + Slopes(Slope{to=p, ..}).src/extractor.rs (1)
59-82
: Consolidate duplicated docs and move#[must_use]
There are two independent doc blocks for
new
, and the attribute sits in the middle of them. Collapse them to one coherent comment to avoid rustdoc noise and keep the attribute adjacent to the function sig.- /// Construct a new [`SharedState`]. - /// - /// # Examples - /// ... - /// ``` - #[must_use] - /// Creates a new `SharedState` instance wrapping the provided `Arc<T>`. - /// - /// # Examples + /// Construct a new [`SharedState`]. + /// + /// # Examples /// /// ``` /// use std::sync::Arc; /// use wireframe::extractor::SharedState; @@ - pub fn new(inner: Arc<T>) -> Self { + #[must_use] + pub fn new(inner: Arc<T>) -> Self {src/middleware.rs (1)
9-26
: Consider allowing mutable services inNext::call
Next::call
takes&self
, which means middleware that needs interior mutability must rely onMutex/Cell
.
Accepting&mut self
would give middleware authors the option of simple mutable state while still working for&self
services thanks to auto-reborrowing:- pub async fn call(&self, req: ServiceRequest) -> Result<ServiceResponse, S::Error> { + pub async fn call(&mut self, req: ServiceRequest) -> Result<ServiceResponse, S::Error> {This is optional, but switching later is a breaking change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
physics/physics.dl
(1 hunks)src/extractor.rs
(3 hunks)src/middleware.rs
(1 hunks)src/server.rs
(3 hunks)
🔇 Additional comments (3)
physics/physics.dl (1)
24-28
: Clarifygradient
semantics for future‐proofing
gradient
is stored as a plaini32
, but the comment does not explain the unit or expected range (e.g. rise-over-run, milli-percent, signed angle). Down-stream rules and client code will need a stable contract. Consider adding a short comment or a type alias such astype Gradient = i32
so the intent can evolve without breaking the schema.src/extractor.rs (1)
109-129
: Tests look good and are now in an isolated moduleRelocating the payload tests under a dedicated
#[cfg(test)]
keeps the impl clean while maintaining coverage. The assertions cover both boundary cases; no further action needed.src/server.rs (1)
29-37
: Constructor initialisation LGTMInitialising
listener
toNone
explicitly removes any ambiguity and makes the invariant inrun
clearer. No issues spotted.
Wrong repository 😱 |
Summary
SharedState
docs and testsTesting
cargo fmt --all -- --check
cargo clippy -- -D warnings
cargo test
https://chatgpt.com/codex/tasks/task_e_684c36bd8b7c8322a1b645fb14bb6211
Summary by Sourcery
Rewrite server and middleware modules to compile cleanly, fix SharedState docs and tests, and introduce a basic DDlog schema for physics engine.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Documentation
Style