fix: bound the habit read path and let the performance audit see production (ORB-162) - #699
fix: bound the habit read path and let the performance audit see production (ORB-162)#699thomasluizon wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e82c623888
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ea3ab83e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21f7fe36cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 435467548e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
All three were fixed in BOTH copies of the logic. tools/lib/performance-measurement.mjs and .claude/workflows/audit.mjs carry the same code under different names, and the suite exercises both, so fixing one would have left the pair drifting. That pairing is itself the defect class ORB-215 exists to end. 1. audit.mjs:48, measure the PROJECTED result width, not the full table row. `bytesPerRow` comes from pg_stats and describes the whole row, so charging it to a narrow projection overstated a careful query in direct proportion to how well it was written, which is backwards for a signal meant to find unbounded reads. The column count was also wrong in both directions: `select *` counted ONE column and so never matched full-entity-projection, and a comma inside `coalesce(a, b)` counted as a column boundary. The scaling is an approximation and says so; it never charges MORE than the measured row width. This corrected a real FALSE POSITIVE in the fixture. `reminder-sweep` selects 2 of 29 Habits columns, so the old arithmetic claimed 190 MB a month against a real 13 MB and the background budget fired on a query nowhere near it. The assertion now names a genuinely wide sweep, and a second assertion records that the narrow one correctly no longer fires. The fixture was extended rather than reseeded: nothing was relaxed to make a test pass. 2. audit.mjs:133, reject or merge duplicate measured-query mappings. `new Map(mappings.map(...))` silently kept the LAST entry. Two mappings that disagree about executionContext produce opposite signals, so whichever the agent emitted second decided the finding and nothing recorded there had been a choice. Identical duplicates merge; a real disagreement throws naming both. An unmapped query now names itself instead of dying on a bare TypeError. 3. use-habits.ts:155, reconcile response-backed rewards without a list cache. Rewards the server had already granted were discarded whenever the habit was absent from the list cache, the ordinary state on a deep link or a cold navigation: XP banked server-side and never shown, achievements never refreshed, silently. Reconciliation is now driven by the response alone. The CELEBRATION deliberately still requires a known good habit. A bad habit's streak is consecutive abstinence, the opposite semantics, and an unresolvable habit cannot be shown to be either, so celebrating on a guess stays forbidden. Reconciling is safe; celebrating is not. Both existing tests that defend that line still pass unchanged. One fixture was corrected rather than worked around: it sent `xpEarned: 25` for a bad habit, a response the API cannot produce. GamificationService.cs:170 is `habit.IsBadHabit ? 0 : ...`, verified in source. The client needed its own gate only because the fixture encoded an impossible server reply, and that gate is what dropped the rewards. The same code also dropped BadHabitBreaker, which GamificationService.cs:175 genuinely grants to bad habits. Parity: the hook fix and its test land in apps/web AND apps/mobile. Red-capable proof, each guard removed then restored: projected width disabled -> 2 FAIL star expansion disabled -> 3 FAIL duplicate mapping index -> 1 FAIL rewards re-gated on cache -> 1 FAIL web, 1 FAIL mobile all restored -> ORBIT TOOLS GATE OK, ORBIT HOOKS OK Suites, all with --force: web 2578 passed, mobile 1621 passed, 4 tasks successful. Refs ORB-162 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
The three remaining P1s are closed in
|
| Assumption | How it broke |
|---|---|
a SELECT list splits on , |
coalesce(a, b) counted as two columns |
a SELECT list splits on , |
select * counted as ONE column, so a star never matched full-entity-projection |
bytesPerRow is what a query sends |
it is the whole row, so a 2-of-29 column projection was billed 15x its real egress |
| one mapping per queryId | new Map(mappings.map(...)) silently kept the last of a conflicting pair |
| every measured query is mapped | an unmapped one died on a bare TypeError naming nothing |
The multiplier: the same logic exists TWICE, in tools/lib/performance-measurement.mjs and inlined in .claude/workflows/audit.mjs, under different names (selectedColumnCount against an inline expression, DAYS_PER_MONTH against PERFORMANCE_MONTH_DAYS). The suite exercises both, one by import and one by slicing the workflow text into a VM, so a fix applied to one copy leaves the other wrong and green. That is precisely the ORB-215 pair class, inside the tool ORB-215's own audits depend on.
Scope
- Single-source the measured-performance logic. One module, imported by both consumers, or a generated workflow with a drift gate like
arch-map.ymlalready has. Two hand-maintained copies of arithmetic is the multiplier on every finding above. - Replace positional SQL parsing with something that can say "I do not know." Every current signal is derived from regexes over a query shape. The honest failure mode is a null that suppresses a signal, not a wrong number that raises one. Audit each signal for which way it fails today.
- Property-test the arithmetic rather than adding one fixture per discovered case. Egress must never exceed
rows * bytesPerRow; projected columns must never exceed the table's column count; a signal must never fire on a query whose measured egress is under its budget. Those three invariants would have caught four of the nine P1s before any reviewer saw them.
Out of scope
- The three P1s themselves. They are fixed in fix: bound the habit read path and let the performance audit see production (ORB-162) #699 and this ticket owns the CLASS, not the instances.
- The audit workflow's agent prompts, fan-out, or verdict handling. This is only the measured-performance arithmetic and its SQL reading.
Expected behaviour
A new query shape that the parser cannot read produces a null and no signal, and says so. A change to the arithmetic cannot land in one copy only. A reviewer reading a background-sweep-budget finding can trust that the number behind it is what the query actually sends.
Acceptance criteria
- One copy of the measured-performance logic, with a gate that fails if a second diverges.
- The three invariants above are property-tested, not fixture-tested.
- Every signal documents which way it fails when the query shape cannot be parsed, and the tests prove it fails that way.
- No existing signal changes verdict on the recorded fixture except where the change is a documented correction, as with the narrow-sweep false positive fixed in fix: bound the habit read path and let the performance audit see production (ORB-162) #699.
Related
Adjacent to ORB-174. Split out of ORB-162 deliberately: #699's three remaining P1s were specific, located and small, so they were finished there rather than deferred, and the systemic hardening is this ticket because the pattern says the next blind spot is one state over from the last fix.
Evidence: connector passes on #699 across commits 4ea3ab83, 21f7fe36, 43546754, and the fixes in 86099b04.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86099b0427
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const projectedBytesPerRow = Number.isFinite(entry.bytesPerRow) && entry.bytesPerRow > 0 | ||
| ? projectionColumns && table?.columnCount > 0 | ||
| ? entry.bytesPerRow * Math.min(1, projectionColumns / table.columnCount) |
There was a problem hiding this comment.
Fresh evidence after the 86099b0 fix is that projected width is now estimated from the fraction of columns selected, even though PostgreSQL columns can differ by orders of magnitude in size. For example, selecting one large text/JSON column from a table with many small scalar columns can be charged only a small fraction of its actual payload, pushing a background sweep below the 50 MiB threshold and corrupting the ranking/readiness verdict; measure the selected expressions' actual width or retain this metric as unknown rather than deriving bytes from column count.
Useful? React with 👍 / 👎.
| void queryClient.invalidateQueries({ | ||
| queryKey: habitKeys.summaryPrefix(), | ||
| refetchType: 'none', |
There was a problem hiding this comment.
When a user logs a habit while the Today Astra summary is mounted, refetchType: 'none' only leaves the summary query stale instead of refreshing its displayed result. useSummary disables focus refetching and schedules its next request for the next summary time bucket, so the card can continue describing the pre-completion habit state until that timer fires or the user manually retries; keep active-summary refetching or reconcile/remove the cached summary immediately. The mobile helper introduces the same behavior for every habit mutation.
Useful? React with 👍 / 👎.
|
1 participant
Closes ORB-162.
https://linear.app/useorbitai/issue/ORB-162
What this does
Fixes the unbounded habit read path that pushed Supabase egress to 112% of its 5 GB quota, and gives the performance audit a way to see production rather than guessing at it.
How this pull request came to exist
The implementing worker committed its complete work and then its process tree was killed at
2026-08-08T04:29:33Z, before it could push or open a pull request. This was recovered through the orchestrator's salvage path: no code was written outside the worker's own commit, nothing was staged, and the branch carries exactly the one commit the worker made (e82c6238).Salvage requires a green touched-workspace test before any push. Recorded receipt, run in the worktree at
e82c6238:node tools/test-tools.mjsORBIT TOOLS GATE OKnode .claude/hooks/test-hooks.mjsORBIT HOOKS OKnpm run type-checknpm run test@orbit/web310 files, 2,577 tests passedBoth harnesses ran because this change touches
tools/**and.claude/**.Review status
The independent cross-vendor review could not be launched on this machine.
tools/launch-worker.mjscompares the working-tree bytes of.claude/skills/pr-review/{SKILL.md,rubric.md}across orbit-ui-mobile and orbit-api and refuses--reviewfor either repository on a mismatch. The committed blobs are byte-identical in both repositories; orbit-api's.gitattributespins only.claude/skills/**/*.mjstoeol=lfwhile orbit-ui-mobile also pins*.md, so withcore.autocrlf=truethe same blob materializes as LF in one checkout and CRLF in the other. Repairing a tool or a skill from inside a run is forbidden, so it is recorded rather than fixed.This pull request therefore has not had an independent review and is not READY. Do not merge it on the strength of green checks alone.