Skip to content

Conversation

@Mettwasser
Copy link
Member

@Mettwasser Mettwasser commented Aug 17, 2025

What did you fix?

Closes

Additionally, added support for custom base URLs.


Reproduction steps


Evidence/screenshot/link to line

Considerations

  • Does this contain a new dependency? [Yes]
  • Does this introduce opinionated data formatting or manual data entry? [Yes]
  • Does this pr include updated data files in a separate commit that can be reverted for a clean code-only pr? [Yes]
  • Have I run the linter? [Yes]
  • Is is a bug fix, feature request, or enhancement? [Bug Fix/Feature]

Summary by CodeRabbit

  • New Features

    • Configurable worldstate API base URL with Default client construction.
    • Expanded Order data: typed buy/sell, quantity, richer optional metadata, and chrono timestamps.
    • Syndicate variants extended with a safe fallback.
  • Breaking Changes

    • Worldstate query API now accepts a base URL parameter.
    • Client constructor/signature updated to accept an HTTP client and base URL (use Default).
  • Documentation

    • Examples updated to use Client::default() and include base URL in queries.
  • Chores

    • VCS ignore rules updated; test fixtures migrated to JSON files and simplified test harness.

BREAKING CHANGE: API change for Client::new and Queryable::query
…s now its own enum

BREAKING CHANGE: API changes for Order::created_at, Order::updated_at, Order::type
BREAKING CHANGE: SyndicateMission::syndicate_key is now the enum
@Mettwasser Mettwasser requested a review from TobiTenno August 17, 2025 12:00
@Mettwasser Mettwasser self-assigned this Aug 17, 2025
@Mettwasser Mettwasser added the enhancement New feature or request label Aug 17, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 17, 2025

Walkthrough

Repository updates VCS ignores; expands market Order model with a typed OrderType, chrono timestamps, and additional metadata; worldstate client and Queryable/Endpoint APIs now accept a runtime base_url and hold a reqwest client; syndicate enums extended; macro endpoint builders updated; many fixtures moved from Rust fixture wrappers into JSON files and tests switched to file-backed rstest inputs.

Changes

Cohort / File(s) Summary
VCS ignore updates
/.gitignore
Added root-level ignores: /node_modules, /package-lock.json, /package.json under comment # Local Commitlint stuff.
Market order model
src/market/models/order.rs, src/market/mod.rs
Added public OrderType enum and re-exported it; changed Order.r#type: StringOrderType; created_at/updated_at: StringDateTime<Utc>; added quantity: u32 and optional fields per_trade, rank, charges, subtype, amber_stars, cyan_stars, group.
Order-with-user formatting & tests
src/market/models/order_with_user.rs
Moved serde(flatten) placement and renamed test from rivenorder_with_user; no API signature changes.
Worldstate client & Queryable/Endpoint APIs
src/worldstate/client.rs, src/worldstate/models/base.rs, src/worldstate/models/mod.rs, src/worldstate/mod.rs, README.md
Client now holds http: reqwest::Client and base_url: String; removed auto-derive Default and added impl Default; constructor changed to new(reqwest_client, base_url) and docs use Client::default(); Endpoint and Queryable signatures now take base_url: &str and return owned String endpoints; internal calls updated to pass &self.base_url and &self.http.
Syndicate types & missions
src/worldstate/models/syndicate.rs, src/worldstate/models/syndicate_mission.rs
Added #[non_exhaustive] to Syndicate; added serde alias for Cavia, new variants TheHex and Other; SyndicateMission.syndicate_key: StringSyndicate.
Macro-generated endpoint changes
warframe-macros/src/model/struct_impl/args.rs
Endpoint builders changed to accept base_url: &str; endpoint_en/endpoint now return String built by concatenating base_url with relative paths and language.
Fixture migration: Rust fixtures → JSON files
src/worldstate/fixtures/*src/worldstate/models/fixtures/*.json
Removed many Rust fixture! wrapper files and modules; added numerous JSON fixture files (alert.json, arbitration.json, archon_hunt.json, cambion_drift.json, cetus.json, construction_progress.json, daily_deal.json, deep_archimedea.json, event.json, fissure.json, flash_sale.json, global_upgrade.json, invasion.json, item_.json, news.json, nightwave.json, orb_vallis.json, sortie.json, steel_path.json, syndicate_mission.json, void_trader.json, weapon_.json, etc.). Tests updated to use file-backed rstest inputs and simplified imports.
Model surface reductions
src/worldstate/models/*.rs (various)
Removed or renamed several public fields: e.g., News.as_string, Reward.as_string and Reward.item_string, Nightwave.reward_types, Invasion.eta and Invasion.start_string, Fissure.mission_keymission_type_key; tests adjusted accordingly.
Test harness updates
src/worldstate/models/**/*.rs
Many test modules simplified: removed dual inline fixtures, switched to #[files(...)] #[mode = "str"] rstest parameters, and removed obsolete *_ml tests.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Client
  participant Queryable
  participant Endpoint
  participant Reqwest

  Caller->>Client: fetch::<T>()
  Client->>Queryable: T::query(&self.base_url, &self.http)
  Queryable->>Endpoint: endpoint_en(base_url) / endpoint(base_url, lang)
  Endpoint-->>Queryable: String URL
  Queryable->>Reqwest: http.get(URL).send()
  Reqwest-->>Queryable: Response
  Queryable-->>Client: Parsed T::Return
  Client-->>Caller: Result<T::Return>
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Possibly related PRs

Suggested labels

released

Suggested reviewers

  • TobiTenno
  • CraigEge

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title "feat: v8" is concise and uses a conventional-commit prefix but is too terse and does not describe the primary functional changes in this PR (e.g., custom base_url support, Queryable/Endpoint signature changes, Order model/type updates, and extensive fixture/data additions), so it is related but not sufficiently descriptive for reviewers scanning history. Please rename the PR to a short, specific summary of the main change (for example: "feat(worldstate): add custom base_url and update Queryable endpoints" or "feat(market/worldstate): add OrderType, chrono timestamps, and base_url support") and update the description to mention the major public API impacts before merging.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch v8

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Mettwasser Mettwasser enabled auto-merge (rebase) August 17, 2025 12:03
Copy link

@coderabbitai coderabbitai bot left a 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

🔭 Outside diff range comments (2)
src/worldstate/client.rs (2)

165-171: Honor custom base_url for item queries

query_item and query_item_using_lang still hit the hardcoded warframestat host, ignoring self.base_url. Use the configured base URL to keep behavior consistent.

     pub async fn query_item(&self, query: &str) -> Result<Option<Item>, Error> {
-        self.query_by_url(format!(
-            "https://api.warframestat.us/items/{}/?language=en",
-            urlencoding::encode(query),
-        ))
+        self.query_by_url(format!(
+            "{}/items/{}/?language=en",
+            self.base_url,
+            urlencoding::encode(query),
+        ))
         .await
     }
@@
     pub async fn query_item_using_lang(
         &self,
         query: &str,
         language: Language,
     ) -> Result<Option<Item>, Error> {
-        self.query_by_url(format!(
-            "https://api.warframestat.us/items/{}/?language={}",
-            urlencoding::encode(query),
-            language
-        ))
+        self.query_by_url(format!(
+            "{}/items/{}/?language={}",
+            self.base_url,
+            urlencoding::encode(query),
+            language
+        ))
         .await
     }

Also applies to: 198-209


34-34: Update examples: replace zero-arg Client::new() with Client::default() or the new constructor

The Client::new() signature now requires (reqwest_client: reqwest::Client, base_url: String). All remaining zero-arg calls in docs and examples will fail to compile. Please update the following locations:

• src/worldstate/mod.rs:18
• src/worldstate/client.rs:34, 90, 122, 153, 187, 261, 350, 444, 543
• README.md:18

Suggested changes (pick one style across examples):

  1. Simple default constructor:
- let client = Client::new();
+ let client = Client::default();
  1. Explicit new constructor:
let client = Client::new(
    reqwest::Client::new(),
    "https://api.warframestat.us".into(),
);

Quick check to ensure no zero-arg calls remain:

rg -n -C2 -g '!**/target/**' -P '\bClient::new\s*\(\s*\)'
🧹 Nitpick comments (12)
.gitignore (1)

10-13: Root-level Node ignores look fine for local-only tooling.

These will keep local commitlint/Husky artifacts out of VCS. If you later adopt JS tooling in-repo, remember to drop these ignores so package.json/lock can be committed.

If commitlint config or other Node-based tooling is intended to be part of the repo in a future commit, remove the following lines:

  • /package.json
  • /package-lock.json
  • /node_modules
src/worldstate/models/syndicate.rs (2)

46-47: Double-check intent: alias changes serialization behavior for Cavia.

Switching from rename = "EntratiLabSyndicate" to alias = "EntratiLabSyndicate" means:

  • Deserialization accepts both "Cavia" and "EntratiLabSyndicate".
  • Serialization will emit "Cavia" (not "EntratiLabSyndicate").

If you need to maintain previous serialization output for backward compatibility (while still accepting the new name), prefer keeping rename and adding an alias.

Apply if you want to preserve the old serialized value:

-    #[serde(alias = "EntratiLabSyndicate")]
+    #[serde(rename = "EntratiLabSyndicate", alias = "Cavia")]

56-63: Verified HexSyndicate mapping; optional test fixture recommended.

Confirmed via the official worldstate JSON (https://content.warframe.com/dynamic/worldState.php) that The Hex is indeed represented as "HexSyndicate". The existing

#[serde(rename = "HexSyndicate")]
TheHex,
#[serde(other)]
Other,

is correct and future-proof.

• No changes needed in src/worldstate/models/syndicate.rs for the serde attributes.
• Optionally, add a small test (e.g. in a tests/ module or below the enum) to assert that:

  • deserializing "HexSyndicate" yields Syndicate::TheHex
  • deserializing an unknown string yields Syndicate::Other
src/worldstate/models/syndicate_mission.rs (1)

45-46: Typed syndicate_key improves safety; note the public API change.

Changing StringSyndicate is a breaking change for consumers matching on raw strings. Given this is v8, that’s fine, but call it out in the release notes/migration guide.

Consider adding a test that asserts a concrete variant (e.g., a known syndicate value) to catch accidental string mismatches early, rather than only checking that deserialization succeeds.

src/market/models/order.rs (1)

14-16: Reconsider deriving Ord/PartialOrd on the full Order struct.

Auto-derived ordering is lexicographic by field order (starting with id), which likely isn’t what you want if you ever sort by recency (created_at). This can lead to surprising behavior downstream.

Option A (simplest): drop ordering derives to avoid accidental misuse.

-#[derive(Debug, Clone, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Debug, Clone, Deserialize, PartialEq, Eq, Hash)]

Option B: implement explicit ordering by created_at, then id as a tiebreaker.

impl core::cmp::Ord for Order {
    fn cmp(&self, other: &Self) -> core::cmp::Ordering {
        self.created_at
            .cmp(&other.created_at)
            .then_with(|| self.id.cmp(&other.id))
    }
}
impl core::cmp::PartialOrd for Order {
    fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
        Some(self.cmp(other))
    }
}
src/market/models/order_with_user.rs (1)

34-34: Test rename is fine; consider asserting more than parse success.

Given Order gained typed fields (enum/time), add a quick assertion to ensure those fields deserialize as expected (e.g., order.r#type == OrderType::Sell and created_at parsed), guarding against format regressions.

I can extend the test to assert OrderType and a parsed created_at if you’d like.

src/worldstate/models/mod.rs (1)

27-29: Docs: remove trailing slash from base URL to avoid //pc...

Examples pass "https://api.warframestat.us/" but the code constructs URLs as {base_url}/pc..., yielding a double slash when a trailing slash is provided. Use "https://api.warframestat.us" instead for consistency with Client::default() and to avoid // in URLs.

Apply:

-//!     let cetus: Cetus = Cetus::query("https://api.warframestat.us/", &reqwest_client).await?;
+//!     let cetus: Cetus = Cetus::query("https://api.warframestat.us", &reqwest_client).await?;
-//!
-//!     let fissures: Vec<Fissure> =
-//!         Fissure::query("https://api.warframestat.us/", &reqwest_client).await?;
+//!
+//!     let fissures: Vec<Fissure> =
+//!         Fissure::query("https://api.warframestat.us", &reqwest_client).await?;
warframe-macros/src/model/struct_impl/args.rs (1)

38-49: Trim trailing / in base_url before formatting to prevent // in final URLs

If callers pass a base URL with a trailing slash, current formatting will produce https://host.tld//pc.... Normalize base_url once in the generated impls.

         impl crate::worldstate::models::base::Endpoint for #struct_name {
-            fn endpoint_en(base_url: &str) -> String {
-                format!("{}{}", base_url, #endpoint_en)
-            }
+            fn endpoint_en(base_url: &str) -> String {
+                let base = base_url.trim_end_matches('/');
+                format!("{base}{}", #endpoint_en)
+            }
             fn endpoint(base_url: &str, language: crate::worldstate::language::Language) -> String {
-                format!(
-                    "{}/pc{}/?language={}",
-                    base_url,
-                    #endpoint_value,
-                    language
-                )
+                let base = base_url.trim_end_matches('/');
+                format!("{}/pc{}/?language={}", base, #endpoint_value, language)
             }
         }

Also applies to: 42-52

src/worldstate/models/base.rs (2)

25-33: Docstring is outdated: english endpoint no longer “free/compile-time”

Now that endpoint_en takes a runtime base_url and returns a String, the comment implying compile-time concatenation is inaccurate. Update to reflect runtime formatting and allocation.


84-90: Propagate non-2xx HTTP statuses with error_for_status()

Currently, a non-2xx response falls through to JSON parsing, which can mask HTTP errors. Use error_for_status() to surface HTTP errors consistently.

-            Ok(request_executor
-                .get(Self::endpoint_en(base_url))
-                .send()
-                .await?
-                .json::<Self::Return>()
-                .await?)
+            Ok(request_executor
+                .get(Self::endpoint_en(base_url))
+                .send()
+                .await?
+                .error_for_status()?
+                .json::<Self::Return>()
+                .await?)
-            Ok(request_executor
-                .get(Self::endpoint(base_url, language))
-                .send()
-                .await?
-                .json::<Self::Return>()
-                .await?)
+            Ok(request_executor
+                .get(Self::endpoint(base_url, language))
+                .send()
+                .await?
+                .error_for_status()?
+                .json::<Self::Return>()
+                .await?)

Also applies to: 102-108

src/worldstate/client.rs (2)

50-60: Nit: Docstring shows trailing slash, default value does not

Doc says default requests go to https://api.warframestat.us/, but default base_url is "https://api.warframestat.us". Align to avoid confusion.

-    /// Requests are sent to `https://api.warframestat.us/` by default.
+    /// Requests are sent to `https://api.warframestat.us` by default.

62-70: Sanitize base_url in constructor to avoid accidental // joins

Users may pass a URL with a trailing slash. Normalize it once on construction.

-    pub fn new(reqwest_client: reqwest::Client, base_url: String) -> Self {
-        Self {
-            http: reqwest_client,
-            base_url,
-        }
-    }
+    pub fn new(reqwest_client: reqwest::Client, base_url: String) -> Self {
+        let base_url = base_url.trim_end_matches('/').to_string();
+        Self { http: reqwest_client, base_url }
+    }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a20b75c and ef5a5b6.

📒 Files selected for processing (9)
  • .gitignore (1 hunks)
  • src/market/models/order.rs (2 hunks)
  • src/market/models/order_with_user.rs (2 hunks)
  • src/worldstate/client.rs (4 hunks)
  • src/worldstate/models/base.rs (3 hunks)
  • src/worldstate/models/mod.rs (1 hunks)
  • src/worldstate/models/syndicate.rs (3 hunks)
  • src/worldstate/models/syndicate_mission.rs (2 hunks)
  • warframe-macros/src/model/struct_impl/args.rs (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
warframe-macros/src/model/struct_impl/args.rs (1)
src/worldstate/models/base.rs (2)
  • endpoint_en (27-27)
  • endpoint (32-32)
src/worldstate/client.rs (1)
src/worldstate/models/base.rs (2)
  • query (79-91)
  • query_with_language (96-109)
🔇 Additional comments (6)
src/worldstate/models/syndicate.rs (1)

5-5: Good call adding #[non_exhaustive].

Prevents downstream exhaustive matches from breaking as DE adds new syndicates.

src/worldstate/models/syndicate_mission.rs (1)

3-4: Import path LGTM.

Pulling Syndicate from crate::worldstate::queryable matches the crate’s re-export patterns used elsewhere.

src/market/models/order.rs (2)

7-12: Strongly-typed OrderType is a solid improvement.

#[serde(rename_all = "camelCase")] correctly maps to "sell"/"buy". Deriving Copy and comparisons makes sense for a small enum.


51-55: Chrono timestamps for created_at/updated_at are appropriate.

The worldstate/market APIs use ISO 8601; DateTime<Utc> deserializes cleanly. Good move away from raw strings.

src/market/models/order_with_user.rs (1)

18-19: Serde flatten positioning change is benign.

Moving #[serde(flatten)] below the doc comment has no semantic effect. All good.

src/worldstate/client.rs (1)

45-48: LGTM: Client fields and visibility refactor are clear

The move to http: reqwest::Client and base_url: String is straightforward and aligns with the new URL strategy.

@Mettwasser
Copy link
Member Author

One small thing: Due to the change with the custom URLs, the english endpoints are NOT allocation-free anymore. So it has a bit more overhead. Not a big deal but I thought it's worth mentioning

@Mettwasser
Copy link
Member Author

Actionable comments posted: 0
🔭 Outside diff range comments (2)

🧹 Nitpick comments (12)

📜 Review details

Great catch!

BREAKING CHANGE: removed as_string and item_string
BREAKING CHANGE: renamned mission_key to mission_type_key
BREAKING CHANGE: removed the eta and start_string field
BREAKING CHANGE: removed the reward_types field
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (9)
src/worldstate/models/flash_sale.rs (1)

38-44: Rstest fixture wiring looks good; optional better error paths on parse failures.

Current parse will panic without field path context. Consider serde_path_to_error for clearer failures.

Apply this diff within the test body:

-        from_str::<R>(flash_sale_en).unwrap();
+        let mut de = serde_json::Deserializer::from_str(flash_sale_en);
+        serde_path_to_error::deserialize::<_, R>(&mut de).unwrap();

And adjust imports in the test module:

use serde_json::Deserializer; // replace from_str import

If not already present, add a dev-dependency:

[dev-dependencies]
serde_path_to_error = "0.1"
src/worldstate/models/global_upgrades.rs (1)

33-39: Strengthen the test — rename and assert fixture non-empty.

Verified: fixture top-level is a JSON array.

-    fn test(
+    fn parses_fixture(
         #[files("src/worldstate/models/fixtures/global_upgrade.json")]
         #[mode = str]
         global_upgrade_en: &str,
     ) {
-        from_str::<R>(global_upgrade_en).unwrap();
+        let items: R = from_str(global_upgrade_en).unwrap();
+        assert!(!items.is_empty(), "fixture should contain at least one GlobalUpgrade");
     }
src/worldstate/models/items/weapon.rs (1)

358-358: Prefer a relative import to reduce path brittleness.

use super::Weapon; is shorter and resilient to module path changes.

Apply this diff:

-    use crate::worldstate::models::items::weapon::Weapon;
+    use super::Weapon;
src/worldstate/models/void_trader.rs (1)

41-47: Optional: add build.rs to track glob changes

When using #[files] + #[mode = str], adding/removing files matching the glob won’t trigger rebuilds unless you tell Cargo. Consider a minimal build.rs that prints cargo::rerun-if-changed for the fixtures dir. Reference: rstest docs “Files path as input arguments.” (docs.rs)

+// build.rs
+fn main() {
+    println!("cargo::rerun-if-changed=src/worldstate/models/fixtures");
+}
src/worldstate/models/fixtures/item_nanospores.json (1)

1-6: Microcopy nit: missing whitespace in description

There’s no space after the period: “behandeln.Quelle”. Consider “behandeln. Quelle”.

-    "description": "Faseriger Technocyte-Tumor. Befallenes Gewebe ist mit Vorsicht zu behandeln.Quelle: Saturn, Neptun, Eris und Deimos.",
+    "description": "Faseriger Technocyte-Tumor. Befallenes Gewebe ist mit Vorsicht zu behandeln. Quelle: Saturn, Neptun, Eris und Deimos.",
src/worldstate/models/arbitration.rs (1)

17-35: Be more selective about invalid-expiry fallback

Mapping all Invalid/OutOfRange parse errors to MAX_UTC can mask malformed data. Consider limiting the fallback to OutOfRange only and surface other parse errors.

-        .or_else(|err| {
-            if matches!(
-                err.kind(),
-                chrono::format::ParseErrorKind::OutOfRange
-                    | chrono::format::ParseErrorKind::Invalid
-            ) {
+        .or_else(|err| {
+            if matches!(err.kind(), chrono::format::ParseErrorKind::OutOfRange) {
                 Ok(DateTime::<Utc>::MAX_UTC)
             } else {
                 Err(serde::de::Error::custom(err.to_string()))
             }
         })
devtools.nu (1)

46-50: Avoid path traversal, ensure dir exists, and validate JSON before writing

Current implementation may overwrite arbitrary paths if file_name contains ../ and doesn’t validate JSON. Recommend sanitizing the filename, creating the directory, and pretty‑writing validated JSON.

-export def "worldstate-fixture create" [
-    file_name:string, # The name of the fixture file to create
-]: nothing -> nothing {
-    let file_path = $"src/worldstate/models/fixtures/($file_name).json"
-    clipboard paste | save $file_path
-}
+export def "worldstate-fixture create" [
+    file_name:string, # The name of the fixture file to create (basename only, no paths)
+]: nothing -> nothing {
+    let dir = "src/worldstate/models/fixtures"
+    mkdir $dir
+    # sanitize: strip directories and replace illegal chars
+    let safe_name = ($file_name | path basename | str replace -r '[^A-Za-z0-9_.-]' '_' )
+    let file_path = $"($dir)/($safe_name).json"
+    # validate & pretty‑format JSON from clipboard; fail if invalid
+    let json = (clipboard paste | from json)
+    $json | to json -p 2 | save --raw --force $file_path
+}
src/worldstate/models/construction_progress.rs (1)

32-38: Rstest file-backed fixture: good; add cheap sanity asserts.

Parsing-only tests can miss obvious NaN/inf regressions. Add minimal finiteness checks.

-    ) {
-        from_str::<R>(construction_progress_en).unwrap();
-    }
+    ) {
+        let cp: R = from_str::<R>(construction_progress_en).unwrap();
+        assert!(cp.fomorian_progress.is_finite());
+        assert!(cp.razorback_progress.is_finite());
+        assert!(cp.unknown_progress.is_finite());
+    }
src/worldstate/models/fixtures/alert.json (1)

1-80: Fixture looks solid; consider adding edge-case coverage.

To exercise more code paths, include at least one alert with non-empty countedItems, and a nightmare: true or archwingRequired: true case.

If acceptable, tweak one entry:

-                "countedItems": [],
+                "countedItems": [ { "count": 150, "type": "Kuva" } ],

Confirm the Reward model handles countedItems as in production responses.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4116f3a and 9fb6947.

📒 Files selected for processing (74)
  • devtools.nu (1 hunks)
  • src/worldstate/fixtures/alert.rs (0 hunks)
  • src/worldstate/fixtures/arbitration.rs (0 hunks)
  • src/worldstate/fixtures/archon_hunt.rs (0 hunks)
  • src/worldstate/fixtures/cambion_drift.rs (0 hunks)
  • src/worldstate/fixtures/cetus.rs (0 hunks)
  • src/worldstate/fixtures/construction_progress.rs (0 hunks)
  • src/worldstate/fixtures/daily_deal.rs (0 hunks)
  • src/worldstate/fixtures/deep_archimedea.rs (0 hunks)
  • src/worldstate/fixtures/event.rs (0 hunks)
  • src/worldstate/fixtures/fissure.rs (0 hunks)
  • src/worldstate/fixtures/flash_sale.rs (0 hunks)
  • src/worldstate/fixtures/global_upgrade.rs (0 hunks)
  • src/worldstate/fixtures/invasion.rs (0 hunks)
  • src/worldstate/fixtures/item.rs (0 hunks)
  • src/worldstate/fixtures/mod.rs (0 hunks)
  • src/worldstate/fixtures/news.rs (0 hunks)
  • src/worldstate/fixtures/nightwave.rs (0 hunks)
  • src/worldstate/fixtures/orb_vallis.rs (0 hunks)
  • src/worldstate/fixtures/sortie.rs (0 hunks)
  • src/worldstate/fixtures/steel_path.rs (0 hunks)
  • src/worldstate/fixtures/syndicate_mission.rs (0 hunks)
  • src/worldstate/fixtures/void_trader.rs (0 hunks)
  • src/worldstate/mod.rs (1 hunks)
  • src/worldstate/models/alert.rs (1 hunks)
  • src/worldstate/models/arbitration.rs (1 hunks)
  • src/worldstate/models/archon_hunt.rs (1 hunks)
  • src/worldstate/models/cambion_drift.rs (1 hunks)
  • src/worldstate/models/cetus.rs (1 hunks)
  • src/worldstate/models/construction_progress.rs (1 hunks)
  • src/worldstate/models/daily_deal.rs (1 hunks)
  • src/worldstate/models/deep_archimedea.rs (1 hunks)
  • src/worldstate/models/event.rs (1 hunks)
  • src/worldstate/models/fissure.rs (2 hunks)
  • src/worldstate/models/fixtures/alert.json (1 hunks)
  • src/worldstate/models/fixtures/arbitration.json (1 hunks)
  • src/worldstate/models/fixtures/archon_hunt.json (1 hunks)
  • src/worldstate/models/fixtures/cambion_drift.json (1 hunks)
  • src/worldstate/models/fixtures/cetus.json (1 hunks)
  • src/worldstate/models/fixtures/construction_progress.json (1 hunks)
  • src/worldstate/models/fixtures/daily_deal.json (1 hunks)
  • src/worldstate/models/fixtures/deep_archimedea.json (1 hunks)
  • src/worldstate/models/fixtures/event.json (1 hunks)
  • src/worldstate/models/fixtures/fissure.json (1 hunks)
  • src/worldstate/models/fixtures/flash_sale.json (1 hunks)
  • src/worldstate/models/fixtures/global_upgrade.json (1 hunks)
  • src/worldstate/models/fixtures/invasion.json (1 hunks)
  • src/worldstate/models/fixtures/item_nanospores.json (1 hunks)
  • src/worldstate/models/fixtures/item_sigil.json (1 hunks)
  • src/worldstate/models/fixtures/news.json (1 hunks)
  • src/worldstate/models/fixtures/nightwave.json (1 hunks)
  • src/worldstate/models/fixtures/orb_vallis.json (1 hunks)
  • src/worldstate/models/fixtures/sortie.json (1 hunks)
  • src/worldstate/models/fixtures/steel_path.json (1 hunks)
  • src/worldstate/models/fixtures/syndicate_mission.json (1 hunks)
  • src/worldstate/models/fixtures/void_trader.json (1 hunks)
  • src/worldstate/models/fixtures/weapon_melee.json (1 hunks)
  • src/worldstate/models/fixtures/weapon_ranged.json (2 hunks)
  • src/worldstate/models/flash_sale.rs (1 hunks)
  • src/worldstate/models/global_upgrades.rs (1 hunks)
  • src/worldstate/models/invasion.rs (1 hunks)
  • src/worldstate/models/items/mod.rs (1 hunks)
  • src/worldstate/models/items/weapon.rs (2 hunks)
  • src/worldstate/models/mission_type.rs (1 hunks)
  • src/worldstate/models/news.rs (1 hunks)
  • src/worldstate/models/nightwave.rs (1 hunks)
  • src/worldstate/models/orb_vallis.rs (1 hunks)
  • src/worldstate/models/reward.rs (0 hunks)
  • src/worldstate/models/reward_type.rs (1 hunks)
  • src/worldstate/models/sortie.rs (1 hunks)
  • src/worldstate/models/steel_path.rs (1 hunks)
  • src/worldstate/models/syndicate.rs (3 hunks)
  • src/worldstate/models/syndicate_mission.rs (3 hunks)
  • src/worldstate/models/void_trader.rs (1 hunks)
💤 Files with no reviewable changes (23)
  • src/worldstate/models/reward.rs
  • src/worldstate/fixtures/steel_path.rs
  • src/worldstate/fixtures/global_upgrade.rs
  • src/worldstate/fixtures/flash_sale.rs
  • src/worldstate/fixtures/mod.rs
  • src/worldstate/fixtures/sortie.rs
  • src/worldstate/fixtures/construction_progress.rs
  • src/worldstate/fixtures/item.rs
  • src/worldstate/fixtures/invasion.rs
  • src/worldstate/fixtures/nightwave.rs
  • src/worldstate/fixtures/syndicate_mission.rs
  • src/worldstate/fixtures/archon_hunt.rs
  • src/worldstate/fixtures/event.rs
  • src/worldstate/fixtures/void_trader.rs
  • src/worldstate/fixtures/alert.rs
  • src/worldstate/fixtures/daily_deal.rs
  • src/worldstate/fixtures/cambion_drift.rs
  • src/worldstate/fixtures/deep_archimedea.rs
  • src/worldstate/fixtures/fissure.rs
  • src/worldstate/fixtures/arbitration.rs
  • src/worldstate/fixtures/news.rs
  • src/worldstate/fixtures/orb_vallis.rs
  • src/worldstate/fixtures/cetus.rs
✅ Files skipped from review due to trivial changes (25)
  • src/worldstate/models/fixtures/nightwave.json
  • src/worldstate/models/fixtures/void_trader.json
  • src/worldstate/models/fixtures/event.json
  • src/worldstate/models/fixtures/archon_hunt.json
  • src/worldstate/models/archon_hunt.rs
  • src/worldstate/models/fixtures/construction_progress.json
  • src/worldstate/models/fixtures/cambion_drift.json
  • src/worldstate/models/fixtures/deep_archimedea.json
  • src/worldstate/models/fixtures/item_sigil.json
  • src/worldstate/models/fixtures/sortie.json
  • src/worldstate/models/mission_type.rs
  • src/worldstate/models/fixtures/syndicate_mission.json
  • src/worldstate/models/fixtures/invasion.json
  • src/worldstate/models/reward_type.rs
  • src/worldstate/models/fixtures/global_upgrade.json
  • src/worldstate/models/fixtures/fissure.json
  • src/worldstate/models/fixtures/news.json
  • src/worldstate/models/fixtures/orb_vallis.json
  • src/worldstate/models/fixtures/cetus.json
  • src/worldstate/models/fixtures/daily_deal.json
  • src/worldstate/models/fixtures/flash_sale.json
  • src/worldstate/models/fixtures/weapon_ranged.json
  • src/worldstate/models/fixtures/arbitration.json
  • src/worldstate/models/fixtures/steel_path.json
  • src/worldstate/models/fixtures/weapon_melee.json
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/worldstate/models/syndicate_mission.rs
  • src/worldstate/models/syndicate.rs
  • src/worldstate/mod.rs
🧰 Additional context used
🧬 Code graph analysis (18)
src/worldstate/models/deep_archimedea.rs (2)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/invasion.rs (2)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/arbitration.rs (16)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/archon_hunt.rs (1)
  • test (72-78)
src/worldstate/models/cambion_drift.rs (1)
  • test (34-40)
src/worldstate/models/cetus.rs (1)
  • test (34-40)
src/worldstate/models/construction_progress.rs (1)
  • test (32-38)
src/worldstate/models/daily_deal.rs (1)
  • test (41-47)
src/worldstate/models/event.rs (1)
  • test (66-72)
src/worldstate/models/fissure.rs (1)
  • test (76-82)
src/worldstate/models/flash_sale.rs (1)
  • test (38-44)
src/worldstate/models/global_upgrades.rs (1)
  • test (33-39)
src/worldstate/models/news.rs (1)
  • test (49-55)
src/worldstate/models/orb_vallis.rs (1)
  • test (34-40)
src/worldstate/models/sortie.rs (1)
  • test (50-56)
src/worldstate/models/syndicate_mission.rs (1)
  • test (65-71)
src/worldstate/models/alert.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/arbitration.rs (1)
  • test (93-99)
src/worldstate/models/nightwave.rs (1)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/construction_progress.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/void_trader.rs (2)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/news.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/sortie.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/daily_deal.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/cambion_drift.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/void_trader.rs (1)
  • test (42-48)
src/worldstate/models/orb_vallis.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/fissure.rs (1)
  • test (76-82)
src/worldstate/models/fissure.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/global_upgrades.rs (2)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/flash_sale.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/event.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/invasion.rs (1)
  • test (76-82)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/steel_path.rs (3)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/alert.rs (1)
  • test (32-38)
src/worldstate/models/fissure.rs (1)
  • test (76-82)
src/worldstate/models/cetus.rs (2)
src/worldstate/models/nightwave.rs (1)
  • test (95-101)
src/worldstate/models/alert.rs (1)
  • test (32-38)
🔇 Additional comments (23)
src/worldstate/models/flash_sale.rs (1)

33-36: LGTM: Trait-based Return alias keeps tests decoupled.

Importing Queryable and using <FlashSale as Queryable>::Return is a solid way to bind the test to the API shape without duplicating types.

src/worldstate/models/global_upgrades.rs (1)

28-28: LGTM: Using Queryable::Return in tests keeps them aligned with the macro’s return_style.
This import is correct and makes the test resilient to future API/return type changes.

src/worldstate/models/steel_path.rs (2)

33-33: LGTM: importing Queryable directly keeps the test coupled to the public API.
Clean swap from bespoke fixtures to trait-based return type. No issues spotted.


38-42: RSTest #[files(...)]/#[mode = str] usage and fixture verified.

Fixture present: src/worldstate/models/fixtures/steel_path.json — jq validated JSON. Cargo.lock lists rstest = 0.25.0; rstest documents #[files(...)] and #[mode = str]. (docs.rs)

  • Optional: rename fn test -> fn deserialize_ok for clarity.
src/worldstate/models/void_trader.rs (2)

37-46: Tests switched to file-backed fixtures: LGTM

Importing Queryable and using rstest’s #[files] with #[mode = str] is correct and keeps the test concise.


41-47: Fixtures: all #[files(...)] paths exist (verified)
Ran the provided script; every referenced fixture file was present in the repo.

src/worldstate/models/deep_archimedea.rs (1)

73-82: Tests: concise and consistent with suite

Good move to file-backed rstest input; aligns with other model tests.

src/worldstate/models/news.rs (1)

44-53: Tests: OK

Import + #[files]/#[mode = str] usage looks correct; deserialization smoke test remains clear.

src/worldstate/models/items/mod.rs (1)

232-242: Tests: item enum coverage looks good

Direct Item import + two representative fixtures (Sigil, Misc) is a solid sanity check.

src/worldstate/models/sortie.rs (1)

45-54: Tests: approved

Queryable import and file-backed fixture parameterization are correct.

src/worldstate/models/cambion_drift.rs (1)

29-38: Tests: looks good

Consistent with the new fixture-driven pattern; Queryable import is correct.

src/worldstate/models/invasion.rs (1)

71-80: Tests: approved

Fixture-based rstest parameterization is correct; keeps the test minimal.

src/worldstate/models/arbitration.rs (1)

88-97: Arbitration test migration to file fixture: LGTM

Import trim + rstest #[files] usage aligns with the repo-wide approach.

src/worldstate/models/cetus.rs (1)

29-38: Cetus test switch to external fixture: LGTM

Consistent with other model tests; no issues spotted.

src/worldstate/models/alert.rs (1)

27-36: Alert test refactor to #[files]: LGTM

Looks consistent; parsing via serde_json::from_str:: is sufficient here.

src/worldstate/models/fissure.rs (1)

71-80: Test migration for fissures: LGTM

Fixture-driven parsing aligns with the rest.

Given the field change above, ensure the fixture isn’t masking the bug by using a non-existent key name.

src/worldstate/models/nightwave.rs (2)

90-99: Nightwave test migration to file fixture: LGTM

Consistent with other models.


65-82: Breaking change — reward_types removed from Nightwave

  • reward_types was removed from the public Nightwave struct (src/worldstate/models/nightwave.rs:65-82). Add a MIGRATION note and changelog entry.
  • Confirm downstream crates/consumers have been updated and that no references to Nightwave.reward_types remain.
src/worldstate/models/orb_vallis.rs (1)

29-38: Orb Vallis test migration to file fixture: LGTM

Matches the new standardized testing approach.

src/worldstate/models/daily_deal.rs (1)

36-45: Test refactor to file-backed fixture looks good — verify fixture paths locally

Import path simplification and rstest file fixture usage are consistent with the new pattern.

The verification script produced repeated "/etc/profile: line 4: id: command not found" and no results; this is inconclusive. Run the check below locally and paste the output or confirm all referenced fixture files exist:

#!/bin/bash
# Verify that all test #[files(...)] targets exist
rg -nP '\#\[files\("([^"]+)"\)\]' -g 'src/worldstate/models/**' -N \
  | awk -F\" '{print $2}' \
  | xargs -I{} bash -lc 'test -f "{}" || echo "Missing: {}"'
src/worldstate/models/construction_progress.rs (1)

27-27: Import cleanup LGTM.

Using Queryable directly simplifies the test module.

src/worldstate/models/event.rs (2)

61-61: Import cleanup LGTM.

Consistent with other model tests.


66-70: Rstest fixture wiring LGTM.

Matches the established pattern (array return style).

@Mettwasser Mettwasser disabled auto-merge September 20, 2025 14:41
@Mettwasser Mettwasser merged commit 44fc133 into master Sep 20, 2025
4 checks passed
@Mettwasser Mettwasser deleted the v8 branch September 20, 2025 14:41
@wfcd-bot-boi
Copy link
Collaborator

🎉 This PR is included in version 8.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
Member

@TobiTenno TobiTenno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants