Merged
Conversation
Reviewer's GuideIntroduces a monorepo-aware CI planning script and GitHub Actions workflow that determine impacted JS, Python, and docs projects from git diffs, then run targeted builds/tests/lint/typechecks, including topological installation and checking of Python packages and their local dependencies. Sequence diagram for Python package CI execution per matrix entrysequenceDiagram
participant GH as GithubActions_python_job
participant RUNPY as ci_run_python_package_mjs
participant UTIL as ci_utils_mjs
participant PIP as python_pip
participant TOOLS as python_tools_build_ruff_mypy_pytest
GH->>RUNPY: node scripts/ci-run-python-package.mjs packageDir
RUNPY->>UTIL: discoverPythonPackages(repoRoot)
UTIL-->>RUNPY: python package manifests
RUNPY->>RUNPY: normalize names
RUNPY->>RUNPY: find targetManifest by dir
RUNPY->>UTIL: topologicalLocalDependencies(targetManifest.name, manifestsByName)
UTIL-->>RUNPY: ordered manifests
RUNPY->>RUNPY: split dependencyInstalls vs target
alt has local dependencyInstalls
RUNPY->>PIP: python -m pip install -e dep1 -e dep2 ...
PIP-->>RUNPY: dependencies installed
end
RUNPY->>PIP: python -m pip install -e target[dev or base]
PIP-->>RUNPY: target package installed
alt targetManifest.hasDevDependencyGroup is false
RUNPY->>PIP: python -m pip install build pytest pytest-asyncio pytest-cov mypy ruff
PIP-->>RUNPY: dev tools installed
end
RUNPY->>TOOLS: python -m build --wheel --sdist .
TOOLS-->>RUNPY: build artifacts
opt targetManifest.hasRuff
RUNPY->>TOOLS: python -m ruff check .
TOOLS-->>RUNPY: ruff complete
end
opt targetManifest.hasMypy
RUNPY->>TOOLS: python -m mypy .
TOOLS-->>RUNPY: mypy complete
end
opt targetManifest.hasPytest or tests dir exists
RUNPY->>TOOLS: python -m pytest .
TOOLS-->>RUNPY: tests complete
end
RUNPY-->>GH: JSON summary (package, path, dependencies, dryRun)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 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 |
Review Summary by QodoAdd monorepo-aware CI workflow with smart change detection
WalkthroughsDescription• Add monorepo-aware CI workflow for JavaScript, Python, and docs checks • Compute impacted packages from branch without assuming origin/dev and next/next consistency • Install local Python package dependencies in topological order before checks • Implement intelligent change detection to run only affected packages Diagramflowchart LR
A["Git Changes"] --> B["CI Plan Script"]
B --> C["Detect JS Changes"]
B --> D["Detect Python Changes"]
B --> E["Detect Docs Changes"]
C --> F["JavaScript Job"]
D --> G["Python Job"]
E --> H["Docs Job"]
F --> I["Build/Test/Lint/Typecheck"]
G --> J["Install Dependencies<br/>Topologically"]
J --> K["Run Checks"]
H --> L["Docs Checks"]
File Changes1. .github/workflows/ci.yml
|
Code Review by Qodo
1. Broken docs workspace commands
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The TOML parsing in
parsePyprojectManifest(e.g.,readTomlArray+extractQuotedStrings) assumes a very specific formatting (double-quoted strings, simple arrays, no inline tables), which may break on valid but differently formattedpyproject.tomlfiles; consider either using a TOML parser or loosening these assumptions (e.g., support single quotes and more general array syntax). - The Python dev tooling detection is tightly coupled to a single optional-dependency group named
dev; if a package uses different group names (e.g.,testorlint) or defines tools only in[tool.*]sections,hasDevDependencyGroup/hasPytest/hasMypy/hasRuffmay be inaccurate, so it could be worth making this detection more flexible or explicitly keyed on tool presence rather than a hard-coded group name.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The TOML parsing in `parsePyprojectManifest` (e.g., `readTomlArray` + `extractQuotedStrings`) assumes a very specific formatting (double-quoted strings, simple arrays, no inline tables), which may break on valid but differently formatted `pyproject.toml` files; consider either using a TOML parser or loosening these assumptions (e.g., support single quotes and more general array syntax).
- The Python dev tooling detection is tightly coupled to a single optional-dependency group named `dev`; if a package uses different group names (e.g., `test` or `lint`) or defines tools only in `[tool.*]` sections, `hasDevDependencyGroup`/`hasPytest`/`hasMypy`/`hasRuff` may be inaccurate, so it could be worth making this detection more flexible or explicitly keyed on tool presence rather than a hard-coded group name.
## Individual Comments
### Comment 1
<location path="scripts/ci-utils.mjs" line_range="6-13" />
<code_context>
+import { existsSync, readdirSync, readFileSync } from "node:fs";
+import path from "node:path";
+
+const IGNORED_DIRS = new Set([
+ ".git",
+ ".next",
+ ".turbo",
+ "build",
+ "coverage",
+ "dist",
+ "node_modules",
+]);
+
</code_context>
<issue_to_address>
**suggestion (performance):** Consider ignoring common Python artifact directories in the recursive file walk to avoid unnecessary IO.
`IGNORED_DIRS` currently covers JS build artifacts but not common Python ones such as `.venv`, `.mypy_cache`, `.pytest_cache`, `.ruff_cache`, and `__pycache__`. Since `walkFiles` only needs to find `pyproject.toml`, adding these would cut down unnecessary traversal and avoid walking into in-repo virtualenvs, especially in large repos.
</issue_to_address>
### Comment 2
<location path="scripts/ci-utils.mjs" line_range="52-42" />
<code_context>
+ });
+}
+
+function extractQuotedStrings(source) {
+ const values = [];
+ const matcher = /"([^"]+)"/g;
+ let match;
+
+ while ((match = matcher.exec(source)) !== null) {
+ values.push(match[1]);
+ }
+
+ return values;
+}
+
+function readTomlArray(lines, startIndex) {
+ let cursor = startIndex;
+ let combined = lines[cursor];
+
+ while (!combined.includes("]") && cursor + 1 < lines.length) {
+ cursor += 1;
+ combined += `\n${lines[cursor]}`;
+ }
+
+ return {
+ items: extractQuotedStrings(combined),
+ nextIndex: cursor + 1,
</code_context>
<issue_to_address>
**issue (bug_risk):** The ad‑hoc TOML parsing for pyproject dependencies is fairly brittle; consider hardening it or documenting the assumptions.
This pipeline currently only handles a narrow TOML subset (double-quoted strings, simple `key = value` lines, `[...]` arrays) and will misparse or ignore many valid `pyproject.toml` constructs (single quotes, inline tables, trailing comments, multi-line strings, etc.), which can silently drop dependencies from the graph.
If adding a TOML parser is out of scope, at least tighten the heuristics (e.g., explicitly ignoring lines with trailing comments or non-string values) so unexpected formats are skipped rather than producing incorrect dependency names.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+6
to
+13
| const IGNORED_DIRS = new Set([ | ||
| ".git", | ||
| ".next", | ||
| ".turbo", | ||
| "build", | ||
| "coverage", | ||
| "dist", | ||
| "node_modules", |
There was a problem hiding this comment.
suggestion (performance): Consider ignoring common Python artifact directories in the recursive file walk to avoid unnecessary IO.
IGNORED_DIRS currently covers JS build artifacts but not common Python ones such as .venv, .mypy_cache, .pytest_cache, .ruff_cache, and __pycache__. Since walkFiles only needs to find pyproject.toml, adding these would cut down unnecessary traversal and avoid walking into in-repo virtualenvs, especially in large repos.
| .filter((relativePath) => relativePath !== "package.json") | ||
| .map((relativePath) => { | ||
| const manifest = JSON.parse(readFileSync(path.join(rootDir, relativePath), "utf8")); | ||
| return { |
There was a problem hiding this comment.
issue (bug_risk): The ad‑hoc TOML parsing for pyproject dependencies is fairly brittle; consider hardening it or documenting the assumptions.
This pipeline currently only handles a narrow TOML subset (double-quoted strings, simple key = value lines, [...] arrays) and will misparse or ignore many valid pyproject.toml constructs (single quotes, inline tables, trailing comments, multi-line strings, etc.), which can silently drop dependencies from the graph.
If adding a TOML parser is out of scope, at least tighten the heuristics (e.g., explicitly ignoring lines with trailing comments or non-string values) so unexpected formats are skipped rather than producing incorrect dependency names.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
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="scripts/ci-utils.mjs">
<violation number="1" location="scripts/ci-utils.mjs:6">
P2: Missing Python-specific directories (`.venv`, `__pycache__`, `.mypy_cache`, `.pytest_cache`, `.ruff_cache`) from `IGNORED_DIRS`. Since `walkFiles` traverses the actual filesystem (not git), a local `.venv` directory would cause `discoverPythonPackages` to pick up `pyproject.toml` files from installed third-party packages, corrupting the dependency graph and CI matrix.</violation>
<violation number="2" location="scripts/ci-utils.mjs:184">
P2: Detect and fail on circular local dependencies instead of silently returning an invalid “topological” order.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const visited = new Set(); | ||
|
|
||
| function visit(name) { | ||
| if (visited.has(name)) { |
There was a problem hiding this comment.
P2: Detect and fail on circular local dependencies instead of silently returning an invalid “topological” order.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/ci-utils.mjs, line 184:
<comment>Detect and fail on circular local dependencies instead of silently returning an invalid “topological” order.</comment>
<file context>
@@ -0,0 +1,211 @@
+ const visited = new Set();
+
+ function visit(name) {
+ if (visited.has(name)) {
+ return;
+ }
</file context>
| import { existsSync, readdirSync, readFileSync } from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| const IGNORED_DIRS = new Set([ |
There was a problem hiding this comment.
P2: Missing Python-specific directories (.venv, __pycache__, .mypy_cache, .pytest_cache, .ruff_cache) from IGNORED_DIRS. Since walkFiles traverses the actual filesystem (not git), a local .venv directory would cause discoverPythonPackages to pick up pyproject.toml files from installed third-party packages, corrupting the dependency graph and CI matrix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/ci-utils.mjs, line 6:
<comment>Missing Python-specific directories (`.venv`, `__pycache__`, `.mypy_cache`, `.pytest_cache`, `.ruff_cache`) from `IGNORED_DIRS`. Since `walkFiles` traverses the actual filesystem (not git), a local `.venv` directory would cause `discoverPythonPackages` to pick up `pyproject.toml` files from installed third-party packages, corrupting the dependency graph and CI matrix.</comment>
<file context>
@@ -0,0 +1,211 @@
+import { existsSync, readdirSync, readFileSync } from "node:fs";
+import path from "node:path";
+
+const IGNORED_DIRS = new Set([
+ ".git",
+ ".next",
</file context>
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+185
to
+191
| - name: Docs checks | ||
| run: | | ||
| npm -w docs run build | ||
| npm -w docs run check:coverage | ||
| npm -w docs run check:content | ||
| npm -w docs run check:snippets | ||
| npm -w docs run check:links |
There was a problem hiding this comment.
1. Broken docs workspace commands 🐞 Bug ≡ Correctness
The docs job runs npm -w docs and several check:* scripts that are not defined by any existing docs workspace, so the job will fail whenever it is triggered. Additionally, there is no docs workspace declared in the repo’s root workspaces list, so npm -w docs cannot resolve a target.
Agent Prompt
### Issue description
The GitHub Actions `docs` job targets a non-existent workspace (`docs`) and runs `check:*` scripts that aren’t defined in the repo’s docs packages, causing the job to fail when triggered.
### Issue Context
- Root workspaces include `datafn/docs` and `searchfn/docs` packages (named `@datafn/docs` / `@searchfn/docs`).
- Neither of these packages defines `check:coverage`, `check:content`, `check:snippets`, or `check:links` scripts.
### Fix Focus Areas
- .github/workflows/ci.yml[176-191]
- package.json[6-27]
- datafn/docs/package.json[1-10]
- searchfn/docs/package.json[1-10]
### What to change
- Update the workflow to run docs checks against the actual workspace(s), e.g. `npm -w @datafn/docs run build` and/or `npm -w @searchfn/docs run build`.
- Either add the referenced `check:*` scripts to the targeted docs package(s) or remove/replace those commands with scripts that actually exist (e.g., only run `build` if that’s the intended guardrail).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+31
to
+37
| const DOCS_PATTERNS = [ | ||
| /^docs\//, | ||
| /^docsfn\//, | ||
| /^packages\/docs-theme\//, | ||
| /^scripts\/check-docs-/, | ||
| /^scripts\/docs-coverage-check\.mjs$/, | ||
| ]; |
There was a problem hiding this comment.
2. Docs changes never trigger 🐞 Bug ≡ Correctness
ci-plan.mjs only marks run_docs true for paths like docs/ and docsfn/, but the repo’s docs apps live under datafn/docs/ and searchfn/docs/, so docs edits won’t trigger the Docs Guardrails job. This creates a systematic coverage gap where docs changes bypass docs validation entirely.
Agent Prompt
### Issue description
Docs changes in this repo won’t set `run_docs=true` because `DOCS_PATTERNS` doesn’t include the actual docs app locations.
### Issue Context
The repo contains docs apps under `datafn/docs` and `searchfn/docs`, but `DOCS_PATTERNS` only covers `docs/`, `docsfn/`, and a few other paths.
### Fix Focus Areas
- scripts/ci-plan.mjs[31-37]
- datafn/docs/package.json[1-10]
- searchfn/docs/package.json[1-10]
### What to change
- Expand `DOCS_PATTERNS` to include the real docs directories (e.g., `^datafn/docs/` and `^searchfn/docs/`) and any other docs locations you intend to guard.
- Keep the patterns aligned with whatever docs job targets in `.github/workflows/ci.yml` to avoid drift.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+226
to
+233
| const jsPackages = discoverJsPackages(repoRoot); | ||
| const pythonPackages = discoverPythonPackages(repoRoot).map((manifest) => ({ | ||
| ...manifest, | ||
| name: manifest.name.toLowerCase(), | ||
| })); | ||
|
|
||
| const jsByDir = new Map(jsPackages.map((manifest) => [manifest.dir, manifest])); | ||
| const pythonByDir = new Map(pythonPackages.map((manifest) => [manifest.dir, manifest])); |
There was a problem hiding this comment.
3. Js plan mixes subrepos 🐞 Bug ≡ Correctness
ci-plan.mjs discovers JS packages by walking all package.json files in the entire repo, which includes separate subprojects like searchfn/* that have their own package-lock.json and are not included as root workspaces. The workflow then runs only a root-level npm ci and Turbo commands, so changes under those subprojects can lead to skipped checks or failing task runs due to missing install context.
Agent Prompt
### Issue description
The JS CI plan currently discovers *all* `package.json` files across the repository, including packages that are not part of the root workspace/install context (notably the `searchfn/*` subproject which has its own lockfile). The JS workflow job installs only root dependencies (`npm ci` at repo root) and then runs Turbo from the root, so changes in those subprojects won’t be handled correctly.
### Issue Context
- Root `package.json` workspaces don’t include `searchfn/*`.
- `searchfn/` has its own `package-lock.json` and multiple nested packages (e.g. `searchfn/core/package.json`).
### Fix Focus Areas
- scripts/ci-utils.mjs[37-49]
- scripts/ci-plan.mjs[226-233]
- .github/workflows/ci.yml[76-84]
- package.json[6-27]
### What to change
- Adjust `discoverJsPackages`/`ci-plan` to only include packages that are actually part of the root workspace graph (e.g., derive allowed package roots from the root `workspaces` globs and only collect manifests under those).
- OR, if subprojects like `searchfn/*` are intended to be checked, add explicit CI handling for them (e.g., a separate job that runs `npm ci` in `searchfn/` and runs that subproject’s build/test/lint/typecheck commands).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52f9a0167f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - name: Docs checks | ||
| run: | | ||
| npm -w docs run build |
There was a problem hiding this comment.
The docs job currently runs npm -w docs ..., but this repo has no docs workspace (the docs packages are @datafn/docs and @searchfn/docs), so the step fails immediately when triggered. I verified this in this repo by running npm -w docs run build, which returns No workspaces found: --workspace=docs; any change that sets run_docs=true will therefore fail CI regardless of code correctness.
Useful? React with 👍 / 👎.
Comment on lines
+31
to
+33
| const DOCS_PATTERNS = [ | ||
| /^docs\//, | ||
| /^docsfn\//, |
There was a problem hiding this comment.
The run_docs selector only matches docs/ and docsfn/, but this repository’s actual docs apps live under datafn/docs/ and searchfn/docs/ (both already exist in the tree). As a result, edits in those docs directories do not set run_docs=true, so the Docs Guardrails job is skipped for the primary docs surfaces the workflow is meant to protect.
Useful? React with 👍 / 👎.
Greptile SummaryAdds a monorepo-aware CI workflow that uses a planning step to compute impacted JS and Python packages from git diff, then runs targeted builds and checks. There is one parsing bug in
Confidence Score: 4/5Safe to merge after fixing the TOML array parser early-termination bug; no other logic issues found. One confirmed P1 bug: readTomlArray silently drops array entries when any dependency uses PEP 508 extras notation (e.g. ruff[format], boto3[crt]), breaking topological install order and tool-detection flags. All other CI logic across the workflow, plan script, and runner script is correct. scripts/ci-utils.mjs — readTomlArray function (lines 64–76) Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions
participant plan as plan job
participant js as javascript job
participant py as python job (matrix)
participant docs as docs job
GH->>plan: trigger (push / PR / manual)
plan->>plan: git fetch origin dev/next/main
plan->>plan: node scripts/ci-plan.mjs
plan-->>GH: outputs (run_js, python_matrix, run_docs, filters)
par if run_js == true
GH->>js: checkout + npm ci
js->>js: npx turbo run build [filters]
js->>js: npx turbo run typecheck [filters]
js->>js: npx turbo run test [filters]
js->>js: npx turbo run lint [filters]
and if run_python == true
GH->>py: checkout + pip install (per matrix entry)
py->>py: node ci-run-python-package.mjs path
py->>py: pip install local deps in topo order
py->>py: python -m build + ruff + mypy + pytest
and if run_docs == true
GH->>docs: checkout + npm ci
docs->>docs: npm -w docs run build + checks
end
Prompt To Fix All With AIThis is a comment left during a code review.
Path: scripts/ci-utils.mjs
Line: 64-76
Comment:
**`readTomlArray` stops early on `]` inside quoted strings**
The loop condition `!combined.includes("]")` halts as soon as any `]` character appears anywhere in `combined` — including inside a quoted PEP 508 specifier like `"ruff[format]"` or `"boto3[crt]"`. Every array entry after that line is silently dropped. A local monorepo package listed after a bracketed dep in `[project] dependencies` will never be discovered (breaking topological install order in `ci-run-python-package.mjs`), and entries in the `dev` optional-dependency group after a bracketed dep won't contribute to the `hasRuff`/`hasMypy`/`hasPytest` detection flags.
```suggestion
function readTomlArray(lines, startIndex) {
let cursor = startIndex;
let combined = lines[cursor];
while (cursor + 1 < lines.length) {
const unquoted = combined.replace(/"[^"]*"/g, '""');
if (unquoted.includes("]")) break;
cursor += 1;
combined += `\n${lines[cursor]}`;
}
return {
items: extractQuotedStrings(combined),
nextIndex: cursor + 1,
};
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "SF-28 add monorepo-aware CI workflow" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+64
to
+76
| function readTomlArray(lines, startIndex) { | ||
| let cursor = startIndex; | ||
| let combined = lines[cursor]; | ||
|
|
||
| while (!combined.includes("]") && cursor + 1 < lines.length) { | ||
| cursor += 1; | ||
| combined += `\n${lines[cursor]}`; | ||
| } | ||
|
|
||
| return { | ||
| items: extractQuotedStrings(combined), | ||
| nextIndex: cursor + 1, | ||
| }; |
There was a problem hiding this comment.
readTomlArray stops early on ] inside quoted strings
The loop condition !combined.includes("]") halts as soon as any ] character appears anywhere in combined — including inside a quoted PEP 508 specifier like "ruff[format]" or "boto3[crt]". Every array entry after that line is silently dropped. A local monorepo package listed after a bracketed dep in [project] dependencies will never be discovered (breaking topological install order in ci-run-python-package.mjs), and entries in the dev optional-dependency group after a bracketed dep won't contribute to the hasRuff/hasMypy/hasPytest detection flags.
Suggested change
| function readTomlArray(lines, startIndex) { | |
| let cursor = startIndex; | |
| let combined = lines[cursor]; | |
| while (!combined.includes("]") && cursor + 1 < lines.length) { | |
| cursor += 1; | |
| combined += `\n${lines[cursor]}`; | |
| } | |
| return { | |
| items: extractQuotedStrings(combined), | |
| nextIndex: cursor + 1, | |
| }; | |
| function readTomlArray(lines, startIndex) { | |
| let cursor = startIndex; | |
| let combined = lines[cursor]; | |
| while (cursor + 1 < lines.length) { | |
| const unquoted = combined.replace(/"[^"]*"/g, '""'); | |
| if (unquoted.includes("]")) break; | |
| cursor += 1; | |
| combined += `\n${lines[cursor]}`; | |
| } | |
| return { | |
| items: extractQuotedStrings(combined), | |
| nextIndex: cursor + 1, | |
| }; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/ci-utils.mjs
Line: 64-76
Comment:
**`readTomlArray` stops early on `]` inside quoted strings**
The loop condition `!combined.includes("]")` halts as soon as any `]` character appears anywhere in `combined` — including inside a quoted PEP 508 specifier like `"ruff[format]"` or `"boto3[crt]"`. Every array entry after that line is silently dropped. A local monorepo package listed after a bracketed dep in `[project] dependencies` will never be discovered (breaking topological install order in `ci-run-python-package.mjs`), and entries in the `dev` optional-dependency group after a bracketed dep won't contribute to the `hasRuff`/`hasMypy`/`hasPytest` detection flags.
```suggestion
function readTomlArray(lines, startIndex) {
let cursor = startIndex;
let combined = lines[cursor];
while (cursor + 1 < lines.length) {
const unquoted = combined.replace(/"[^"]*"/g, '""');
if (unquoted.includes("]")) break;
cursor += 1;
combined += `\n${lines[cursor]}`;
}
return {
items: extractQuotedStrings(combined),
nextIndex: cursor + 1,
};
}
```
How can I resolve this? If you propose a fix, please make it concise.
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
Validation
Summary by Sourcery
Introduce a monorepo-aware CI workflow that plans and runs targeted JavaScript, Python, and docs checks based on changed files.
New Features:
Enhancements:
Summary by cubic
Implements SF-28 by adding a monorepo‑aware CI that runs only impacted JavaScript, Python, and docs checks. Speeds up CI and installs Python local deps in topological order for correct builds.
.github/workflows/ci.ymlwith a Plan job that resolves a safe diff base and determines impacted JS, Python, anddocs.turbobuild/test/lint/typecheck with scoped--filtertargets; falls back to full runs on global changes.ruff,mypy,pytest, andbuildviascripts/ci-run-python-package.mjs.docsand runs guardrails (check:coverage,check:content,check:snippets,check:links).scripts/ci-plan.mjsandscripts/ci-utils.mjsto detect changed packages and compute filters without assumingorigin/devandorigin/nexthave the same projects.Written for commit 52f9a01. Summary will update on new commits.