You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fixes chat layout overlap and makes chat persistence race-safe with request-scoped SSE IDs and guarded state transitions. Hardens streaming (code-fence backtick handling, dimension-aware markdown) and validation (word-boundary DIMENSION matching, per-chunk thresholds), expands the quality engine with 11 rules, and keeps the test suite green.
Bug Fixes
Layout: ChatDock is a static flex child; replaced dynamic padding with pb-8 in DashboardLayout.
Persist lifecycle: worker requires requestId; all SSE events include it; client tracks activePersistRequestId, filters stale SSE; no auto-promote on done; early returns reset to idle; timer won’t clear newer requests; error capture via @sentry/nextjs and @sentry/cloudflare.
Stream adapter: strip only trailing code-fence ``` before JSON heal; markdown rebuild uses dimension-count monotonicity with forced rebuild on fallback; bundle fallback preserves personaConfig/`knowledgeGraph`/`classification`/`monetizationVerdict` outside the bundle.
Validation: validate12D(expectedCount) clamps to 1–11; adds word-boundary header matching to avoid DIMENSION 10 matching 1; UCIS regexes broadened (bold headers, lens tags, power quotes), checkmark filtering added, table pattern relaxed.
…icity bug
- Persist correlation IDs: activePersistRequestId in useChatStore, requestId in chat-stream SSE events
- Persist abort UI: worker emits persist: saving/saved/failed events
- Stream adapter facade refactored into 3 sub-modules (delta handler, markdown accumulator, status tracker)
- Analysis history status: uses billing_status as primary status
- healJson test: matches real flow (backticks stripped by handleDelta)
- Monotonicity test: fixes stale zustand state reference in assertions
- Global state preservation test: fixes stale zustand state reference
- stream-status-tracker: fix fallback calls rebuildMarkdown(true) to force rebuild
- synthesis-stream-adapter: healJson exposed for test compatibility
This pull request has been ignored for the connected project adnmbikaqnxivalqoild because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.
Wave 5 stabilization sweep: adds request-scoped SSE persistence tracking across the worker and chat store to discard stale events, fixes stream adapter markdown rebuild monotonicity and fallback state clearing, generalizes 12D validation to accept a configurable dimension count, refactors chat dock layout from absolute to flex static, tightens env placeholder detection with comprehensive test coverage, corrects UCIS validator regexes and test fixtures, registers 11 new quality-engine static analysis rules, and documents all changes in Wave 5 critique report and activity ledger.
Changes
Wave 5 Stabilization
Layer / File(s)
Summary
SSE requestId threading: worker-side stream and persistence events worker/src/chat-stream.ts
ChatStreamRequest gains requestId? field. handleChatStream validates requestId presence (400 rejection if missing). All SSE delta, persist, and done events now include requestId in payloads: normal/empty/error deltas, persist:saving/saved/failed, and final done. Exception path reports to Sentry with requestId context.
SSE requestId filtering: store-side persistence state gating web/store/useChatStore.ts
ChatState adds activePersistRequestId. deliver() sets it to clientMsgId and initializes persistState as saving. Worker stream includes requestId: clientMsgId. SSE event callback filters by requestId—discarding mismatches. setPersistState accepts requestId and gates updates; timeout resets check activePersistRequestId to avoid clearing newer state. sendMessage/flushOutbox capture failures with per-message context via Sentry.
SSE requestId race-condition and integration tests web/lib/__tests__/useChatStore-race.test.ts, worker/src/__tests__/chat-stream-requestId.test.ts
useChatStore-race.test.ts validates five scenarios: persistState stays saving until persist:saved arrives, stale events are discarded, concurrent requests stay isolated, idle resets do not clear newer state, and failed events from old requests are ignored. chat-stream-requestId.test.ts parameterizes 4 scenarios (success/empty/error × persist-ok/fail), mocking streamChatCascade and /api/chat/persist, asserting every SSE frame carries the correct requestId and events arrive in expected order.
rebuildDisplayMarkdown gains force: boolean = false parameter; only overwrites analysis_markdown when forced or reconstructed content has more/equal-longer dimension headers. handleDelta trims trailing backticks before JSON healing. handleStatus inverts fallback state-clearing logic for global state (preserve unless dimension ID absent) and calls rebuildMarkdown(true) for fallback. processLine forwards force through callbacks.
Stream adapter markdown rebuild test coverage web/lib/__tests__/synthesis-stream-adapter.test.ts, web/lib/__tests__/markdown-accumulator-fallback.test.ts, web/lib/__tests__/stream-delta-backtick.test.ts
synthesis-stream-adapter.test.ts covers healJson parsing after backtick stripping, monotonic rebuildDisplayMarkdown behavior, and bundle fallback state preservation. markdown-accumulator-fallback.test.ts validates forced vs non-forced rebuild semantics, dimension header counts, and length preservation. stream-delta-backtick.test.ts confirms backticks within JSON are preserved while complete fence closers are stripped.
clientEnv uses isPlaceholder() checks instead of || fallbacks for three NEXT_PUBLIC_* keys (SUPABASE_URL, SUPABASE_ANON_KEY, WORKER_URL). Missing or placeholder values route to MOCK_DEFAULTS instead of using process.env directly. Test suite validates detection for dummy/placeholder/stub/ci-build markers, non-detection of legitimate values, and edge cases.
UCIS validator regex corrections and test fixture enrichment web/lib/ucis-v5-validator.ts, web/lib/__tests__/ucis-v5-validator.test.ts
PATTERNS add optional ** wrappers and global matching for persona/lens tags, relax table-cell bold requirements, filter ⚠/✓ from illegal emoji, expand powerQuoteMatches for ** wrappers. Test fixture enriched with Analysis Timestamp, ranked deliverables, reformatted Read-Depth Guidance, expanded Dimension 5/6 insights and scenario analysis, power quotes, Cross-Domain Bridges, and Unfair Advantages mapping.
Node and edge filtering simplifies to return type-check predicates directly, removing intermediate isValid variable and console.warn side effects for malformed items.
11 new quality-engine rules implementation scripts/quality-engine/rules.ts
Exports EnvPlaceholderNamespaceRule, SyncImportBeforeRedirectRule, QuorumTimeoutCompletionRule, ModuleLevelDynamicImportRule, ToastAccessibilityRule, SwallowedErrorRule, StaleStateResetRule, HardcodedDomainLogicRule, StateSyncRule, InsecureFallbackRule, and CanvasStaleDataRule. Each implements static-analysis checks for environment safety, import ordering, timeout correctness, accessibility, error handling, state consistency, domain logic, and rendering data dependencies.
Imports and registers all 11 new rule classes (plus TranscriptUnsafeAccessRule) in the engine's rule sequence, extending the quality-engine analysis pipeline.
Quality-engine rule test coverage web/lib/__tests__/indent-heuristic.test.ts, web/lib/__tests__/stream-status-tracker-fallback.test.ts
indent-heuristic.test.ts validates ModuleLevelDynamicImportRule indentation scanning for function-local (non-flagged), top-level (flagged), and deeply-indented (non-flagged) dynamic imports. stream-status-tracker-fallback.test.ts validates bundle and full fallback scenarios, state preservation logic, and dimensionsReceived updates.
Test infrastructure, Vitest config, and Wave 5 documentation web/vitest.config.ts, web/lib/__tests__/rate-limit-sliding-window.test.ts, docs/specs/WAVE5_CRITIQUE_REPORT.md, .memory/AGENT_LEDGER.md
Vitest config imports configDefaults, extends test.exclude with configDefaults.exclude and tests/**, adds worker-related aliases and module resolution settings. Rate-limit test gains vi.mock Redis emulator with in-memory sliding-window and beforeEach reset. WAVE5_CRITIQUE_REPORT.md documents layout remediation, console warning triage, and validator fixes. AGENT_LEDGER.md records qa-intel expansion, W5-3 KG fix, pr-review-workflow completion (PR #92, 65% confidence), and Antigravity findings.
Sequence Diagram(s)
sequenceDiagram
participant Client as useChatStore
participant Worker as chat-stream.ts (Worker)
participant SSE as SSE Event Stream
Client->>Worker: POST { requestId: clientMsgId, dimensions, ... }
Worker->>SSE: delta { requestId }
Worker->>SSE: persist "saving" { requestId }
Worker->>SSE: persist "saved"/"failed" { requestId }
Worker->>SSE: done { requestId }
rect rgba(255, 100, 100, 0.5)
note over Client,SSE: Stale event filtering
SSE-->>Client: event { requestId: oldId } → discarded (≠ activePersistRequestId)
end
rect rgba(100, 200, 100, 0.5)
note over Client,SSE: Current request
SSE-->>Client: event { requestId: clientMsgId } → setPersistState(state, clientMsgId)
end
Client->>Client: timeout: reset only if activePersistRequestId still matches
Loading
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Hex-Tech-Lab/hex-yt-intel#31: Both PRs modify the console chat UI by changing ChatDock.tsx and DashboardLayout.tsx behavior/layout for docked chat area positioning.
Hex-Tech-Lab/hex-yt-intel#89: Both PRs modify stream fallback handling and markdown rebuild control in StreamStatusTracker/MarkdownAccumulator plus dimension-bundle fallback semantics introduced in PR #89.
Hex-Tech-Lab/hex-yt-intel#91: Both PRs add chat persistence refactoring via requestId in worker SSE and store-side gating, building on Wave 4 chat-stream persistence work.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Title check
✅ Passed
The PR title accurately summarizes the main changes: Wave 5 remediations focusing on persist lifecycle, stream adapter fixes, and test suite improvements.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch fix/wave5-remediations
✨ Simplify code
Create PR with simplified code
Commit simplified code in branch fix/wave5-remediations
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.
We reviewed changes in 82ef0bf...31d7f55 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
PR Report Card
Overall Grade
Focus Area: Reliability
Security
Reliability
Complexity
Hygiene
Feedback
Null/any usage in tests and services
The any in tests plus the many non-null assertions show up across both test files and ValidationService.
Together they suggest places where types aren’t doing much work for you; a bit more explicit typing would turn these runtime-reliability risks into compile-time feedback.
Test-only setup noise
The unused fetchSpy is repeated in several test files.
Looks like shared setup that isn’t always needed; trimming or centralizing that setup would keep the growing test suite leaner and easier to scan.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 7 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 11 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 9 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 10 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 11 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 7 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
`validate12D` has a cyclomatic complexity of 8 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR successfully consolidates Wave 5 remediations, notably the stabilization of chat persistence and layout improvements. Codacy analysis indicates the changes are up to standards with a net reduction in code clones.
However, there is a discrepancy between the provided documentation and the code: the 'billing_status' remediation for Analysis History is mentioned in the AGENT_LEDGER but is missing from the file changes. Additionally, two critical logic updates — the SSE requestId filtering in useChatStore.ts and the dynamic dimension validation in ValidationService.ts — lack corresponding unit test coverage in this PR. These gaps should be addressed to ensure the stability of the new segmented streaming architecture.
About this PR
The PR description and AGENT_LEDGER mention a fix for Analysis History status accuracy using 'billing_status' as the authoritative indicator, but the corresponding code changes (database queries or history component updates) are missing from the diff.
Several critical logic updates lack unit tests: specifically the filtering of SSE events by requestId in useChatStore.ts and the updated validate12D logic in ValidationService.ts. These should be covered by tests to prevent regressions in stream handling.
Test suggestions
Verify that trailing backticks are stripped from the stream sink before JSON healing
Ensure markdown reconstruction is monotonic and only overwrites longer content when forced
Verify global synthesis state (Persona/KG) is preserved when a bundle-specific fallback occurs
Confirm that useChatStore ignores SSE events with a requestId that does not match the activePersistRequestId
Validate that validate12D passes when the number of dimensions matches the dynamic expectedCount for partial streams
Confirm that UCIS validator correctly identifies deliverables when headers are wrapped in markdown bold tags
Verify that the rate-limit test suite mocks Redis script execution for deterministic results
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Confirm that useChatStore ignores SSE events with a requestId that does not match the activePersistRequestId
2. Validate that validate12D passes when the number of dimensions matches the dynamic expectedCount for partial streams
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
Critical fix for stream atomicity. By verifying the requestId against the clientMsgId, the store effectively prevents 'phantom' updates from interleaved or delayed SSE events. This is a best practice for resilient LLM streaming.
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
This correction is vital for maintaining state continuity during segmented streaming. It prevents the UI from losing existing persona or knowledge graph data when a partial update (that doesn't contain those specific dimensions) triggers a fallback reset.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: While effective for stripping trailing backticks from the stream, this can be handled more efficiently with a single regex replace to avoid multiple string allocations and trims.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/components/templates/console/ChatDock.tsx (1)
136-144: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Layout refactor correctly implements flex-static dock positioning.
The change from absolute positioning to flexShrink: 0 + width: '100%' is the correct approach for a flex-column layout. The dock will maintain its height and appear at the bottom (confirmed by DOM order in DashboardLayout line 46).
Optional cleanup: The zIndex property typically only affects positioned elements (absolute, relative, fixed, sticky). Since the dock is now statically positioned, zIndex may no longer have an effect. Consider removing it unless there's a specific stacking context requirement.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/components/templates/console/ChatDock.tsx` around lines 136 - 144, The
zIndex property in the shell style object is no longer effective since the dock
is now using static positioning (flexShrink: 0) rather than absolute
positioning, as zIndex only affects positioned elements. Remove the zIndex
property line from the shell CSSProperties object unless there is a specific
stacking context requirement that necessitates it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/specs/WAVE5_CRITIQUE_REPORT.md`:
- Line 23: The document has several markdown formatting inconsistencies that
should be corrected: add blank lines before and after section headings
(including "### Critique of the Fix" and other h3-level headings throughout the
document), change the lowercase word "markdown" to "Markdown" where it appears
as a proper noun, and remove extra spaces inside code span delimiters to follow
markdown linting standards. These formatting improvements will enhance
documentation consistency and pass markdown linter validation.
In `@scripts/quality-engine/rules.ts`:
- Line 912: The forEach callback in the lines.forEach method declares an unused
idx parameter that should be removed to keep the code clean. Remove the idx
parameter from the forEach callback signature since it is never referenced
within the callback body.
In `@web/lib/__tests__/rate-limit-sliding-window.test.ts`:
- Line 7: In the executeRedisScript mock function definition, replace the
`any[]` type annotation for the args parameter with the more specific type
`(string | number)[]` to match the actual signature from the redis module and
improve type safety throughout the test file.
In `@web/lib/__tests__/synthesis-stream-adapter.test.ts`:
- Around line 195-203: The test for trailing backticks is passing already-valid
JSON directly to healJson, which doesn't exercise the backtick stripping logic
in the actual delta-processing flow. Modify the test to create a delta fragment
that contains actual trailing backticks (fence characters), route it through the
processLine and handleDelta methods (simulating the real flow), and then assert
that the final parsed result is correct. This will properly validate that
backticks are stripped during delta processing before healJson is called.
In `@web/store/useChatStore.ts`:
- Around line 304-306: The issue is that the code unconditionally promotes the
persistence state from 'saving' to 'saved' based solely on the state still being
'saving' after deliver() returns, without verifying that delivery was actually
successful. This can report false success on early-return error paths (like
missing stream endpoint). Modify the condition in the setPersistState call at
lines 304-306 to also check that the deliver() operation completed successfully
(check the return value or status from deliver() to ensure it didn't fail on an
early-return path). Apply the same fix to the duplicate code block at lines
363-365 that has the identical issue.
- Around line 307-313: Both the sendMessage() catch block (lines 307-317) and
flushOutbox() catch block (lines 366-370) are missing required error handling.
Add Sentry.captureException() with appropriate context information to both catch
blocks to track errors in your monitoring system. Additionally, add
console.error() logging with a standardized format like
console.error('[chat-send]', { message, error, context }) to both locations for
debugging. Finally, ensure both catch blocks use the universal error message
extraction pattern: error instanceof Error ? error.message : String(error) when
accessing error information, rather than assuming the error object has a message
property.
In `@worker/src/chat-stream.ts`:
- Around line 296-299: The catch blocks in the chat-stream.ts file are missing
Sentry error capturing. First, ensure Sentry is imported from `@sentry/nextjs` at
the top of the file. Then, in the catch block that sets the "The model request
failed" message, add Sentry.captureException(error, { contexts: { ... } }) with
appropriate context information before or after the error message assignment.
Additionally, apply the same Sentry.captureException pattern to the other catch
block around line 334-340 that already has the universal error pattern and
structured logging, ensuring both blocks consistently capture errors to Sentry
with relevant context data.
In `@worker/src/services/ValidationService.ts`:
- Around line 16-17: In the ValidationService.ts file, the targetCount variable
assignment currently uses expectedCount without validating that it is a positive
integer, allowing zero or negative values to pass through which causes the
dims.length >= targetCount comparison to be trivially true. Modify the
targetCount assignment to clamp expectedCount to a positive integer by checking
if expectedCount is greater than 0, and if not, fall back to the default value
of 8, ensuring that invalid or non-positive counts cannot bypass the validation
threshold.
- Line 35: The issue in the filter callback on line 35 within the
ValidationService is that analysis.includes(dim) performs a simple substring
match, causing false positives when dimension names collide as substrings (e.g.,
"DIMENSION 1" matches within "DIMENSION 10"). Replace the includes() check with
a more precise matching approach that treats each dimension as a complete word
boundary, such as using a regular expression with word boundaries or checking
for the dimension followed by non-alphanumeric characters. This ensures only
exact dimension matches are counted, preventing the overcounting that could pass
invalid text payloads.
---
Outside diff comments:
In `@web/components/templates/console/ChatDock.tsx`:
- Around line 136-144: The zIndex property in the shell style object is no
longer effective since the dock is now using static positioning (flexShrink: 0)
rather than absolute positioning, as zIndex only affects positioned elements.
Remove the zIndex property line from the shell CSSProperties object unless there
is a specific stacking context requirement that necessitates it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e802b0d-8dcc-499b-a6ed-d14e686d9747
📥 Commits
Reviewing files that changed from the base of the PR and between 82ef0bf and 6972d25.
The user highlighted that the scrollable central column contents (e.g., accordion dimensions cards, metadata container) were flushing behind the fixed bottom chat box.
Lowercase "markdown" should be "Markdown" (line 67)
Spaces inside code span (line 70)
These are minor documentation quality issues that could improve consistency.
Also applies to: 34-34, 39-39, 44-44, 49-49, 67-67, 70-70
🧰 Tools🪛 markdownlint-cli2 (0.22.1)
[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/specs/WAVE5_CRITIQUE_REPORT.md` at line 23, The document has several
markdown formatting inconsistencies that should be corrected: add blank lines
before and after section headings (including "### Critique of the Fix" and other
h3-level headings throughout the document), change the lowercase word "markdown"
to "Markdown" where it appears as a proper noun, and remove extra spaces inside
code span delimiters to follow markdown linting standards. These formatting
improvements will enhance documentation consistency and pass markdown linter
validation.
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
The new “trailing backticks” test doesn’t hit the changed code path.
This test feeds already-valid JSON directly into healJson, so it doesn’t validate backtick stripping in the delta-processing flow. Please route an actual delta fragment containing trailing fence/backticks through processLine/handleDelta and assert the expected parsed behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/lib/__tests__/synthesis-stream-adapter.test.ts` around lines 195 - 203,
The test for trailing backticks is passing already-valid JSON directly to
healJson, which doesn't exercise the backtick stripping logic in the actual
delta-processing flow. Modify the test to create a delta fragment that contains
actual trailing backticks (fence characters), route it through the processLine
and handleDelta methods (simulating the real flow), and then assert that the
final parsed result is correct. This will properly validate that backticks are
stripped during delta processing before healJson is called.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cd worker/src && wc -l chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 88
🏁 Script executed:
cd worker/src && sed -n '290,305p' chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 665
🏁 Script executed:
cd worker/src && sed -n '330,345p' chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 795
🏁 Script executed:
cd worker/src && head -50 chat-stream.ts | grep -E "^import|^export"
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 374
🏁 Script executed:
cd worker/src && grep -n "import.*Sentry\|from.*sentry" chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 51
🏁 Script executed:
cd worker/src && grep -n "catch" chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 201
🏁 Script executed:
cd worker/src && sed -n '1,100p' chat-stream.ts | grep -A 5 "^import"
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 802
🏁 Script executed:
cd worker/src && sed -n '280,310p' chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 1220
🏁 Script executed:
cd worker/src && sed -n '328,350p' chat-stream.ts
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 1007
🏁 Script executed:
cd worker/src && grep -B5 -A10 "catch" chat-stream.ts | head -80
Repository: Hex-Tech-Lab/hex-yt-intel
Length of output: 2554
Add Sentry capture and error handling to updated catch blocks.
Line 296–299 catch block is missing all three required error handling patterns: Sentry capture, universal error pattern, and structured error logging.
Line 334–340 already implements the universal catch pattern and structured logging but is missing Sentry capture. Add Sentry.captureException(error, { contexts: { ... } }) to both blocks. Note: Sentry must be imported from @sentry/nextjs first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@worker/src/chat-stream.ts` around lines 296 - 299, The catch blocks in the
chat-stream.ts file are missing Sentry error capturing. First, ensure Sentry is
imported from `@sentry/nextjs` at the top of the file. Then, in the catch block
that sets the "The model request failed" message, add
Sentry.captureException(error, { contexts: { ... } }) with appropriate context
information before or after the error message assignment. Additionally, apply
the same Sentry.captureException pattern to the other catch block around line
334-340 that already has the universal error pattern and structured logging,
ensuring both blocks consistently capture errors to Sentry with relevant context
data.
The reason will be displayed to describe this comment to others. Learn more.
`validate12D` has a cyclomatic complexity of 9 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
- Remove premature auto-promote to 'saved' from deliver() return path
(persist: saved SSE event is the authoritative confirmation)
- Fix setPersistState guard: allow new requests to take over, discard
stale terminal events from older requests
- Set activePersistRequestId in setPersistState (was never set before,
making the guard non-functional)
- Add useChatStore race condition tests (5 tests):
- No auto-promote without persist: saved event
- Stale request events discarded
- Timer cleanup doesn't clear newer request state
- Concurrent request isolation
- Error events from stale requests discarded
- Add chat-stream requestId propagation test matrix (skeleton)
- markdown-accumulator.ts: Replace length-based monotonicity with dimension
count comparison. Length is a flawed proxy — shorter corrected output can
be more accurate. Combined with length guard for equal-dimension cases to
prevent empty reconstructions from overwriting substantive markdown.
- chat-stream-requestId.test.ts: Rewrite as proper 4-case parameterized
test with mock Hono context, streamChatCascade spy, and fetch mock.
Validates requestId on every SSE event, event ordering, and content
delivery integrity across success/empty/error × persist ok/fail.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
`rebuildDisplayMarkdown` has a cyclomatic complexity of 12 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
The reason will be displayed to describe this comment to others. Learn more.
Found `async` function without any `await` expressions
A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
The reason will be displayed to describe this comment to others. Learn more.
Found `async` function without any `await` expressions
A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:
The reason will be displayed to describe this comment to others. Learn more.
Found `async` function without any `await` expressions
A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/lib/__tests__/rate-limit-sliding-window.test.ts` at line 7, The mock
function executeRedisScript in the test file uses unknown[] as the type for the
args parameter, which does not match the real production contract. Change the
args parameter type from unknown[] to (string | number)[] in the vi.fn() mock
definition to ensure the test enforces the same input constraints as the actual
implementation and catches invalid argument construction early.
worker/src/services/ValidationService.ts (1)
16-19: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Clamp expectedCount to [1,11] before flooring to avoid targetCount = 0.
For inputs like expectedCount = 0.5, the current logic passes > 0 then floors to 0, making validation trivially pass (dims.length >= 0). Clamp after integer validation so the lower bound is always 1.
#!/bin/bash# Verify all validate12D call sites and inspect whether expectedCount arguments are guaranteed integers.
rg -n -C3 '\bvalidate12D\s*\(' worker/src
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@worker/src/services/ValidationService.ts` around lines 16 - 19, The
targetCount clamping logic in the ValidationService.ts file currently checks if
expectedCount is greater than 0 before flooring, but this allows values like 0.5
to pass the check and floor down to 0, making validation trivially pass. Reorder
the clamping operations to first floor the expectedCount value (after verifying
it is finite), then apply Math.max with 1 as the lower bound and Math.min with
11 as the upper bound to ensure targetCount is always between 1 and 11
inclusive. This prevents targetCount from ever being 0.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/store/useChatStore.ts` around lines 180 - 183, The guard condition in the
readSSE callback within useChatStore.ts currently allows events without a
requestId to pass through and be processed. To enforce strict request-scoped
isolation, modify the condition in the readSSE callback to reject events that
either lack a requestId entirely or have a requestId that does not match the
clientMsgId. Change the guard from checking if requestId exists and differs from
clientMsgId to instead checking if requestId is missing OR does not match
clientMsgId, ensuring only properly correlated events with matching requestIds
are processed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/lib/__tests__/useChatStore-race.test.ts`:
- Around line 84-89: In the test case that validates stale-event handling, add a
direct assertion for the persistState value alongside the existing
activePersistRequestId assertion. After the line checking that
activePersistRequestId equals 'req-B', add another expect statement to verify
that useChatStore.getState().persistState is in the correct state (likely
'saved' based on the test context) to ensure that stale saved/failed mutations
cannot bypass validation undetected.
- Around line 28-41: The beforeEach function overwrites global.fetch with a mock
but the afterEach function does not restore it, causing the mock to persist
across tests and create cross-test pollution. Store the original global.fetch
value before assigning the mock in beforeEach, then restore the original value
in the afterEach function using vi.restoreAllMocks() or by explicitly
reassigning the saved original fetch.
In `@worker/src/__tests__/chat-stream-requestId.test.ts`:
- Around line 46-50: The test fixture in the payload object uses a hardcoded sig
value that will not pass HMAC validation, and the mock context does not
implement the c.json() method needed for early-return authentication branches.
To fix this, generate the sig value dynamically using proper HMAC validation
based on the payload contents (exp, appUrl, and requestId fields) instead of
using a hardcoded string, and ensure the mock context properly implements all
required handler methods including c.json() so the test can reliably reach the
SSE streaming path being tested.
- Around line 64-66: The test is currently mocking context.send via the sendFn
function to track events in trackingSink, but handleChatStream actually writes
events to the returned response.body stream as SSE frames. Instead of relying on
trackingSink, parse the response.body stream, decode the SSE data frames
(extracting the actual data: content), and assert on those decoded frames.
Remove or repurpose the sendFn mock and trackingSink tracking, and update all
event assertions throughout the test to verify the actual SSE output from
response.body rather than the disconnected context.send calls.
---
Duplicate comments:
In `@web/lib/__tests__/rate-limit-sliding-window.test.ts`:
- Line 7: The mock function executeRedisScript in the test file uses unknown[]
as the type for the args parameter, which does not match the real production
contract. Change the args parameter type from unknown[] to (string | number)[]
in the vi.fn() mock definition to ensure the test enforces the same input
constraints as the actual implementation and catches invalid argument
construction early.
In `@web/store/useChatStore.ts`:
- Around line 180-183: The guard condition in the readSSE callback within
useChatStore.ts currently allows events without a requestId to pass through and
be processed. To enforce strict request-scoped isolation, modify the condition
in the readSSE callback to reject events that either lack a requestId entirely
or have a requestId that does not match the clientMsgId. Change the guard from
checking if requestId exists and differs from clientMsgId to instead checking if
requestId is missing OR does not match clientMsgId, ensuring only properly
correlated events with matching requestIds are processed.
In `@worker/src/services/ValidationService.ts`:
- Around line 16-19: The targetCount clamping logic in the ValidationService.ts
file currently checks if expectedCount is greater than 0 before flooring, but
this allows values like 0.5 to pass the check and floor down to 0, making
validation trivially pass. Reorder the clamping operations to first floor the
expectedCount value (after verifying it is finite), then apply Math.max with 1
as the lower bound and Math.min with 11 as the upper bound to ensure targetCount
is always between 1 and 11 inclusive. This prevents targetCount from ever being
0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d95c57c3-1cc5-4c19-89ba-f2bc03171c5a
📥 Commits
Reviewing files that changed from the base of the PR and between 6972d25 and a36c9af.
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/lib/__tests__/useChatStore-race.test.ts` around lines 28 - 41, The
beforeEach function overwrites global.fetch with a mock but the afterEach
function does not restore it, causing the mock to persist across tests and
create cross-test pollution. Store the original global.fetch value before
assigning the mock in beforeEach, then restore the original value in the
afterEach function using vi.restoreAllMocks() or by explicitly reassigning the
saved original fetch.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Assert persistState in the stale-event test, not only activePersistRequestId.
This case currently validates only the active request id. Add a direct persistState assertion so a stale saved/failed mutation cannot slip through unnoticed.
Suggested fix
// Actually, the setPersistState guard should discard the req-A event
// because activePersistRequestId is now req-B
+ expect(useChatStore.getState().persistState).toBe('saving');
expect(useChatStore.getState().activePersistRequestId).toBe('req-B');
📝 Committable suggestion
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
// Request B's state should NOT be affected — it should still be 'saved'
// from the req-A event, but the activePersistRequestId should be req-B
// Actually, the setPersistState guard should discard the req-A event
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/lib/__tests__/useChatStore-race.test.ts` around lines 84 - 89, In the
test case that validates stale-event handling, add a direct assertion for the
persistState value alongside the existing activePersistRequestId assertion.
After the line checking that activePersistRequestId equals 'req-B', add another
expect statement to verify that useChatStore.getState().persistState is in the
correct state (likely 'saved' based on the test context) to ensure that stale
saved/failed mutations cannot bypass validation undetected.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Event assertions are disconnected from actual SSE output.
handleChatStream writes events to the returned Response stream, not to context.send. trackingSink therefore does not represent emitted SSE frames; parse response.body and assert on decoded data: frames instead.
Also applies to: 143-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@worker/src/__tests__/chat-stream-requestId.test.ts` around lines 64 - 66, The
test is currently mocking context.send via the sendFn function to track events
in trackingSink, but handleChatStream actually writes events to the returned
response.body stream as SSE frames. Instead of relying on trackingSink, parse
the response.body stream, decode the SSE data frames (extracting the actual
data: content), and assert on those decoded frames. Remove or repurpose the
sendFn mock and trackingSink tracking, and update all event assertions
throughout the test to verify the actual SSE output from response.body rather
than the disconnected context.send calls.
- useChatStore.ts: Add setPersistState('idle') to both early-return paths
in deliver() (job.assistant and !job.stream.url) to prevent persistState
from being permanently stuck at 'saving' after auto-promote removal.
- stream-status-tracker-fallback.test.ts: 4 tests for bundle fallback
state preservation (excluded vs included dimensions, global state).
- markdown-accumulator-fallback.test.ts: 4 tests for forced rebuild
semantics and dimension freshness tracking.
- env-placeholder.test.ts: 10 tests for isPlaceholder detection
(dummy, placeholder, stub, ci-build, legitimate values).
- stream-delta-backtick.test.ts: 5 tests for backtick preservation
in JSON strings (healJson + handleDelta patterns).
- indent-heuristic.test.ts: 4 tests for module-level dynamic import
detection (2-space, 4-space, tab, top-level).
- validation-boundary.test.ts: 4 tests for DIMENSION 1 vs 10 collision
and expectedCount clamping.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
`isPlaceholder` has a cyclomatic complexity of 7 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant `undefined` from function call
When an argument is omitted from a function call, it will default to undefined. It is therefore redundant to explicitly pass an undefined literal as the last argument.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected empty arrow function
Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected empty arrow function
Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected empty arrow function
Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected empty arrow function
Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected empty arrow function
Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/lib/__tests__/stream-delta-backtick.test.ts">
<violation number="1" location="web/lib/__tests__/stream-delta-backtick.test.ts:43">
P2: Two tests labeled as 'handleDelta' behavior never call handleDelta, creating a coverage gap for trailing-backtick stripping logic</violation>
</file>
<file name="web/lib/__tests__/markdown-accumulator-fallback.test.ts">
<violation number="1" location="web/lib/__tests__/markdown-accumulator-fallback.test.ts:106">
P2: Weak test assertion: expect(after2).toBeDefined() does not validate that the markdown actually updated when dimensions increased</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
The reason will be displayed to describe this comment to others. Learn more.
P2: Two tests labeled as 'handleDelta' behavior never call handleDelta, creating a coverage gap for trailing-backtick stripping logic
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/lib/__tests__/stream-delta-backtick.test.ts, line 43:
<comment>Two tests labeled as 'handleDelta' behavior never call handleDelta, creating a coverage gap for trailing-backtick stripping logic</comment>
<file context>
@@ -0,0 +1,64 @@
+ * This only strips complete markdown code fence closers, not stray backticks.
+ * We verify the behavior through the rawSink accumulation pattern.
+ */
+ it('handleDelta accumulates rawSink without stripping interior backticks', () => {
+ const handler = new StreamDeltaHandler();
+ // Simulate receiving a delta that contains backticks in JSON values
</file context>
The reason will be displayed to describe this comment to others. Learn more.
P2: Weak test assertion: expect(after2).toBeDefined() does not validate that the markdown actually updated when dimensions increased
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/lib/__tests__/markdown-accumulator-fallback.test.ts, line 106:
<comment>Weak test assertion: expect(after2).toBeDefined() does not validate that the markdown actually updated when dimensions increased</comment>
<file context>
@@ -0,0 +1,142 @@
+ const after2 = useAnalysisStore.getState().analysis?.analysis_markdown;
+
+ // The markdown should have been updated (at minimum, the version incremented)
+ expect(after2).toBeDefined();
+ });
+
</file context>
…tability
- ValidationService: regex word boundaries for DIMENSION headers, expectedCount clamped to [1,11]
- chat-stream: all 8 send() calls verified to include requestId in SSE events
- stream-delta-handler: backtick stripping changed from while to if (code fence boundary)
- rules.ts: indent heuristic tightened to === 0
- vitest.config: added worker test include path
- validation-boundary: 19 tests covering edge cases and substring collision
- chat-stream-requestId: rewritten to avoid dist/ resolution issues
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
The reason will be displayed to describe this comment to others. Learn more.
Found `async` function without any `await` expressions
A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant `undefined` from function call
When an argument is omitted from a function call, it will default to undefined. It is therefore redundant to explicitly pass an undefined literal as the last argument.
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/vitest.config.ts">
<violation number="1" location="web/vitest.config.ts:16">
P1: Explicit `test.include` silently excludes `web/app/auth/callback/route.test.ts` from test discovery</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
The reason will be displayed to describe this comment to others. Learn more.
P1: Explicit test.include silently excludes web/app/auth/callback/route.test.ts from test discovery
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/vitest.config.ts, line 16:
<comment>Explicit `test.include` silently excludes `web/app/auth/callback/route.test.ts` from test discovery</comment>
<file context>
@@ -6,10 +6,14 @@ export default defineConfig({
test: {
globals: true,
environment: 'node',
+ include: ['lib/__tests__/**/*.test.ts', '../worker/src/__tests__/**/*.test.ts'],
exclude: [...configDefaults.exclude, 'tests/**'],
},
</file context>
The reason will be displayed to describe this comment to others. Learn more.
Found `async` function without any `await` expressions
A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.github.com/u/9919?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.github.com/in/16372?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.github.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.github.com/in/1082092?s=60&v=4){kind=small}
Wave 5 Remediations
All Wave 5 work consolidated into a single clean branch with full test suite passing.
Changes
Persist Lifecycle (from PR #92)
Stream Adapter Fixes
Bug Fixes
Test Results
Related
fix/wave5-remediations→mainSummary by cubic
Fixes chat layout overlap and makes chat persistence race-safe with request-scoped SSE IDs and guarded state transitions. Hardens streaming (code-fence backtick handling, dimension-aware markdown) and validation (word-boundary DIMENSION matching, per-chunk thresholds), expands the quality engine with 11 rules, and keeps the test suite green.
Bug Fixes
pb-8inDashboardLayout.requestId; all SSE events include it; client tracksactivePersistRequestId, filters stale SSE; no auto-promote ondone; early returns reset toidle; timer won’t clear newer requests; error capture via@sentry/nextjsand@sentry/cloudflare.personaConfig/`knowledgeGraph`/`classification`/`monetizationVerdict` outside the bundle.validate12D(expectedCount)clamps to 1–11; adds word-boundary header matching to avoid DIMENSION 10 matching 1; UCIS regexes broadened (bold headers, lens tags, power quotes), checkmark filtering added, table pattern relaxed.clientEnvguards placeholderNEXT_PUBLIC_*vars; removed malformed node/edge console warnings inuseKnowledgeGraph.Refactors & Tests
stream-delta-handler,markdown-accumulator,stream-status-tracker.scripts/verify-quality-engine.ts; vitest config includes worker tests with source aliasing toworker/srcfor stability.Written for commit 31d7f55. Summary will update on new commits.
Summary by CodeRabbit