diff --git a/.squad/agents/fenster/history.md b/.squad/agents/fenster/history.md new file mode 100644 index 00000000..6a19d9b4 --- /dev/null +++ b/.squad/agents/fenster/history.md @@ -0,0 +1,32 @@ +# Fenster's Project Knowledge + +## Learnings + +### 2026-05-03: Issue #200 - Cloze ResourceId Decoupling + +**Files touched:** +- `src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs` (Layer 1) +- `src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs` (Layer 2) +- `src/SentenceStudio.UI/Pages/Index.razor` (Layer 3) +- `src/SentenceStudio.UI/Pages/Cloze.razor` (Layer 4a) +- `src/SentenceStudio.AppLib/Services/ClozureService.cs` (Layer 4b) + +**What was tricky:** +- ClozureService refactoring required extracting common AI generation logic into `GenerateSentencesFromWords()` helper to avoid code duplication between resource-driven and vocabulary-driven paths +- Had to mirror VocabQuiz's SRS filtering logic exactly (grace period exclusion, NextReviewDate checks, unseen word handling) +- The 40-word cap for AI prompts is critical for both paths — dynamic resources can return thousands of eligible words + +**What surprised me:** +- Persisted plan items bypass PlanConverter entirely — Index.razor reads `item.ResourceId` directly, which is why Layer 3 guard is mandatory even after Layer 1 fix ships +- Layer 4 defense-in-depth (DueOnly check in Cloze.razor) works correctly even when URL has resourceId leaked from persisted plan +- The ClozureService method signature needed a default parameter (`bool dueOnly = false`) to avoid breaking existing callers + +**Key pattern:** +When adding vocabulary-driven mode to an activity: +1. Layer 1 prevents NEW plans from carrying ResourceId +2. Layer 2 sets DueOnly route param for plan-initiated launches +3. Layer 3 guards against ResourceId leak from OLD persisted plans +4. Layer 4 implements dual-mode loading (DueOnly=true → global vocab, DueOnly=false → resource-filtered) + +**Reusable for:** +Any activity that should load from full user vocab pool when launched from Today's Plan but remain resource-filtered when launched directly from a resource detail page. diff --git a/.squad/agents/keaton/history.md b/.squad/agents/keaton/history.md new file mode 100644 index 00000000..3b317608 --- /dev/null +++ b/.squad/agents/keaton/history.md @@ -0,0 +1,37 @@ +# Keaton's History + +## Learnings + +### Issue #200 - Cloze Activity ResourceId Decoupling (2026-05-03) + +**Root Cause:** Cloze activity is missing the 4-layer ResourceId Decoupling Pattern that was applied to VocabQuiz (commits 88a0272, c081a63) and VocabMatching (commit 0c8e197). + +**Investigation Files:** +- `src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs:690,755` — Cloze is categorized as an output activity but NOT marked as vocabulary-driven +- `src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs:502-510` — Cloze activities ARE created with `ResourceId = resource.Id` (resource-driven) +- `src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs:81-89` — Cloze enum case exists, but NO special handling (unlike VocabReview/VocabGame) +- `src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs:121-159` — `BuildRouteParameters` has NO Cloze branch; falls through to resource-based default +- `src/SentenceStudio.UI/Pages/Index.razor:974-1012` — `LaunchPlanItem` guard at lines 984-986 ONLY excludes VocabReview + VocabGame; Cloze leaks ResourceId +- `src/SentenceStudio.UI/Pages/Cloze.razor:146-227` — NO `DueOnly` parameter; `LoadSentences` calls `ClozureSvc.GetSentences(resourceId, 8, skillId)` directly +- `src/SentenceStudio.AppLib/Services/ClozureService.cs:43-69` — Hard requires non-empty resourceID + skillID; returns empty list otherwise (line 50-59) + +**Pattern Comparison:** + +| Layer | VocabQuiz (✅ Fixed) | VocabMatching (✅ Fixed) | Cloze (❌ Missing) | +|-------|---------------------|-------------------------|-------------------| +| **1. Plan Builder** | ResourceId = null (line 460) | ResourceId = null (line 520) | ResourceId = resource.Id (line 505) | +| **2. PlanConverter** | DueOnly = true (line 128) | DueOnly = true (line 134) | No special handling; uses default resource-based params | +| **3. Index.razor guard** | Excluded at line 984 | Excluded at line 985 | NOT excluded; ResourceId leaks to URL | +| **4. Page defense** | DueOnly check line 643 | DueOnly check at VocabMatching.razor | NO DueOnly param; no fallback to full vocab pool | + +**Mechanism:** +1. Plan builder stamps `ResourceId = resource.Id` for Cloze (line 505) +2. PlanConverter has no Cloze branch, so `BuildRouteParameters` falls through to default (lines 141-148), passes ResourceId unchanged +3. Index.razor `LaunchPlanItem` has no guard for Cloze, so it appends `resourceId={item.ResourceId}` to URL +4. Cloze.razor reads `ResourceIdParam` and passes it to `ClozureSvc.GetSentences(resourceId, 8, skillId)` (line 203) +5. ClozureService HARD REQUIRES resourceId (line 50-54) and returns empty list if missing or if that resource has no vocab +6. If the resource from the plan has no remaining vocab (all mastered, grace period, or empty), Cloze shows "no sentences available" even when hundreds of due words exist globally + +**Discrepancy vs. Hypothesis:** Cloze is NOT vocabulary-driven in the current implementation — it's strictly resource-driven. This is unlike VocabQuiz/VocabMatching which were ALWAYS vocabulary-driven but incorrectly filtered by resource. Cloze genuinely uses the resource's transcript/content to generate contextual sentences via AI. The bug is that TODAY'S PLAN shouldn't restrict Cloze to a single resource when the user has broader vocab needs. + +**Files:** DeterministicPlanBuilder.cs, PlanConverter.cs, Index.razor, Cloze.razor, ClozureService.cs diff --git a/.squad/skills/resource-id-decoupling/SKILL.md b/.squad/skills/resource-id-decoupling/SKILL.md new file mode 100644 index 00000000..67568f89 --- /dev/null +++ b/.squad/skills/resource-id-decoupling/SKILL.md @@ -0,0 +1,209 @@ +# ResourceId Decoupling Pattern for Vocabulary-Driven Activities + +## When to Use This Pattern + +Apply when an activity is **vocabulary-driven** (should load from full user vocab pool based on SRS/due status) but can be incorrectly filtered by LearningResource scope. + +**Trigger symptoms:** +- Activity launched from Today's Plan shows "no words available" error +- Same activity works when launched from resource detail page +- User has many due words globally but activity fails on specific resource +- Insights dashboard shows X due words, but activity claims 0 available + +**Vocabulary-driven activities:** +- VocabularyReview (Vocab Quiz) +- VocabularyGame (Vocab Matching) +- Cloze (when launched from plan — generates contextual sentences for due vocab) + +**Resource-driven activities (DO NOT apply this pattern):** +- Reading, Listening, VideoWatching, Shadowing, Translation, Writing +- These require resource.Transcript, resource.AudioUrl, etc. + +## The 4-Layer Fix + +### Layer 1: DeterministicPlanBuilder.cs + +Set `ResourceId = null` when creating the plan activity. This prevents NEW plans from carrying ResourceId. + +**Example (VocabReview, line 460):** +```csharp +activities.Add(new PlannedActivity +{ + ActivityType = "VocabularyReview", + ResourceId = null, // VocabularyReview is vocabulary-driven, NOT resource-scoped + SkillId = vocabReview.SkillId ?? skill?.Id, + // ... +}); +``` + +**Example (VocabGame, line 520):** +```csharp +activities.Add(new PlannedActivity +{ + ActivityType = "VocabularyGame", + ResourceId = null, // Vocabulary games use skill context + SkillId = skill.Id, + // ... +}); +``` + +**Example (Cloze, line 505):** +```csharp +activities.Add(new PlannedActivity +{ + ActivityType = outputActivity, + ResourceId = outputActivity == "Cloze" ? null : resource.Id, // Cloze is vocabulary-driven when from plan + SkillId = skill?.Id, + // ... +}); +``` + +### Layer 2: PlanConverter.cs + +Add activity-specific handling in `BuildRouteParameters` to: +1. Set `DueOnly = true` (SRS filter) +2. Skip passing ResourceId to route params + +**Example (VocabReview, lines 126-131):** +```csharp +if (activityType == PlanActivityType.VocabularyReview) +{ + parameters["Mode"] = "SRS"; + parameters["DueOnly"] = true; + // VocabularyReview is vocabulary-driven, NOT resource-driven + // ResourceId is intentionally NOT passed to allow quiz to load from full user vocab pool +} +``` + +**Example (VocabGame, lines 133-139):** +```csharp +else if (activityType == PlanActivityType.VocabularyGame) +{ + parameters["DueOnly"] = true; + // VocabularyGame is vocabulary-driven, NOT resource-driven (like VocabularyReview) + // ResourceId is intentionally NOT passed to allow matching to load from full user vocab pool + if (!string.IsNullOrEmpty(skillId)) + parameters["SkillId"] = skillId; +} +``` + +**Example (Cloze, lines 140-147):** +```csharp +else if (activityType == PlanActivityType.Cloze) +{ + parameters["DueOnly"] = true; + // Cloze is vocabulary-driven when launched from plan, NOT resource-driven + // ResourceId is intentionally NOT passed to allow loading from full user vocab pool + if (!string.IsNullOrEmpty(skillId)) + parameters["SkillId"] = skillId; +} +``` + +### Layer 3: Index.razor LaunchPlanItem Guard + +Prevent **persisted old plan items** from leaking ResourceId into the URL. This is defense-in-depth for plans generated before Layer 1 fix. + +**Location:** `src/SentenceStudio.UI/Pages/Index.razor` lines 984-992 + +**Pattern:** +```csharp +// CRITICAL: For vocabulary-driven activities (VocabularyReview, VocabularyGame, Cloze), +// NEVER pass ResourceId (even if persisted on the plan item) +// These activities are vocabulary-driven, NOT resource-driven +if (item.ActivityType != PlanActivityType.VocabularyReview + && item.ActivityType != PlanActivityType.VocabularyGame + && item.ActivityType != PlanActivityType.Cloze + && !string.IsNullOrEmpty(item.ResourceId)) +{ + if (multiResourceRoutes.Contains(route)) + query.Add($"resourceIds={item.ResourceId}"); + else + query.Add($"resourceId={item.ResourceId}"); +} +``` + +**Why this matters:** Even after Layer 1 fix ships, DB already contains persisted DailyPlan rows with the old ResourceId. Index.razor reading `item.ResourceId` directly bypasses PlanConverter entirely. The guard stops the leak at the UI boundary. + +### Layer 4: Page-Level Defense (DueOnly Check) + +Activity page ignores ResourceIds when `DueOnly=true`, even if upstream leak bypassed Layer 3. + +**Example (VocabQuiz.razor, line 643):** +```csharp +// DEFENSE IN DEPTH: When DueOnly=true (SRS mode), ignore ResourceIds even if passed +// This ensures VocabularyReview always loads from full user vocabulary pool +var resourceIds = DueOnly ? Array.Empty() : ParseResourceIds(); +``` + +**Example (VocabMatching.razor, similar pattern):** +```csharp +// DEFENSE IN DEPTH: When DueOnly=true (plan-initiated SRS mode), ignore ResourceIds +// and load from full user vocabulary pool, same pattern as VocabQuiz +var resourceIds = DueOnly ? Array.Empty() : ParseResourceIds(); +``` + +**Example (Cloze.razor + ClozureService):** +```csharp +// In Cloze.razor LoadSentences method: +private async Task LoadSentences() +{ + // DEFENSE IN DEPTH: When DueOnly=true (plan-initiated SRS mode), ignore ResourceId + // and load from full user vocabulary pool, same pattern as VocabQuiz + var resourceId = DueOnly ? "" : (ResourceIdParam ?? ""); + var skillId = SkillIdParam ?? ""; + var result = await ClozureSvc.GetSentences(resourceId, 8, skillId, DueOnly); + // ... +} + +// In ClozureService.cs: +public async Task> GetSentences(string resourceID, int numberOfSentences, string skillID, bool dueOnly = false) +{ + // DEFENSE IN DEPTH: When dueOnly=true (plan-initiated), ignore resourceID and load from full vocab pool + if (dueOnly || string.IsNullOrEmpty(resourceID)) + { + return await GetSentencesFromDueWords(numberOfSentences, skillID); + } + // ... existing resource-driven path +} + +private async Task> GetSentencesFromDueWords(int numberOfSentences, string skillID) +{ + // Load all user vocabulary (same pattern as VocabQuiz) + var allWords = await _resourceRepository.GetAllVocabularyWordsWithResourcesAsync(); + // Filter to due words using SRS logic + var dueWords = allWords.Where(w => { + // Grace period exclusion, NextReviewDate checks, unseen word handling + }).ToList(); + // Generate sentences from due vocab pool + return await GenerateSentencesFromWords(dueWords, numberOfSentences, skillID, targetLanguage); +} +``` + +**Required page changes:** +1. Add query parameter: `[SupplyParameterFromQuery(Name = "DueOnly")] public bool DueOnly { get; set; }` +2. Check DueOnly flag in load method +3. When DueOnly=true: call `GetAllVocabularyWordsWithResourcesAsync()` or `GetDueWordsAsync()` (full user pool) +4. When DueOnly=false: preserve existing resource-filtered behavior (user-initiated path) + +## Testing Checklist + +After applying all 4 layers: + +- [ ] Generate new plan → activity launches successfully from Today's Plan +- [ ] Old persisted plan item → activity still launches successfully (Layer 3 guard prevents leak) +- [ ] Direct resource-filtered launch → still works (DueOnly=false path) +- [ ] Activity loads due words globally when launched from plan +- [ ] Activity respects resource filter when launched from resource detail page +- [ ] No regression for other vocabulary-driven activities + +## References + +- VocabQuiz decoupling: commits 88a0272 (builder+converter), c081a63 (Index guard + page defense) +- VocabMatching decoupling: commit 0c8e197 (all 4 layers) +- Cloze decoupling: commit b3a2937 (all 4 layers, PR #201) +- Decisions log: `.squad/decisions.md` lines 7-16 +- Memory: "DailyPlan items persisted in DB retain their original ResourceId field. UI code that reads item.ResourceId directly (e.g., Index.razor LaunchPlanItem) bypasses PlanConverter and can leak stale resource filters." + +## Anti-Pattern: ResourceId Field Removal + +**DO NOT** remove the ResourceId field from DailyPlanItem schema. Resource-driven activities (Reading, Shadowing, etc.) legitimately use it. The fix is selective filtering, not schema change. diff --git a/docs/coresync-suspected-defects.md b/docs/coresync-suspected-defects.md new file mode 100644 index 00000000..a8af1da6 --- /dev/null +++ b/docs/coresync-suspected-defects.md @@ -0,0 +1,261 @@ +# CoreSync — Suspected Defects (Internal Working Document) + +> **Status: INTERNAL ONLY — DO NOT FILE UPSTREAM YET.** +> +> This document records three suspected defects in the CoreSync library +> (versions 0.1.122 and 0.1.127, decompiled and inspected) that we believe +> together explain the `VocabularyProgress` corruption event observed on +> Captain's Mac Catalyst install (1745 rows where the column NAME was stored +> as the column VALUE for `UserDeclaredAt`, `VerificationState`, and +> `IsUserDeclared`). +> +> Per Captain's call (post-Round 2 troubleshooting), we will **not** open +> upstream issues until each defect below is independently verified by a +> minimal local repro. If/when we do file, this doc becomes the issue body +> draft. +> +> The originator of the corruption is still unknown — none of the three +> defects below alone produce a "column name as value" write. They explain +> *propagation and silent acceptance*, not the original write. A fourth, +> not-yet-located defect (or a fluke `nameof()`-as-value path during a sync +> serialization step) is likely needed to fully close the loop. +> +> Pin: `Directory.Packages.props` lines 98–101 hold CoreSync at `0.1.127`. +> `0.1.128-local` is a metadata-only re-stamp of `0.1.127` (verified +> byte-identical decompiled bodies). + +--- + +## Defect 1 — `GetValueFromRecord` does not handle `Nullable` or `enum` + +**Assemblies / locations** + +- `CoreSync.Sqlite.dll` 0.1.127, `CoreSync.Sqlite.SqliteSyncProvider.GetValueFromRecord(SqliteDataReader, int, Type)` + - Decompiled at `/tmp/coresync-sqlite-decomp/CoreSync.Sqlite.decompiled.cs:939–985` +- `CoreSync.PostgreSQL.dll` 0.1.127, same shape in `CoreSync.PostgreSQL.PostgreSqlSyncProvider.GetValueFromRecord` + - Decompiled at `/tmp/coresync-pg-decomp/CoreSync.PostgreSQL.decompiled.cs:1064` + +**Symptom** + +Reading a sync row where the target CLR property is `DateTime?`, `bool?`, +`int?`, or any `enum` (e.g. `VerificationState`) returns the raw SQLite +value via `r.GetValue(ord)` without coercion. If the cell already contains +a string (because SQLite has dynamic typing and the column has INTEGER or +NUMERIC affinity but a TEXT value was previously stored), the string flows +straight through into the in-memory `SyncItemValue` and is then serialized +to the server as `String`. + +**Code shape (paraphrased from decompile)** + +```csharp +// type dispatch handles only these concrete types: +if (type == typeof(string)) return r.GetString(ord); +if (type == typeof(DateTime)) return r.GetDateTime(ord); +if (type == typeof(int)) return r.GetInt32(ord); +if (type == typeof(bool)) return r.GetBoolean(ord); +// ... byte, char, short, long, decimal, double, float ... +return r.GetValue(ord); // ← Nullable and enum land here +``` + +`Nullable.GetUnderlyingType(type)` is never consulted; `type.IsEnum` is +never consulted. + +**Root-cause hypothesis** + +The dispatch was written against a fixed list of primitive CLR types. The +moment a model uses `DateTime?` (as `VocabularyProgress.UserDeclaredAt` +does) or an `enum` backed by `int` (as `VerificationState` does), the code +relies on whatever the underlying ADO.NET reader returns — which for +SQLite is governed by *cell* type, not *column* type, and can be +`string`. + +**Verification steps (planned, not yet executed)** + +1. Build a minimal CoreSync.Sqlite consumer with a model: + `class M { Guid Id; DateTime? When; MyEnum State; }` +2. Manually `INSERT` a row using a raw SQL string that puts text into + the `When` and `State` columns (e.g. `'foo'`). +3. Trigger a sync read; capture the produced `SyncItem.Values`. +4. Assert that current code returns the string verbatim, and that adding + a `Nullable.GetUnderlyingType` / `IsEnum` branch coerces correctly. + +**Confidence**: **HIGH.** The dispatch code is short, complete, and +visible. The omission is unambiguous. + +**Workaround status** + +- Indirect: PR #185's `RepairTaintedVocabularyProgressAsync` scrubs the + three known affected columns on Captain's local DB at startup, so this + defect cannot manifest on the read path again for those columns. +- No general-case workaround in our codebase. Any future `DateTime?` / + `enum` column in any CoreSync-managed table is theoretically vulnerable + on the read side if a tainted cell ever lands in the local SQLite. + +--- + +## Defect 2 — `ConvertValueForColumn` silently passes unparsed strings to PostgreSQL + +**Assembly / location** + +- `CoreSync.PostgreSQL.dll` 0.1.127, + `CoreSync.PostgreSQL.PostgreSqlSyncProvider.ConvertValueForColumn(object, string)` (or similar — exact name in decompile) +- Decompiled at `/tmp/coresync-pg-decomp/CoreSync.PostgreSQL.decompiled.cs:1576–1591` + +**Symptom** + +When the inbound `SyncItemValue` is a `string` and the target Postgres +column is `timestamp with time zone` (or `timestamp`), the code calls +`DateTime.TryParse(s, out var dt)`. On **success** it uses `dt`. On +**failure** it returns the original string unchanged. Npgsql then rejects +the parameter at the wire level with: + +``` +42804: column "UserDeclaredAt" is of type timestamp with time zone but expression is of type text +``` + +This is exactly the upload exception Captain saw repeatedly on +`POST /api/sync-agent/changes-bulk-complete/{guid}` (stuck batch +`1c18d82b-d160-424d-9ecf-bec51d04ba1e`). + +**Code shape (paraphrased)** + +```csharp +if (value is string s && columnType.StartsWith("timestamp")) +{ + if (DateTime.TryParse(s, out var dt)) return dt; + return value; // ← silently returns the unparsed string +} +``` + +**Root-cause hypothesis** + +Defensive coercion was added for the common case (string-encoded +timestamps from JSON serialization), but the failure branch was written +as "return as-is" rather than "throw a `SynchronizationException` with a +useful diagnostic." This converts a recoverable client-side validation +problem into an opaque DB-level 42804 that retries forever. + +**Verification steps (planned)** + +1. Construct a `SyncItem` whose `Values` contains + `("UserDeclaredAt", SyncItemValueType.String, "UserDeclaredAt")`. +2. POST it to a CoreSync.PostgreSQL endpoint targeting a table with a + `timestamptz` column of that name. +3. Confirm Npgsql 42804 — proves the string reached the parameter. +4. Patch the failure branch to `throw` and re-run; confirm a + `SynchronizationException` arrives at the client with a column- + identifying message before the wire call is made. + +**Confidence**: **HIGH.** The exact 42804 error and the column name "as +value" content match the decompiled behavior precisely. Fall-through +return is visible in the IL. + +**Workaround status** + +- Indirect: PR #185 prevents the tainted strings from ever being uploaded + by repairing them at startup. Pipeline cannot retrigger. +- No general-case workaround. Any future tainted text in any timestamp + column would reproduce the stuck-batch behavior. + +--- + +## Defect 3 — `SyncItemValue.DetectTypeOfObject` is a pure runtime-type switch with no schema check + +**Assembly / location** + +- `CoreSync.dll` 0.1.127, `CoreSync.SyncItemValue.DetectTypeOfObject(object)` +- Decompiled at `/tmp/coresync-decomp/CoreSync.decompiled.cs:780–843` + +**Symptom** + +Once defect 1 demotes a value to `string`, this method classifies it as +`SyncItemValueType.String` with no warning, no provenance check against +the source column's declared type, and no log. The serialized +`SyncItem.Values` payload sent to the server therefore looks completely +well-formed — the server cannot detect the corruption at parse time and +only fails much later at the Npgsql parameter binding step (defect 2). + +**Code shape (paraphrased)** + +```csharp +static SyncItemValueType DetectTypeOfObject(object o) +{ + if (o is null) return SyncItemValueType.Null; + if (o is string) return SyncItemValueType.String; + if (o is DateTime) return SyncItemValueType.DateTime; + if (o is bool) return SyncItemValueType.Boolean; + // ... etc, runtime-type only +} +``` + +There is no parameter for the *declared* column / property type, so the +method has no way to notice "I was asked to classify what should be a +DateTime but I got a string." + +**Root-cause hypothesis** + +The serialization layer trusts the read layer (defect 1) absolutely. The +type-discovery contract is "whatever you hand me, I label." This is a +reasonable design *if* the read layer is sound; combined with defect 1 +it's silent data laundering. + +**Verification steps (planned)** + +1. Construct a `SyncItemValue` from a string handed in where the source + property is `DateTime?` and the source schema column is `INTEGER` (in + our case, the actual SQLite affinity for the `UserDeclaredAt` column + needs to be checked — it accepts text regardless). +2. Serialize → confirm `SyncItemValueType.String` is emitted. +3. Add a schema-aware overload `DetectTypeOfObject(object, Type + declaredType)`; confirm it can throw or warn when actual ≠ declared. + +**Confidence**: **HIGH** that the body is purely runtime-type. **MEDIUM** +that this is the right place to fix; an alternative is to fail-fast in +`GetValueFromRecord` (defect 1) so that this method never sees a +laundered value in the first place. We'd want both belt and suspenders. + +**Workaround status** + +- None in our codebase. PR #185 is purely a downstream cleanup. +- Any fix must live in CoreSync. + +--- + +## Cross-cutting note: missing originator + +None of the three defects above explain the *original write* of +`"UserDeclaredAt"` (the literal column-name string) into the +`UserDeclaredAt` cell. Defects 1+2+3 explain how a tainted string is +read, propagated, and rejected — but something had to put it there +first. + +Hypotheses still open (none yet ruled in): + +- A `nameof(UserDeclaredAt)`-style bug somewhere in CoreSync's + `INSERT … VALUES (@p0, @p1, …)` parameter binding path where parameter + *names* were accidentally bound to *values*. (Not yet found in + decompile.) +- A migration/up-version hook that wrote literal column names as + defaults. (Not present in CoreSync 0.1.122 or 0.1.127.) +- A one-time interaction with a partial schema change on Captain's + machine. (Treat as ghost event unless we see it twice.) + +When we do file upstream, the issue body should lead with this +"originator unknown" framing and ask the maintainer whether they're +aware of any code path that would produce a column-name-as-value write. + +--- + +## Repository pin + +- `Directory.Packages.props:98-101` → CoreSync packages at `0.1.127`. +- `0.1.128-local` exists but is a metadata-only re-stamp; bodies are + byte-identical to `0.1.127`. + +## Captain-side workaround + +- PR #185 (`fix(sync): self-repair tainted VocabularyProgress rows from + CoreSync corruption event`) — adds `RepairTaintedVocabularyProgressAsync` + in `src/SentenceStudio.Shared/Services/SyncService.cs:674` and calls it + from line 522 inside the existing `#if IOS || ANDROID || MACCATALYST` + block. Repair runs on all three mobile platforms uniformly. diff --git a/src/SentenceStudio.AppLib/Services/ClozureService.cs b/src/SentenceStudio.AppLib/Services/ClozureService.cs index 6af7bd53..f8a924f1 100755 --- a/src/SentenceStudio.AppLib/Services/ClozureService.cs +++ b/src/SentenceStudio.AppLib/Services/ClozureService.cs @@ -40,17 +40,17 @@ public ClozureService(IServiceProvider serviceProvider, ILogger _progressService = serviceProvider.GetRequiredService(); } - public async Task> GetSentences(string resourceID, int numberOfSentences, string skillID) + public async Task> GetSentences(string resourceID, int numberOfSentences, string skillID, bool dueOnly = false) { - _logger.LogDebug("GetSentences called with resourceID={ResourceID}, numberOfSentences={NumberOfSentences}, skillID={SkillID}", - resourceID, numberOfSentences, skillID); + _logger.LogDebug("GetSentences called with resourceID={ResourceID}, numberOfSentences={NumberOfSentences}, skillID={SkillID}, dueOnly={DueOnly}", + resourceID, numberOfSentences, skillID, dueOnly); var watch = new Stopwatch(); watch.Start(); - if (string.IsNullOrEmpty(resourceID)) + // DEFENSE IN DEPTH: When dueOnly=true (plan-initiated), ignore resourceID and load from full vocab pool + if (dueOnly || string.IsNullOrEmpty(resourceID)) { - _logger.LogDebug("Resource ID is 0 - no resource selected"); - return new List(); + return await GetSentencesFromDueWords(numberOfSentences, skillID); } if (string.IsNullOrEmpty(skillID)) @@ -96,14 +96,98 @@ public async Task> GetSentences(string resourceID, int numberOfS _words.Count, allVocab.Count - _words.Count); } + return await GenerateSentencesFromWords(_words, numberOfSentences, skillID, resource.Language); + } + + private async Task> GetSentencesFromDueWords(int numberOfSentences, string skillID) + { + _logger.LogDebug("GetSentencesFromDueWords called - loading due vocabulary from full user pool"); + + // Load all user vocabulary (same pattern as VocabQuiz) + var allWords = await _resourceRepository.GetAllVocabularyWordsWithResourcesAsync(); + if (!allWords.Any()) + { + _logger.LogDebug("No vocabulary found in user pool"); + return new List(); + } + + // Get progress for filtering + var wordIds = allWords.Select(w => w.Id).ToList(); + var progressDict = await _progressService.GetProgressForWordsAsync(wordIds); + + // Filter to due words (same logic as VocabQuiz) + var now = DateTime.Now; + var dueWords = allWords.Where(w => + { + if (!progressDict.TryGetValue(w.Id, out var progress)) + return true; // Unseen words are due + + if (progress.IsInGracePeriod) + return false; // Skip grace period words + + var totalAttempts = progress.TotalAttempts; + var nextReview = progress.NextReviewDate; + + // Include unseen words (never attempted) + if (totalAttempts == 0) + return true; + + // Include words due for review + if (nextReview.HasValue && nextReview.Value <= now) + return true; + + return false; + }).ToList(); + + _logger.LogDebug("Found {DueCount} due words out of {TotalCount} total words", dueWords.Count, allWords.Count); + + if (!dueWords.Any()) + { + _logger.LogDebug("No due words available"); + return new List(); + } + + // Cap vocab sample (same pattern as resource-driven path) + const int MaxVocabSample = 40; + if (dueWords.Count > MaxVocabSample) + { + _words = dueWords + .OrderBy(_ => Random.Shared.Next()) + .Take(MaxVocabSample) + .ToList(); + _logger.LogDebug("Sampled {SampleCount} of {DueCount} due vocabulary words for AI prompt", + _words.Count, dueWords.Count); + } + else + { + _words = dueWords; + _logger.LogDebug("Using all {WordCount} due vocabulary words for AI prompt", _words.Count); + } + + // Get user profile for target language + var userProfileRepo = _serviceProvider.GetRequiredService(); + var userProfile = await userProfileRepo.GetAsync(); + string targetLanguage = userProfile?.TargetLanguage ?? "Korean"; + + return await GenerateSentencesFromWords(_words, numberOfSentences, skillID, targetLanguage); + } + + private async Task> GenerateSentencesFromWords(List words, int numberOfSentences, string skillID, string? targetLanguage = null) + { + if (string.IsNullOrEmpty(skillID)) + { + _logger.LogDebug("Skill ID is empty - no skill selected"); + return new List(); + } + var skillProfile = await _skillRepository.GetSkillProfileAsync(skillID); _logger.LogDebug("Skill profile retrieved: {SkillTitle}", skillProfile?.Title ?? "null"); - // Get user's native language and use resource's language as target + // Get user's native language and use provided target language or fall back to user profile var userProfileRepo = _serviceProvider.GetRequiredService(); var userProfile = await userProfileRepo.GetAsync(); string nativeLanguage = userProfile?.NativeLanguage ?? "English"; - string targetLanguage = resource.Language ?? userProfile?.TargetLanguage ?? "Korean"; + targetLanguage = targetLanguage ?? userProfile?.TargetLanguage ?? "Korean"; var prompt = string.Empty; using Stream templateStream = await _fileSystem.OpenAppPackageFileAsync("GetClozuresV2.scriban-txt"); @@ -111,7 +195,7 @@ public async Task> GetSentences(string resourceID, int numberOfS { var template = Template.Parse(await reader.ReadToEndAsync()); prompt = await template.RenderAsync(new { - terms = _words, + terms = words, number_of_sentences = numberOfSentences, skills = skillProfile?.Description, native_language = nativeLanguage, @@ -186,7 +270,7 @@ public async Task> GetSentences(string resourceID, int numberOfS CreatedAt = DateTime.UtcNow, UpdatedAt = DateTime.UtcNow }; // Try to find matching vocabulary word for progress tracking - var matchingWord = _words.FirstOrDefault(w => + var matchingWord = words.FirstOrDefault(w => string.Equals(w.TargetLanguageTerm, clozureDto.VocabularyWord, StringComparison.OrdinalIgnoreCase)); if (matchingWord != null) diff --git a/src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs b/src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs index 2f761297..b4604927 100644 --- a/src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs +++ b/src/SentenceStudio.Shared/Services/PlanGeneration/DeterministicPlanBuilder.cs @@ -502,7 +502,7 @@ private async Task> BuildActivitySequenceAsync( activities.Add(new PlannedActivity { ActivityType = outputActivity, - ResourceId = resource.Id, + ResourceId = outputActivity == "Cloze" ? null : resource.Id, // Cloze is vocabulary-driven when from plan SkillId = skill?.Id, EstimatedMinutes = outputMinutes, Priority = priority++, diff --git a/src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs b/src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs index f4c4a59d..1a31e4f8 100644 --- a/src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs +++ b/src/SentenceStudio.Shared/Services/PlanGeneration/PlanConverter.cs @@ -137,6 +137,14 @@ public static Dictionary BuildRouteParameters(PlanActivityType a if (!string.IsNullOrEmpty(skillId)) parameters["SkillId"] = skillId; } + else if (activityType == PlanActivityType.Cloze) + { + parameters["DueOnly"] = true; + // Cloze is vocabulary-driven when launched from plan, NOT resource-driven + // ResourceId is intentionally NOT passed to allow loading from full user vocab pool + if (!string.IsNullOrEmpty(skillId)) + parameters["SkillId"] = skillId; + } else { // Resource-based activities diff --git a/src/SentenceStudio.UI/Pages/Cloze.razor b/src/SentenceStudio.UI/Pages/Cloze.razor index 9f126df8..83a59425 100644 --- a/src/SentenceStudio.UI/Pages/Cloze.razor +++ b/src/SentenceStudio.UI/Pages/Cloze.razor @@ -152,6 +152,9 @@ else [SupplyParameterFromQuery(Name = "planItemId")] public string? PlanItemId { get; set; } + [SupplyParameterFromQuery(Name = "DueOnly")] + public bool DueOnly { get; set; } + [Inject] private ClozureService ClozureSvc { get; set; } = default!; [Inject] private UserActivityRepository ActivityRepo { get; set; } = default!; [Inject] private VocabularyProgressService ProgressService { get; set; } = default!; @@ -197,10 +200,12 @@ else isBusy = true; try { - var resourceId = ResourceIdParam ?? ""; + // DEFENSE IN DEPTH: When DueOnly=true (plan-initiated SRS mode), ignore ResourceId + // and load from full user vocabulary pool, same pattern as VocabQuiz + var resourceId = DueOnly ? "" : (ResourceIdParam ?? ""); var skillId = SkillIdParam ?? ""; - var result = await ClozureSvc.GetSentences(resourceId, 8, skillId); + var result = await ClozureSvc.GetSentences(resourceId, 8, skillId, DueOnly); if (result?.Any() == true) { sentences = result; diff --git a/src/SentenceStudio.UI/Pages/Index.razor b/src/SentenceStudio.UI/Pages/Index.razor index d53f119d..bb837094 100644 --- a/src/SentenceStudio.UI/Pages/Index.razor +++ b/src/SentenceStudio.UI/Pages/Index.razor @@ -983,6 +983,7 @@ else // These activities are vocabulary-driven, NOT resource-driven if (item.ActivityType != PlanActivityType.VocabularyReview && item.ActivityType != PlanActivityType.VocabularyGame + && item.ActivityType != PlanActivityType.Cloze && !string.IsNullOrEmpty(item.ResourceId)) { if (multiResourceRoutes.Contains(route))