Repository navigation
Conversation
An opaque (unregistered) change has no snapshot when it's a create (the entity was never materialized on this client), so LoadChangeContext threw ArgumentNullException from GetAtCommit. Return an empty context for opaque changes instead: "Unknown" name, no preview, no affected entries. Opaque edits get the empty context too; their snapshot would show the entity state excluding the very change being viewed. The tests opt into UnknownChangeHandling.Fallback through a new fixture hook. Production keeps the default Throw for now, so unknown change types fail sync outright and never reach the activity view. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Included review availability: 4 reviews are currently available. Based on recent review activity, included reviews refill at 5 per hour. 📝 WalkthroughWalkthroughThe changes enable fallback deserialization for unknown Harmony changes. ChangesOpaque change handling
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
Comment |
The per-fixture override raced: the IChange converter (which bakes in UnknownChangeHandling) lives in the EF model, and EF caches the model process-wide, so whichever fixture built it first decided the behavior for the whole run. Passed locally when the opaque tests ran first, failed on CI when a default-Throw fixture won. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Review asked not to flip the setting for every test while prod still uses Throw. The per-fixture override originally raced because the setting is baked into the process-cached EF model; the fixture now disables EF service-provider caching whenever a configureServices hook is present, so hook config gets its own model. An isolation test pins the race: it fails if the caching opt-out is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review call: if fallback silently stops applying, the opaque tests fail loudly anyway; the guard isn't worth its review noise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Promoted to sillsdev#2796 |
[Claude, autonomous]
Staging PR — never merge; promoted to sillsdev when polished (see FORK.md).
An opaque (unregistered) change with no snapshot (an opaque create) made the activity detail view throw ArgumentNullException from GetAtCommit; opaque changes now return an empty context ("Unknown", no preview, no affected entries). The new tests opt into UnknownChangeHandling.Fallback via a fixture hook — production still uses the default Throw, so unknown change types currently fail sync before ever reaching the activity view (tracked separately).
🤖 Generated with Claude Code