Skip to content

fix(atoms): keep learning-harvest entries short when the file has drifted - #54

Open
IdoGvili wants to merge 1 commit into
techygarg:mainfrom
IdoGvili:fix/learning-harvest-entry-drift
Open

IdoGvili wants to merge 1 commit into
techygarg:mainfrom
IdoGvili:fix/learning-harvest-entry-drift

Conversation

@IdoGvili

@IdoGvili IdoGvili commented Oct 22, 2026 •

Copy link
Copy Markdown

What does this PR do?

Keeps Harvest's new entries and recurrence notes within the two-line entry format when the existing learnings file already holds longer entries, which the agent otherwise copies.

Type of change

  • New atom
  • New molecule
  • New refiner
  • Improvement to existing skill
  • Documentation fix
  • Other: ___

Breaking change?

  • Yes — this modifies an existing skill's section structure, removes a check, or changes behavior that existing users depend on
  • No — no section, step or check is added or removed. A recurrence note is now a date, (recurred YYYY-MM-DD), where the skill said "recurrence note" without a format.

Skill checklist

New or modified atom

  • Frontmatter: name is lowercase kebab-case and matches the folder name (unchanged)
  • Frontmatter: description includes specific trigger phrases (unchanged)
  • Section order: unchanged (learning-harvest is a special atom with its own order)
  • Self-Validation Checklist uses numbered items with STOP language and BOLD LABEL format (item 6 reworded only)
  • Active Anti-Pattern Scan: not applicable, special atom
  • references/defaults.md is intentionally absent (special atom)
  • Ambiguity Signals: not applicable, special atom
  • Enforces exactly one principle — not bundled concerns

Changes, all in source/atoms/learning-harvest/SKILL.md (and the regenerated skills/ copy):

  • Entry format: adds **STOP: keep to two lines even when existing entries are longer — do not copy their length or shape.**
  • Harvest step 5: a matching entry gets (recurred YYYY-MM-DD), then **STOP: add the date only — if the takeaway changed, reword the entry within two lines.**
  • Harvest step 6 and checklist item 6: count and add recurrence dates.

Testing

Tool used: Claude Code
Model: Claude Opus 5.5 (one subagent per run, reading the skill file directly)
Invoked against: one fake code-forge session on the sample/ User Service (login with account lockout), harvested into three different existing learnings files. The simulated user accepted every candidate. 3 runs per row.
What worked correctly:

Existing learnings file Skill Median entry (chars) Entries over 250 chars Recurrence note (chars)
5 short entries main 220 1 / 12 21–46
50 long entries (fixture below) main 290 7 / 10 116–203
50 long entries (fixture below) this PR 208 0 / 11 21
a real team file (155 entries, 95 KB) main → this PR 297 → 227 9 / 11 → 2 / 10 111–148 → 21

On main, a long file pulls new entries past two lines, and recurrence notes copy the file's own (Recurred …: story) shape. With this PR, every recurrence note is the date only, and new entries stay close to two lines.

Edge cases tested: the session plants four situations, and every row handled them the same way: a repeat of an existing entry (always a recurrence note, never a new entry), a real bug discussed in session ids such as D4 and PR 2 (no id reached an entry), a design decision with no outcome (never proposed), and a library gotcha that broke a test (always captured). Not tested: a missing .lattice/config.yaml, which this change does not touch.

Documentation

  • Updated docs/how-it-works.md skill inventory table (if adding a new skill)
  • No documentation changes needed — no doc describes entry length or the recurrence note.

Anything else?

This is not the size limit from #37: it adds no limit or gate. It makes the existing two-line format and recurrence note hold in a file that has already drifted, which team practice alone cannot do, because every harvest re-reads the file and copies its style.

Fixture: session record
# Session record — code-forge, User Service capability 2 (authentication with account lockout)

This is the record of the session you just finished, in order. The design was approved in an earlier session; its context doc is `.lattice/context/user-authentication.md`. Decisions there are numbered D1–D6 and the work is cut into slices S1–S3, shipped as PR 1–PR 3.

1. **Loaded context.** Read the context doc. Relevant decisions: D2 (one generic failure, "invalid email or password", whether the email exists or not), D4 (the failed-attempt counter and lockout expiry live on the `User` aggregate, not in a separate table), D5 (lockout is 5 consecutive failures, unlock after 30 minutes, per the spec).

2. **S1 — domain (PR 1).** Added `User.RecordFailedLogin(now)` and `User.RecordSuccessfulLogin(now)`. Unit tests for the 5th-failure lockout and the 30-minute unlock pass. The user asked whether the 30 minutes should be a configuration setting. We kept it as a domain constant: the spec fixes the value, and a setting would need validation, a default and docs for a value nobody has asked to change. Recorded this as D6 in the context doc. Nothing else came of it.

3. **S2 — persistence (PR 2).** Added `FailedLoginCount` and `LockedUntil` to the `Users` table. Ran `dotnet ef migrations add AddLockoutFields`. Reviewing the generated migration, it also changed `Users.PhoneNumber` from `varchar(20)` to `varchar(16)` — nothing in this slice touches phone numbers. Cause: we had merged main (which carried a teammate's phone-number change and its snapshot) after the migration was first generated on an older branch state, and then re-ran `migrations add` on top of the old migration instead of removing it first. Removed the migration, confirmed the snapshot matched main, regenerated. The clean migration only adds the two columns. The user said: "this is the same snapshot thing we hit in September."

4. **S2 — integration test (PR 2).** Wrote `Login_LocksAccount_AfterFifthConcurrentFailure`: fires 5 failed logins for one user in parallel against the PostgreSQL test container. It failed — the account was not locked; `FailedLoginCount` was 3 or 4 depending on the run. Cause: each request loaded the `User`, incremented the counter in memory and saved; with no concurrency token, EF Core wrote "last one wins", so concurrent increments were lost and the lockout under D4 fired late or not at all. Fix: added a concurrency token to `User` (PostgreSQL `xmin`) and a single retry on `DbUpdateConcurrencyException` in the login handler. The test now passes 50 runs in a row. The user said this is worth remembering for any counter we keep on an aggregate.

5. **S3 — API (PR 3).** Added `POST /auth/login`. The API test for the success case failed: deserializing the response with `JsonSerializer.Deserialize<LoginResult>(body)` gave `Success = false` and `UserId = Guid.Empty` even with the right password. Cause: `System.Text.Json` matches property names case-sensitively by default, while ASP.NET Core writes camelCase JSON, so every property stayed at its default. The failure-path tests had passed only because their expected values were the defaults (`Success = false`). Fixed by switching the tests to `ReadFromJsonAsync<LoginResult>()`, which uses the web defaults. Re-checked the failure-path tests: they now really read the body.

6. **Small things.** Fixed a typo in the lockout error message ("acount"). Reused the registration email-format validator for the login request.

7. **Closing.** All three slices are done, tests green, context doc status set to `complete`. code-forge now runs its closing Harvest.
Fixture: short learnings file (5 entries)
# Operational Learnings

Experiential patterns from practice. Complements standards (what should be) with experience (what we keep learning).

## Design Patterns
<!-- Decomposition, architecture choices, scope decisions that proved good or bad -->
- 2026-08-14 [design] Value objects designed before the aggregate kept their validation out of the service layer; the ones added later all leaked into handlers — design VOs first

## Implementation Craft
<!-- Coding approaches, library gotchas, design-to-reality gaps -->
- 2026-09-02 [implementation] An EF Core migration added from a stale model snapshot carried unrelated column changes — regenerate it from a snapshot that matches main before committing
- 2026-09-10 [implementation] FluentValidation rules registered per-assembly ran twice when a validator was also registered by hand — register validators one way only

## Quality Signals
<!-- Recurring quality issues that keep appearing despite rules -->
- 2026-08-20 [review] Tests named "should work" kept hiding which branch they covered — name the condition and the outcome

## Reliability
<!-- Bug root causes, failure modes, fragile areas, boundary condition gaps -->
- 2026-09-05 [bug fix] Timestamps compared across DateTime.Now and DateTime.UtcNow drifted by the UTC offset in CI only — store and compare UTC everywhere

## Structural Health
<!-- Architectural drift, debt accumulation, coupling issues, migration lessons -->
Fixture: long learnings file (50 entries, 28 KB)
# Operational Learnings

Experiential patterns from practice. Complements standards (what should be) with experience (what we keep learning).

## Design Patterns
<!-- Decomposition, architecture choices, scope decisions that proved good or bad -->
- 2026-06-04 [design/api-seam] **Splitting the booking API by HTTP verb instead of by use case doubled the number of seams.** Every GET went into BookingQueriesController and every write into BookingCommandsController, and within six weeks the reschedule flow touched 3 controllers, 2 services and 11 DTOs. The obvious fix, a shared BookingFacade, only moved the sprawl into one 900-line class. Grouping endpoints by use case (Reschedule, Cancel, Confirm) cut the files touched per change from 7 to 2 on average over the next sprint. Discriminator: an API whose changes arrive as user flows rather than as table-level CRUD.
- 2026-06-11 [design/aggregate-boundary] **The Invoice aggregate that owned its PaymentAttempts loaded the whole payment history on every webhook.** A tenant with 40 retries per invoice made each webhook load and save 40 attempts plus their gateway responses, about 1.2 MB per call, and webhook p95 rose to 900 ms. Making PaymentAttempt its own aggregate that references InvoiceId by value cut that to 60 ms; the rule "no capture after void" moved into a domain check at capture time. Lazy-loading the attempts would have hidden the cost, not removed it. Discriminator: a child collection that grows without bound and is written by a different actor than its parent.
- 2026-06-23 [design/read-models] **Serving the tenant dashboard from the write model cost a month of index tuning that a read table would have avoided.** The dashboard joined Bookings, Invoices and Staff under five filters, and EXPLAIN showed a 2.3 s nested loop at p95 once a tenant passed 200k bookings. Each new index fixed one filter and slowed inserts by about 6%. A denormalized dashboard_summary table, refreshed by the outbox consumer, served the same page in 40 ms. Discriminator: a read screen whose filters cross three or more aggregates and can tolerate seconds of lag. (Recurred 2026-09-03: the staff utilization report went the same way — four indexes in two weeks before anyone asked whether it needed live data; it did not, and a nightly summary table replaced all four.)
- 2026-07-02 [design/scope-cut] Shipping recurring bookings without the "edit this and following" mode was the right cut. Product asked for three edit modes; we shipped "this one" and "whole series" and logged attempts. In eight weeks only 3% of series edits came from users splitting a series by hand, which did not pay for what the third mode needs: a parent/override table and a merge step on every calendar read. Discriminator: an edit mode that changes the storage model for every reader, not only the writer.
- 2026-07-09 [design/outbox] **Publishing integration events straight from the request handler lost messages whenever SQS throttled.** The handler committed the transaction and then called SendMessageAsync; in a burst of 1,200 bookings a minute about 0.4% of sends failed after the commit, and the notifications worker never heard of those bookings. Retrying inside the handler only stretched request latency. An outbox table written in the same transaction and drained by a hosted service brought the loss to zero for the rest of the quarter. Discriminator: any side effect outside the database that must happen if and only if the commit does. (Recurred 2026-08-27: the audit writer did the same with S3 puts after commit; a burst of 503s left 212 bookings with no audit record. It now writes through the same outbox.) (Recurred 2026-10-01: a new "booking created" email was sent from the controller after SaveChangesAsync; a deploy restart between the two lines dropped 30 emails. It moved behind the outbox as well.)
- 2026-07-21 [design/tenancy] **Tenant isolation as a global query filter beat passing tenantId through every repository method.** The old pattern had 140 call sites that took a tenantId parameter, and a code search found two passing the wrong variable. HasQueryFilter on every tenant-owned entity, fed by a scoped ITenantContext, removed the parameter everywhere. The catch is that IgnoreQueryFilters() in admin tools also drops the soft-delete filter, so each use now needs a second reviewer. Discriminator: a multi-tenant schema where every row carries a tenant column and cross-tenant reads are rare and admin-only.
- 2026-08-05 [design/frontend-state] Putting server data into the global React store turned every list into a cache-invalidation problem. The booking list, the calendar and the sidebar badge each held their own copy, and a cancel updated one of the three. Moving server state into a per-query cache keyed by endpoint and parameters, and keeping the global store for UI state only, deleted about 600 lines of reducers and the whole "badge says 4, list says 3" class of bug.
- 2026-08-26 [design/background-jobs] **One jobs table for every background task made priorities impossible.** Invoice generation, email sends and nightly exports shared one queue and one worker, so a 90-minute export delayed password resets for every tenant. A priority column, tried first, failed: the only worker slot was still on the long job. One queue per latency class (interactive, batch, nightly), each with its own workers, fixed it. Discriminator: background work whose latency needs differ by 10x or more.
- 2026-09-17 [design/api-versioning] **URL-segment versioning kept old mobile clients alive only once we stopped sharing DTOs between versions.** v1 and v2 first returned the same response classes, and a nullable field added for v2 broke a v1 client that rejected unknown properties. Copying the DTOs into V1 and V2 namespaces with explicit mapping looked like duplication but stopped the leaks between versions. Discriminator: a public API with clients you cannot force to upgrade. (Recurred 2026-10-08: a shared enum got a new member for v2 and v1 clients crashed on the unknown value; enums are now copied per version too.)
- 2026-08-14 [design] Value objects designed before the aggregate kept their validation out of the service layer; the ones added later all leaked into handlers — design VOs first

## Implementation Craft
<!-- Coding approaches, library gotchas, design-to-reality gaps -->
- 2026-06-08 [implementation/ef-tracking] **AsNoTracking on the export query cut memory by 70%, and then Update() on the same rows rewrote every column.** The invoice export read 50k rows untracked, and a later "mark exported" step attached them with Update(), which marks every property as modified. Each row rewrote 31 columns, and the audit trigger logged 31 changed fields per row — 1.5M audit rows for one export. ExecuteUpdateAsync with one SET on exported_at took the step from 4 minutes to 6 seconds. Discriminator: a bulk status change on rows you only loaded for another purpose.
- 2026-06-19 [implementation/raw-sql-in-lists] **A hand-built IN list created a new query plan for every list size.** Our raw SQL helper rendered IN (@p0, @p1, ...) for a List<Guid>, so pg_stat_statements held 3,400 distinct statements for one query. EF Core on Npgsql already sends Contains as = ANY(@p) with one array parameter; switching the helper to the same shape cut the statement count to 1 and p95 from 220 ms to 35 ms. Discriminator: hand-written SQL with variable-length IN lists.
- 2026-07-01 [implementation/httpclient] **A new HttpClient per call in the SMS sender exhausted sockets at about 300 sends a minute.** netstat on the worker showed 4,000 sockets in TIME_WAIT, and SendAsync began failing with "address already in use". A static client fixed the sockets but then ignored DNS changes when the provider rotated its endpoint. IHttpClientFactory with a typed client and a 5-minute handler lifetime fixed both. Discriminator: any outbound HTTP call in a long-running process. (Recurred 2026-08-08: the new webhook relay in the integrations worker built its own HttpClient inside a using block; the same TIME_WAIT pile-up came back after a 2,000-event replay, and it moved to a typed client.)
- 2026-07-15 [implementation/async] **A .Result inside a sync-over-async adapter deadlocked only in the test run, never in production.** The PDF renderer exposed a sync interface, so the adapter called RenderAsync(...).Result. Kestrel has no synchronization context, so production was fine; the test project set maxParallelThreads to 1, the runner installed its own context, and the job hung for the full 10-minute timeout. Making the interface async end to end touched 14 files, less than the two days already lost to it. Discriminator: any blocking wait on a Task in code a test host may run on a captured context.
- 2026-07-28 [implementation/react-effects] A useEffect that fetched on every filter change raced itself, and the slowest response won. Typing "smi" into staff search fired three requests; the one for "s" came back last (900 ms against 120 ms) and replaced the right results. Debouncing lowered the rate but kept the race. An AbortController per effect, aborted in the cleanup function, means only the latest request can set state.
- 2026-08-12 [implementation/logging] **Interpolated strings in log calls turned one message template into 80,000.** logger.LogInformation($"Booking {id} confirmed") looks like the template form in review, but the log store indexed each rendered string as a new template and the monthly ingestion bill rose 2.5x. Analyzer rule CA2254 was off in the shared ruleset; making it an error found 312 call sites, and the fix was mechanical. Discriminator: any ILogger call in a codebase that ships logs to an indexed store.
- 2026-08-29 [implementation/ci-cache] **Caching ~/.nuget/packages keyed on the OS alone restored a stale package set after every dependency change.** The restore step reported a cache hit and then downloaded 600 MB anyway, adding 3 minutes per job. Keying on hashFiles('**/packages.lock.json') fixed it, but only once RestorePackagesWithLockFile was on — without lock files there was nothing stable to hash. Discriminator: a CI dependency cache whose key does not include the resolved dependency set.
- 2026-09-24 [implementation/linq-translation] **IsBillable(b) inside a Where clause threw in one query and pulled 90k rows into memory in another.** In the booking list EF Core refused to translate it; in the invoice preview someone had added .ToList() before the Where "to fix it", so 90k bookings were filtered in memory in 11 s. One Expression<Func<Booking, bool>> for the rule kept a single definition and let both queries run in SQL. Discriminator: a domain rule reused inside database LINQ queries.
- 2026-09-02 [implementation] An EF Core migration added from a stale model snapshot carried unrelated column changes — regenerate it from a snapshot that matches main before committing
- 2026-09-10 [implementation] FluentValidation rules registered per-assembly ran twice when a validator was also registered by hand — register validators one way only

## Quality Signals
<!-- Recurring quality issues that keep appearing despite rules -->
- 2026-06-10 [tests/fixture-reuse] **A shared database fixture made 400 integration tests fast and 30 of them order-dependent.** One Postgres container per run cut the suite from 14 minutes to 3, but tests that created staff with fixed emails failed whenever a sibling had already used that email. Resetting the schema per test brought the 14 minutes back. A transaction per test, rolled back on dispose, kept the speed and removed the leaks; the 6 tests that check commit behaviour get their own database. Discriminator: integration tests that share one database across a collection. (Recurred 2026-08-19: the new tenant onboarding tests used a fixed slug "demo-tenant" and failed only in parallel runs; a unique slug per test fixed them.)
- 2026-06-25 [review/pr-size] **PRs over 600 changed lines were approved faster than PRs under 200, and carried most of the escaped defects.** Over three months, median time to approve was 2.1 h for big PRs and 5.4 h for small ones, and 9 of 13 production bugs traced back to big ones. Reviewers approved what they could not hold in their heads. A size label in the template changed nothing; vertical slices, with the schema change in its own PR first, did. Discriminator: large diffs approved with no comments.
- 2026-07-07 [tests/mocked-dbcontext] **Tests that mocked DbSet<T> passed while the real query threw on translation.** A mocked set runs LINQ to Objects, so a LastOrDefault() with no OrderBy passed in 40 unit tests and threw on Postgres the first time a user opened the history tab. Moving the query tests onto a throwaway Postgres container added 25 s to the run and caught 7 more untranslatable queries in the first week. Discriminator: tests of code whose behaviour depends on how a query is translated, not only on what it returns. (Recurred 2026-09-25: a new search endpoint used EF.Functions.ILike, which a mocked set cannot run at all; the author wrapped it in a try/catch "for tests", and the catch shipped.)
- 2026-07-24 [review/nullable] Turning on nullable reference types one project at a time made the project boundaries the place where nulls got in. Domain had zero nullable warnings, but the mappers in the still-oblivious API project fed it values with null!, and there were 210 of those after three months, each a silenced warning. A CI step that counts null! and fails on any increase stopped the growth; it did not remove the existing ones.
- 2026-08-04 [review/catch-all] **catch (Exception) blocks kept returning 200 with an empty list.** Four endpoints swallowed exceptions and returned Ok(new List<T>()), so a broken staff search looked like "no results" for two days and support closed 17 tickets as user error. The analyzer rule for general catches was a warning in a build with 1,100 warnings. Making that one rule an error in the API project found 23 more. Discriminator: a catch-all in a request path that still returns a success status.
- 2026-08-18 [tests/snapshots] **Snapshot tests of whole API responses failed on every unrelated field, so people approved new snapshots without reading them.** In two months 61 of 64 snapshot updates were committed with no reviewer comment, and one of them locked in a wrong invoice total. Narrowing each snapshot to the fields its test is about, and asserting the rest elsewhere, cut snapshot churn by about 80%. Discriminator: snapshot or approval tests over payloads that many people add fields to.
- 2026-09-01 [tests/flaky-retries] **Retrying flaky tests in CI hid a real race for five weeks.** The booking-confirm tests failed about 1 run in 20, so the integration job got a 3x retry. The failures were real: the confirm handler published its event before SaveChangesAsync, and the consumer sometimes read the old state. The retry turned a 5% CI failure into a production bug. A flaky test now goes into a quarantine job with an owner, never behind a retry. Discriminator: a test that fails intermittently on the same assertion. (Recurred 2026-09-30: an end-to-end test on the invoice drawer was set to retry twice; the "flake" was a double-click that submitted two payments in the real app.)
- 2026-09-12 [review/generated-code] **The generated TypeScript API client was committed and reviewed by hand, so reviewers read 3,000-line diffs nobody wrote.** Inside that noise, a real API change that removed a field the mobile app still read was approved unseen. Generating the client in CI from the OpenAPI document, failing the build when the committed copy differs, and reviewing only the OpenAPI diff moved attention to the 40 lines that matter. Discriminator: generated code committed beside hand-written code.
- 2026-10-03 [tests/log-assertions] Tests that asserted on rendered log messages to prove a branch ran broke 50 at a time whenever someone reworded a message. The wording change was never a bug, and after the third mass update people stopped reading the failures. Asserting on the observable result instead — the row written, the event published, the status code — kept every one of those tests meaningful, and none of them failed on the next two log rewrites.
- 2026-08-20 [review] Tests named "should work" kept hiding which branch they covered — name the condition and the outcome

## Reliability
<!-- Bug root causes, failure modes, fragile areas, boundary condition gaps -->
- 2026-06-02 [reliability/timeouts] **The payment provider call had no explicit timeout, so one slow upstream held request threads for 100 s each.** HttpClient's default Timeout is 100 seconds; during the provider's partial outage checkout p99 hit 101 s, the thread pool grew slowly, health checks timed out and ECS replaced healthy tasks. A 5 s per-attempt timeout with one retry, plus a circuit breaker that opens at 50% failures over 30 s, kept the rest of the API serving. Discriminator: any synchronous third-party call inside a user request. (Recurred 2026-08-30: the address lookup had the same default; a 40-minute slowdown there blocked booking creation, because the lookup ran before validation.)
- 2026-06-17 [reliability/idempotency] **Webhook handlers that were not idempotent double-charged 14 customers when the provider redelivered.** The provider retries any delivery not acknowledged within 10 s, and our handler took 12 s at peak, so it processed, timed out from the provider's side, and processed again. Returning 202 fast and handling from a queue reduced redeliveries but did not stop them. A unique index on (provider, event_id), inserted before any processing, made redelivery a no-op. Discriminator: any inbound webhook from a provider that promises at-least-once delivery.
- 2026-07-03 [reliability/pagination] **Offset pagination over a growing table skipped and duplicated rows.** The nightly invoice export paged with OFFSET/LIMIT by created_at; invoices inserted during the 40-minute run shifted the pages, and accounting found 31 duplicates and 12 missing invoices in one month. Keyset pagination on (created_at, id) fixed it, and the id tiebreaker mattered — 2,000 invoices from one bulk import shared a created_at. Discriminator: paging a table that is written while you read it.
- 2026-07-16 [reliability/connection-pool] **The reminder worker held one Postgres connection per tenant loop and starved the API at peak.** It opened one DbContext for the whole loop over 450 tenants and kept the connection for 9 minutes; with three replicas the API began failing with "sorry, too many clients already". Raising max_connections on RDS was the obvious fix and would have eaten memory on the small instance class. A scope per tenant from IServiceScopeFactory released the connection between tenants, and the pool has not gone above 30 since. Discriminator: a long-running loop in a singleton host that uses a scoped DbContext. (Recurred 2026-09-09: the invoice reminder job copied the old worker's loop; peak connections reached 96 before the alarm at 90 fired.) (Recurred 2026-10-11: a one-off backfill console app did it again with a single context for 1.2M rows; it also grew the change tracker to 3 GB before the task was killed.)
- 2026-07-30 [reliability/uploads] **Buffering an uploaded CSV in a MemoryStream restarted the import container on a 600 MB file.** The endpoint called CopyToAsync into a MemoryStream before parsing; the task hit its 1 GB limit, and ECS restarted it four times as the client retried. Streaming the body straight to S3 and parsing it in the worker kept memory flat at about 180 MB. A size limit alone would have rejected files the business wants. Discriminator: any endpoint that takes user files with no hard size cap.
- 2026-08-13 [reliability/cancellation] Ignoring the request CancellationToken kept 20-second reports running after users left the page. The report endpoint did not pass HttpContext.RequestAborted to ToListAsync, so a user who clicked "Run" five times left five queries running; at month-end that was 70 report queries at once and 95% database CPU. Passing the token lets Npgsql cancel the query on the server when the client goes away. Discriminator: an expensive query users can re-trigger.
- 2026-08-28 [reliability/sqs-visibility] **An SQS visibility timeout shorter than the handler's worst case ran the same message on two workers.** The invoice PDF handler usually took 8 s but up to 75 s for invoices with 2,000 lines; with a 30 s visibility timeout the message reappeared, and a second worker rendered and emailed it again. Raising the timeout to 5 minutes fixed the duplicates but delayed retries of real failures by 5 minutes. Extending visibility from the handler every 20 s while it works gave both. Discriminator: queue handlers whose run time has a long tail. (Recurred 2026-10-02: the export worker moved to a new queue created from a template with the 30 s default; 9 exports ran twice before the duplicate-S3-key alarm fired.)
- 2026-09-11 [reliability/readiness] **The readiness check proved only that the process was up, so new tasks took traffic before their caches were warm.** Every deploy produced a 2-minute burst of 5xx, because the first requests to each new ECS task loaded the 40 MB pricing catalogue on demand and timed out at 10 s. A readiness endpoint that returns 503 until a warm-up hosted service finishes removed the burst. Liveness stayed shallow on purpose: a deep liveness check would restart tasks whenever the database was slow. Discriminator: services with expensive first-request initialization.
- 2026-09-29 [reliability/money-rounding] Invoice totals summed in the browser as JavaScript numbers drifted a cent from the server's decimal totals. About 1 invoice in 300 showed a total 0.01 off the PDF, and finance flagged 41 mismatches in a month. Rounding in the component hid most cases and created new ones at .005. The API now returns every total already computed, in minor units, and the frontend only formats them for display.
- 2026-09-05 [bug fix] Timestamps compared across DateTime.Now and DateTime.UtcNow drifted by the UTC offset in CI only — store and compare UTC everywhere

## Structural Health
<!-- Architectural drift, debt accumulation, coupling issues, migration lessons -->
- 2026-06-20 [structure/module-boundaries] **Module boundaries in the monolith held only after cross-module calls had to go through a public contracts project.** Before that, Billing reached into Scheduling's internal DbContext in 37 places, and a column rename in Scheduling broke two Billing reports in production. An architecture test — types in Billing may reference only Scheduling.Contracts — failed the build on day one with 37 violations, which we fixed over four sprints. Discriminator: a monolith split into modules that share one database. (Recurred 2026-09-15: a new Notifications module was added without an entry in that test and had taken 9 direct references to Billing internals before anyone noticed; the test now discovers modules by folder instead of a hard-coded list.)
- 2026-07-08 [structure/mediator] **Routing every call through a mediator made "find usages" useless for tracing a request.** A typical endpoint went controller, mediator send, two pipeline behaviours, handler, and new joiners took about 40 minutes to trace one request in the onboarding exercise. For the 60% of handlers with one caller and no cross-cutting behaviour, calling the service directly cut the hops from 5 to 2. The behaviours that earned their place, transactions and authorization, stayed. Discriminator: an indirection layer whose main users are single-caller handlers.
- 2026-07-22 [structure/generic-repository] **A generic repository over EF Core hid IQueryable, then grew 40 special-case methods to get it back.** IRepository<T> exposed GetById, GetAll and Find(predicate); each report needing a projection or an Include added a method, and BookingRepository reached 1,300 lines. Querying the DbContext directly in read handlers, with repositories kept for loading aggregates, cut it to 180 lines. Discriminator: a repository over an ORM that already is one.
- 2026-08-06 [structure/frontend-coupling] Frontend components used the generated API types directly, so every backend DTO rename touched about 60 React files. Renaming staffMember to staff in one response produced a 61-file frontend PR. One adapter per feature folder that maps API types to view models meant the next rename touched 3 files. The adapters looked like ceremony at first and paid back on the second rename.
- 2026-08-20 [structure/framework-upgrade] **Upgrading every project from .NET 6 to .NET 8 in one PR stalled for three weeks; one deployable per PR finished in eight days.** The big-bang branch had 1,900 changed files and about 40 conflicts per rebase. Multi-targeting the shared libraries (net6.0;net8.0) let each service move on its own, and the order — libraries, then workers, API last — kept main deployable every day. Discriminator: a framework upgrade across more than a handful of deployables. (Recurred 2026-10-06: the React 17 to 18 upgrade started as one branch again; after a week it was split into per-route PRs behind the new root API.)
- 2026-09-03 [structure/shared-tables] **A finance reporting tool that read our tables directly turned every column rename into a cross-team incident.** Its connection read bookings and invoices straight from the primary, and renaming amount to amount_gross broke their month-end report twice. A reporting schema of views that we own, with a one-release deprecation window, gave them a stable surface and gave us freedom in the base tables. It also moved their 15-minute queries off the primary onto a read replica. Discriminator: any consumer outside the team that reads your tables.
- 2026-09-16 [structure/ci-workflows] **One 1,400-line GitHub Actions workflow meant any edit could break deploys for every service.** An indentation change in the frontend job disabled the API deploy step for two days, unnoticed until a hotfix failed to ship. Splitting into reusable workflows (build, test, deploy) with a small caller per service cut the main file to 120 lines and let path filters skip unaffected jobs, saving about 9 minutes per PR. Discriminator: one workflow file shared by several services.
- 2026-09-19 [structure/shared-kernel] **The Shared project became the place every cross-team type went, and every change to it rebuilt all 23 projects.** In June it held 9 types; by September it held 140, including three different Address classes. A change there forced a full 28-minute CI build instead of 6 minutes. Moving each type back to the module that owns it, and leaving only true primitives (Money, TenantId, Result), cut the dependents of a typical change from 23 projects to 4. Discriminator: a "Common" or "Shared" assembly that more than one team adds to and nobody owns.
- 2026-09-26 [structure/package-drift] **Twelve services pinned twelve versions of the internal auth package, and a security fix reached only four of them in the first month.** Services updated only when someone touched them, so the token-validation fix sat unshipped in 8 of them for 34 days. One version for the whole repo in Directory.Packages.props, plus a weekly automated update PR, got the next fix into all 12 in two days. Discriminator: an internal package used by many deployables in one repo.
- 2026-10-14 [structure/soft-delete] An IsDeleted column on every table made unique constraints lie. A staff member deleted and re-invited with the same email hit the unique index, because the deleted row still held it, and support hard-deleted 40 rows by hand. Partial unique indexes (WHERE NOT is_deleted) fixed that, and for the two entities kept only for legal retention, moving deleted rows to archive tables took the filter out of every query. Discriminator: soft delete on tables with natural unique keys.
Agent prompt (SKILL_PATH = the skill file under test, TEST_DIR = the folder holding the session and learnings files)
You are the coding agent at the end of a session in a project that uses the Lattice framework. The code-forge workflow has just finished and now invokes the learning-harvest skill's Harvest Behavior, with session context: "implementation session — code generation from design contracts".

1. The learning-harvest skill text is the file SKILL_PATH. Treat it as your loaded skill instructions and follow it exactly. Do NOT use the Skill tool and do not read any other skill or plugin file: this file is the skill.
2. What happened in the session is in TEST_DIR/session.md. Treat it as your own memory of the session. Active monitoring was on during the session; apply it as the skill describes.
3. The project's operational learnings file is TEST_DIR/operational-learnings.md. Read it as the skill tells you to.
4. Do not edit or create any file. Whenever the skill would have you write to the learnings file, print instead, under a heading "WRITES", the exact final text of every entry you add or change, each exactly as it would appear in the file.

Your reply is exactly what the user would see in the chat. When the skill says to wait for the user, stop and end your reply there.

After the proposal, the simulated user replied: "Accept all, and add the recurrence note too."

🤖 Generated with Claude Code

@IdoGvili
IdoGvili requested a review from techygarg as a code owner October 22, 2026 12:30
…fted

Once operational-learnings.md holds long entries, Harvest copies their
length, and recurrence notes copy the file's own story-style notes. The
two-line rule now holds whatever the existing entries look like, and a
recurrence is recorded as a date.
@IdoGvili
IdoGvili force-pushed the fix/learning-harvest-entry-drift branch from 6b729dc to b044e81 Compare October 22, 2026 13:16

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant