feat: matrix rain layer, strict TS, and clsx consolidation#224
Open
webbertakken wants to merge 2 commits into
Open
feat: matrix rain layer, strict TS, and clsx consolidation#224webbertakken wants to merge 2 commits into
webbertakken wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughTyped and strict-mode changes plus minor runtime adjustments: added/strengthened TypeScript annotations, enabled "strict" in tsconfig, switched many components from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
static/layers/matrix/index.html (2)
326-326: Inconsistent variable declaration style.The rest of the file uses
letandconst, but line 326 usesvar. Consider usingletfor consistency.🔧 Suggested fix
- var alpha + let alpha🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/layers/matrix/index.html` at line 326, Replace the legacy var declaration for the variable "alpha" with a block-scoped let declaration to match the file's modern style (change "var alpha" to "let alpha") and verify its scope/usage in any surrounding functions or blocks (e.g., where "alpha" is read/assigned later) to ensure no hoisting-related behavior changes are introduced.
273-274: Consider adding a null check for canvas context.
getContext('2d')can returnnullin edge cases (e.g., context already acquired with different attributes). While unlikely for this standalone page, a guard would make it more robust.🛡️ Suggested defensive check
const canvas = document.getElementById('canvas') -const ctx = canvas.getContext('2d') +const ctx = canvas.getContext('2d') +if (!ctx) throw new Error('Could not get 2D context')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/layers/matrix/index.html` around lines 273 - 274, The canvas context retrieval can return null; update the code that calls document.getElementById('canvas') and canvas.getContext('2d') to defensively handle a null ctx: check that canvas is found and that const ctx = canvas.getContext('2d') is not null (or use a type guard), and if it is null, bail out gracefully (e.g., return early, throw a descriptive error, or log and disable drawing). Locate the canvas and ctx usage (variables named canvas and ctx) and add the null checks before any drawing calls so later code does not assume a non-null 2D context.notes/61-streamer/index.mdx (1)
5-12: Redundanttarget="_self"attribute.
target="_self"is the default behaviour for anchor elements and can be removed for cleaner markup.✨ Suggested simplification
-### <a href="/layers/matrix" target="_self">Matrix rain</a> +### <a href="/layers/matrix">Matrix rain</a> -<a href="/layers/matrix" target="_self"> +<a href="/layers/matrix"> <img src="/img/streamer/matrix.png" alt="Matrix rain" style={{ maxWidth: '250px', borderRadius: '8px' }} /> </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notes/61-streamer/index.mdx` around lines 5 - 12, Remove the redundant target="_self" attribute from the anchor elements that wrap the "Matrix rain" link/image (the <a href="/layers/matrix" target="_self"> occurrences) so the anchors rely on the default behavior; simply delete the target attribute from both anchor tags around the header and image.src/theme/PwaReloadPopup/index.tsx (1)
16-21: Add live-region semantics for assistive technologies.This update banner may not be announced reliably by screen readers without a live region. Please add
role="status"(orrole="alert") andaria-live="polite"on the container.Suggested patch
- <div className="alert alert--secondary" style={popupStyle}> + <div + className="alert alert--secondary" + style={popupStyle} + role="status" + aria-live="polite" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/theme/PwaReloadPopup/index.tsx` around lines 16 - 21, The popup container in PwaReloadPopup (the outer <div className="alert alert--secondary" style={popupStyle}>) lacks live-region semantics; modify that container to include role="status" (or role="alert") and aria-live="polite" so screen readers announce the update banner; update the JSX for the PwaReloadPopup component to add these attributes on the same div to provide proper live-region support for assistive technologies.src/hooks/useViewportHeight.spec.ts (1)
44-48: Consider adding cleanup forvisualViewportwhen set to undefined.Whilst the current approach works correctly, storing and restoring the original
window.visualViewportvalue (similar to howrestoreVisualViewportis used for mocked cases) would provide slightly better test isolation. This would prevent any potential state leakage between tests.♻️ Example restoration pattern
beforeEach(() => { // Store original value originalVisualViewport = window.visualViewport // ... existing setup }) afterEach(() => { // Restore original value if (originalVisualViewport !== undefined) { Object.defineProperty(window, 'visualViewport', { value: originalVisualViewport, writable: true, configurable: true, }) } // ... existing cleanup })That said, given Vitest's test isolation and the fact that tests explicitly set their required state, the current implementation is likely safe in practice.
Also applies to: 73-77, 114-118, 144-148, 166-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useViewportHeight.spec.ts` around lines 44 - 48, Store the current window.visualViewport in a local variable (e.g., originalVisualViewport) before tests set it to undefined, and restore it in afterEach by calling Object.defineProperty(window, 'visualViewport', { value: originalVisualViewport, writable: true, configurable: true }) when originalVisualViewport is not undefined; apply this pattern alongside the existing restoreVisualViewport logic for all places in useViewportHeight.spec.ts that set window.visualViewport to undefined (the blocks around the undefined assignments and the mocked restore helpers).src/filetypes.d.ts (1)
13-18: Create minimal interfaces for@garmin-fit/sdkimports to restore type safety.Lines 15–17 declare
DecoderandStreamasany, erasing type information for downstream usage insrc/domain/diving/garmin/GarminFiles.ts. The@garmin-fit/sdkpackage provides no official TypeScript definitions, but the actual API surface is small and well-defined:Stream.fromByteArray(),Decoderconstructor,decoder.isFIT(),decoder.checkIntegrity(), anddecoder.read()with specific option and return shape. Creating minimal interfaces for these members would restore compile-time safety without significant effort.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/filetypes.d.ts` around lines 13 - 18, The current declaration declares Decoder and Stream as any, which removes type safety used in GarminFiles.ts; replace these with minimal TS interfaces by declaring Stream with a static fromByteArray(bytes: Uint8Array): Stream and any instance methods used, and declare Decoder as a class/constructor type (export class Decoder) whose instances expose isFIT(): boolean, checkIntegrity(): boolean, and read(options?: { stopOnError?: boolean }): { messages: unknown[]; errors?: unknown[] } (adjust result shape to match actual usage in GarminFiles.ts); update the module declaration to export these types so usages of Stream.fromByteArray, new Decoder(...), decoder.isFIT(), decoder.checkIntegrity(), and decoder.read(...) have proper compile-time types.src/components/common/CodeBlock.tsx (1)
12-17: Preserve incomingclassNamein span wrappers.Lines 13 and 16 overwrite
props.className. Merging keeps these helpers composable.Suggested tweak
-const LineNumber = (props: React.HTMLAttributes<HTMLSpanElement>) => ( - <span {...props} className={styles.number} /> -) -const LineContent = (props: React.HTMLAttributes<HTMLSpanElement>) => ( - <span {...props} className={styles.content} /> -) +const LineNumber = ({ className, ...props }: React.HTMLAttributes<HTMLSpanElement>) => ( + <span {...props} className={cx(className, styles.number)} /> +) +const LineContent = ({ className, ...props }: React.HTMLAttributes<HTMLSpanElement>) => ( + <span {...props} className={cx(className, styles.content)} /> +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/CodeBlock.tsx` around lines 12 - 17, LineNumber and LineContent currently overwrite any incoming props.className which breaks composability; update both components to merge props.className with the local style (styles.number and styles.content) instead of replacing it. In the LineNumber and LineContent functions, construct a combined className (via template string or utility like clsx) that includes props.className and the module class, pass that into the span along with the rest of props (ensure you don't spread className twice), and keep the prop types and spread usage intact so external classes are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/tools/PullRequestsToReleaseText/index.tsx`:
- Line 32: The component PullRequestsToReleaseText currently sets
excludedContributors to an empty array via const [excludedContributors] =
useState<string[]>([]), which effectively disables the exclusion filter; change
the initialization so excludedContributors is derived from the intended source
(e.g., a prop like props.excludedContributors, a context selector, or persisted
storage/localStorage) and keep it as state if it needs to be updated (e.g.,
useState<string[]>(initialExcludedContributors)), or remove state entirely and
use the sourced array directly where the exclusion branch (referenced as
excludedContributors and the exclusion logic at the filter near lines 65-66)
runs, ensuring the exclusion array reflects the real persisted/prop value rather
than always being [].
In `@src/filetypes.d.ts`:
- Around line 1-11: Remove the custom ambient declaration for "node-fetch" (the
declare module 'node-fetch' block in src/filetypes.d.ts) and instead add the
official type package by installing `@types/node-fetch` as a devDependency; ensure
existing imports that use the default export (`fetch`) continue to compile
against the installed types (node-fetch v2's `@types` provides the correct
Response surface including .json(), .text(), .headers, etc.), and delete the
filetypes.d.ts declaration to avoid the narrowed Response type shadowing the
real library API.
In `@src/global.css`:
- Line 9: The `@source` exclusion in src/global.css currently uses "@source not
"../../*.md";" which points above the repo root; update the exclusion to
"@source not "../*.md";" so repository-root Markdown files are correctly
excluded by Tailwind's Oxide scanner — locate the `@source` rule in src/global.css
and replace the "../../*.md" pattern with "../*.md".
---
Nitpick comments:
In `@notes/61-streamer/index.mdx`:
- Around line 5-12: Remove the redundant target="_self" attribute from the
anchor elements that wrap the "Matrix rain" link/image (the <a
href="/layers/matrix" target="_self"> occurrences) so the anchors rely on the
default behavior; simply delete the target attribute from both anchor tags
around the header and image.
In `@src/components/common/CodeBlock.tsx`:
- Around line 12-17: LineNumber and LineContent currently overwrite any incoming
props.className which breaks composability; update both components to merge
props.className with the local style (styles.number and styles.content) instead
of replacing it. In the LineNumber and LineContent functions, construct a
combined className (via template string or utility like clsx) that includes
props.className and the module class, pass that into the span along with the
rest of props (ensure you don't spread className twice), and keep the prop types
and spread usage intact so external classes are preserved.
In `@src/filetypes.d.ts`:
- Around line 13-18: The current declaration declares Decoder and Stream as any,
which removes type safety used in GarminFiles.ts; replace these with minimal TS
interfaces by declaring Stream with a static fromByteArray(bytes: Uint8Array):
Stream and any instance methods used, and declare Decoder as a class/constructor
type (export class Decoder) whose instances expose isFIT(): boolean,
checkIntegrity(): boolean, and read(options?: { stopOnError?: boolean }): {
messages: unknown[]; errors?: unknown[] } (adjust result shape to match actual
usage in GarminFiles.ts); update the module declaration to export these types so
usages of Stream.fromByteArray, new Decoder(...), decoder.isFIT(),
decoder.checkIntegrity(), and decoder.read(...) have proper compile-time types.
In `@src/hooks/useViewportHeight.spec.ts`:
- Around line 44-48: Store the current window.visualViewport in a local variable
(e.g., originalVisualViewport) before tests set it to undefined, and restore it
in afterEach by calling Object.defineProperty(window, 'visualViewport', { value:
originalVisualViewport, writable: true, configurable: true }) when
originalVisualViewport is not undefined; apply this pattern alongside the
existing restoreVisualViewport logic for all places in useViewportHeight.spec.ts
that set window.visualViewport to undefined (the blocks around the undefined
assignments and the mocked restore helpers).
In `@src/theme/PwaReloadPopup/index.tsx`:
- Around line 16-21: The popup container in PwaReloadPopup (the outer <div
className="alert alert--secondary" style={popupStyle}>) lacks live-region
semantics; modify that container to include role="status" (or role="alert") and
aria-live="polite" so screen readers announce the update banner; update the JSX
for the PwaReloadPopup component to add these attributes on the same div to
provide proper live-region support for assistive technologies.
In `@static/layers/matrix/index.html`:
- Line 326: Replace the legacy var declaration for the variable "alpha" with a
block-scoped let declaration to match the file's modern style (change "var
alpha" to "let alpha") and verify its scope/usage in any surrounding functions
or blocks (e.g., where "alpha" is read/assigned later) to ensure no
hoisting-related behavior changes are introduced.
- Around line 273-274: The canvas context retrieval can return null; update the
code that calls document.getElementById('canvas') and canvas.getContext('2d') to
defensively handle a null ctx: check that canvas is found and that const ctx =
canvas.getContext('2d') is not null (or use a type guard), and if it is null,
bail out gracefully (e.g., return early, throw a descriptive error, or log and
disable drawing). Locate the canvas and ctx usage (variables named canvas and
ctx) and add the null checks before any drawing calls so later code does not
assume a non-null 2D context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 670c3f78-642b-415b-b14a-d6025c7766df
⛔ Files ignored due to path filters (2)
static/img/streamer/matrix.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
.gitignoredocusaurus.config.tsnotes/61-streamer/_category_.yamlnotes/61-streamer/index.mdxpackage.jsonsrc/components/Home/Card/Card.tsxsrc/components/Modal/Modal.tsxsrc/components/common/CodeBlock.tsxsrc/components/common/CodeEditor.tsxsrc/components/tools/CommaSeparator/index.tsxsrc/components/tools/GarminToSsiDiveLogHelper/index.tsxsrc/components/tools/PullRequestsToReleaseText/index.tsxsrc/components/tools/TasmotaHelper/calibrateWithEnergyMeter.tsxsrc/components/tools/TasmotaHelper/calibrateWithMultiMeter.tsxsrc/components/tools/TextAnalyser/index.tsxsrc/core/hooks/useCookie.tssrc/core/utils/base64.tssrc/core/utils/md5.tssrc/core/utils/sha256.tssrc/domain/diving/garmin/GarminDive.tssrc/domain/diving/ssi/SsiDive.tssrc/filetypes.d.tssrc/global.csssrc/hooks/useViewportHeight.spec.tssrc/theme/IdealImage/index.tsxsrc/theme/PwaReloadPopup/index.tsxsrc/theme/ToolPage/ToolPageLayout.tsxstatic/layers/matrix/index.htmltsconfig.json
| const cookie = useCookie('release-text-generator-excluded-contributors', { expires: 10 * 365 }) | ||
| const [result, setResult] = useState<string>('') | ||
| const [excludedContributors] = useState<string[]>(cookie.getValue() || []) | ||
| const [excludedContributors] = useState<string[]>([]) |
Contributor
There was a problem hiding this comment.
Potential regression: contributor exclusions are now always empty.
Line 32 hardcodes excludedContributors to [], so the exclusion branch at Line 65 never filters anyone unless another update path exists. If this removal was unintentional, restore a persisted/source-backed value.
Possible minimal fix if exclusions are intentionally static
- const [excludedContributors] = useState<string[]>([])
+ const excludedContributors: string[] = []Also applies to: 65-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/tools/PullRequestsToReleaseText/index.tsx` at line 32, The
component PullRequestsToReleaseText currently sets excludedContributors to an
empty array via const [excludedContributors] = useState<string[]>([]), which
effectively disables the exclusion filter; change the initialization so
excludedContributors is derived from the intended source (e.g., a prop like
props.excludedContributors, a context selector, or persisted
storage/localStorage) and keep it as state if it needs to be updated (e.g.,
useState<string[]>(initialExcludedContributors)), or remove state entirely and
use the sourced array directly where the exclusion branch (referenced as
excludedContributors and the exclusion logic at the filter near lines 65-66)
runs, ensuring the exclusion array reflects the real persisted/prop value rather
than always being [].
Comment on lines
+1
to
+11
| declare module 'node-fetch' { | ||
| function fetch( | ||
| url: string, | ||
| init?: Record<string, unknown>, | ||
| ): Promise<{ | ||
| ok: boolean | ||
| statusText: string | ||
| arrayBuffer(): Promise<ArrayBuffer> | ||
| }> | ||
| export default fetch | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== node-fetch dependency footprint =="
rg -n '"node-fetch"|@types/node-fetch' package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
echo "== node-fetch imports =="
rg -nP --type=ts --type=tsx -C2 "from ['\"]node-fetch['\"]|require\(['\"]node-fetch['\"]\)" || trueRepository: webbertakken/takken.io
Length of output: 212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check `@types/node-fetch` in package.json =="
rg '@types/node-fetch' package.json package-lock.json 2>/dev/null || echo "(not found)"
echo "== Search all files for node-fetch usage =="
rg -i 'node-fetch' --type=js --type=json -C2 2>/dev/null || echo "(no matches in js/json)"
echo "== Check tsconfig or similar config files =="
fd -e tsconfig.json -e tsconfig.*.json --exec cat {} \; 2>/dev/null | head -50
echo "== List .d.ts files in src =="
fd -e d.ts src/ 2>/dev/null | head -20Repository: webbertakken/takken.io
Length of output: 449
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find filetypes.d.ts file =="
find . -name "filetypes.d.ts" 2>/dev/null
echo "== Check if file exists at src/filetypes.d.ts =="
test -f src/filetypes.d.ts && echo "File exists" || echo "File does not exist"
echo "== Broad search for node-fetch mentions =="
rg 'node-fetch' --type-list | head -5
rg 'node-fetch' -t ts -t js -t json --max-count=20 2>/dev/null || echo "(no results)"
echo "== Check git status for new files =="
git ls-files | grep -i filetype || echo "(no filetype files in git)"Repository: webbertakken/takken.io
Length of output: 628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== View filetypes.d.ts file =="
cat src/filetypes.d.ts
echo ""
echo "== Search for any fetch usage in codebase =="
rg 'fetch\s*\(' --type ts --type js -C1 | head -30 || echo "(no fetch calls found)"
echo ""
echo "== Check if node-fetch is actually imported/used =="
rg "from.*node-fetch|require.*node-fetch|import.*node-fetch" -C2 || echo "(no node-fetch imports)"
echo ""
echo "== Check package.json and dependencies =="
head -80 package.json | tail -30Repository: webbertakken/takken.io
Length of output: 3784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== View frontmatter-image-processor.ts =="
cat src/plugins/frontmatter-image-processor.ts
echo ""
echo "== Check node-fetch@2 actual types =="
cat node_modules/node-fetch/package.json 2>/dev/null | grep -A5 '"typings"\|"types"\|"main"' || echo "(node_modules not available)"
echo ""
echo "== Check if statusText is used anywhere =="
rg 'statusText' || echo "(statusText not used)"Repository: webbertakken/takken.io
Length of output: 6936
Use @types/node-fetch instead of custom ambient declaration.
The custom node-fetch module declaration (lines 1–11) narrows the response type, hiding the full API surface. For node-fetch@2, the standard approach is to install @types/node-fetch rather than maintain a bespoke ambient module. Although the current narrowed type (ok, statusText, arrayBuffer()) covers existing usage in src/plugins/frontmatter-image-processor.ts, it creates a maintenance burden: future code attempting to use other Response properties (e.g., .json(), .text(), .headers) would silently diverge from the real library and lose strict-mode type safety.
Remove this declaration and install @types/node-fetch as a dev dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/filetypes.d.ts` around lines 1 - 11, Remove the custom ambient
declaration for "node-fetch" (the declare module 'node-fetch' block in
src/filetypes.d.ts) and instead add the official type package by installing
`@types/node-fetch` as a devDependency; ensure existing imports that use the
default export (`fetch`) continue to compile against the installed types
(node-fetch v2's `@types` provides the correct Response surface including .json(),
.text(), .headers, etc.), and delete the filetypes.d.ts declaration to avoid the
narrowed Response type shadowing the real library API.
| Files like ALL_PROMPTS.md contain UUIDs with hex-like substrings (e.g. 2e33f8) | ||
| that Tailwind v4's Oxide scanner misinterprets as colour values, causing | ||
| "RangeError: Invalid code point" crashes. */ | ||
| @source not "../../*.md"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Tailwind CSS v4, are @sourcepaths resolved relative to the CSS file containing the directive, and would@source not "../*.md";correctly exclude repository-root Markdown files when used insrc/global.css?
💡 Result:
Yes, in Tailwind CSS v4, @source paths are resolved relative to the CSS file containing the directive. For a CSS file at src/global.css, @source not "../.md"; would correctly exclude Markdown files at the repository root (.md), as ../*.md is the relative path from src/ to the root directory matching *.md files.
Citations:
- 1: Add
@sourcesupport tailwindlabs/tailwindcss#14078 - 2: https://tailwindcss.com/docs/detecting-classes-in-source-files
- 3: Add
@source notsupport tailwindlabs/tailwindcss#17255 - 4: https://tailwindcss.com/docs/content-configuration
Fix @source exclusion path depth
Line 9 uses @source not "../../*.md"; which resolves to above the repository root, so root-level Markdown files will not be excluded. Change to @source not "../*.md"; to correctly exclude repository-root Markdown files from Tailwind's Oxide scanner.
Proposed fix
-@source not "../../*.md";
+@source not "../*.md";📝 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
| @source not "../../*.md"; | |
| `@source` not "../*.md"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/global.css` at line 9, The `@source` exclusion in src/global.css currently
uses "@source not "../../*.md";" which points above the repo root; update the
exclusion to "@source not "../*.md";" so repository-root Markdown files are
correctly excluded by Tailwind's Oxide scanner — locate the `@source` rule in
src/global.css and replace the "../../*.md" pattern with "../*.md".
Replace all `import cx from 'classnames'` with `import cx from 'clsx'` across 10 files and remove the classnames dependency from package.json. clsx is lighter, faster, and already the Docusaurus ecosystem standard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enable `strict: true` in tsconfig.json and fix all 90 type errors: - Add type annotations to utility functions (md5, base64, sha256) - Add prop interfaces to React components (Card, CodeBlock, CodeEditor) - Add null guards and narrowing in tool components - Add definite assignment assertions to SsiDive model properties - Fix delete operator usage in useViewportHeight tests - Add type declarations for untyped packages (garmin-fit/sdk, node-fetch) - Install @types/js-cookie for proper cookie typing - Fix loose equality (!=) to strict equality (!==) in base64.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
webbertakken
force-pushed
the
feat/matrix-rain-layer
branch
from
0b21230 to
78e62a6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.local.json:
- Around line 55-57: The .claude/settings.local.json change failed Prettier; run
the formatter and commit the result: run yarn prettier --write
.claude/settings.local.json (or yarn prettier --write . to format all), ensure
the "disabledMcpjsonServers" array formatting matches project Prettier rules,
verify git diff is clean, and commit the formatted file.
- Around line 47-51: The added broad shell permissions "Bash(git add:*)" and
"Bash(git rebase:*)" are too permissive; narrow them to least-privilege by
replacing those wildcard entries with specific allowed command shapes (e.g.,
explicit file/path patterns for git add and explicit safe subcommands/flags for
git rebase, and keep the specific "GIT_EDITOR=true git rebase --continue" entry
if needed), or remove these entries and keep this local settings file out of
source control; update the entries that reference "Bash(git add:*)", "Bash(git
rebase:*)", and "Bash(GIT_EDITOR=true git rebase --continue)" accordingly so
only the exact safe commands remain permitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7acc2f5-96e4-4d86-bfa8-715d3a78f2f5
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.claude/settings.local.jsondocusaurus.config.tsecosystem.config.cjspackage.jsonsrc/components/Home/Card/Card.tsxsrc/components/Modal/Modal.tsxsrc/components/common/CodeBlock.tsxsrc/components/common/CodeEditor.tsxsrc/components/tools/CommaSeparator/index.tsxsrc/components/tools/GarminToSsiDiveLogHelper/index.tsxsrc/components/tools/PullRequestsToReleaseText/index.tsxsrc/components/tools/TasmotaHelper/calibrateWithEnergyMeter.tsxsrc/components/tools/TasmotaHelper/calibrateWithMultiMeter.tsxsrc/components/tools/TextAnalyser/index.tsxsrc/core/hooks/useCookie.tssrc/core/utils/base64.tssrc/core/utils/md5.tssrc/core/utils/sha256.tssrc/domain/diving/garmin/GarminDive.tssrc/domain/diving/ssi/SsiDive.tssrc/filetypes.d.tssrc/hooks/useViewportHeight.spec.tssrc/theme/IdealImage/index.tsxsrc/theme/ToolPage/ToolPageLayout.tsxtsconfig.json
✅ Files skipped from review due to trivial changes (15)
- src/components/Modal/Modal.tsx
- src/components/tools/TasmotaHelper/calibrateWithMultiMeter.tsx
- src/components/tools/TasmotaHelper/calibrateWithEnergyMeter.tsx
- src/theme/ToolPage/ToolPageLayout.tsx
- docusaurus.config.ts
- src/core/utils/sha256.ts
- src/components/common/CodeEditor.tsx
- ecosystem.config.cjs
- tsconfig.json
- package.json
- src/hooks/useViewportHeight.spec.ts
- src/components/Home/Card/Card.tsx
- src/filetypes.d.ts
- src/theme/IdealImage/index.tsx
- src/components/tools/GarminToSsiDiveLogHelper/index.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/tools/CommaSeparator/index.tsx
- src/domain/diving/garmin/GarminDive.ts
- src/components/common/CodeBlock.tsx
- src/core/hooks/useCookie.ts
- src/core/utils/base64.ts
- src/domain/diving/ssi/SsiDive.ts
Comment on lines
+47
to
+51
| "Bash(openspec --version:*)", | ||
| "Bash(yarn install:*)", | ||
| "Bash(git add:*)", | ||
| "Bash(git rebase:*)", | ||
| "Bash(GIT_EDITOR=true git rebase --continue)" |
Contributor
There was a problem hiding this comment.
Tighten newly added shell permissions to least-privilege.
From Line 49 to Line 51, the new git add:* and git rebase:* allowances are very broad and permit arbitrary staging/history rewrites by the agent. Please scope these to explicit safe command shapes (or keep this local settings file untracked) to avoid privilege creep.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.local.json around lines 47 - 51, The added broad shell
permissions "Bash(git add:*)" and "Bash(git rebase:*)" are too permissive;
narrow them to least-privilege by replacing those wildcard entries with specific
allowed command shapes (e.g., explicit file/path patterns for git add and
explicit safe subcommands/flags for git rebase, and keep the specific
"GIT_EDITOR=true git rebase --continue" entry if needed), or remove these
entries and keep this local settings file out of source control; update the
entries that reference "Bash(git add:*)", "Bash(git rebase:*)", and
"Bash(GIT_EDITOR=true git rebase --continue)" accordingly so only the exact safe
commands remain permitted.
Comment on lines
+55
to
+57
| "disabledMcpjsonServers": [ | ||
| "github" | ||
| ] |
Contributor
There was a problem hiding this comment.
Prettier is still failing for this file.
The change around Line 55 to Line 57 did not satisfy formatting checks. Please run yarn prettier --write .claude/settings.local.json and commit the formatted result.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.local.json around lines 55 - 57, The
.claude/settings.local.json change failed Prettier; run the formatter and commit
the result: run yarn prettier --write .claude/settings.local.json (or yarn
prettier --write . to format all), ensure the "disabledMcpjsonServers" array
formatting matches project Prettier rules, verify git diff is clean, and commit
the formatted file.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
strict: trueand fixed all 90 type errors across ~20 files (type annotations, null guards, definite assignments, declaration files)classnameswithclsxacross 10 files, removed the redundant dependencyonBrokenMarkdownLinkstomarkdown.hooks, formattedmatrix.htmlwith PrettierTest plan
yarn typecheckpasses with strict modeyarn lint:jspassesyarn test --run— all 47 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements
Chores