Skip to content

fix: pipecat bot.py detection, OTel dep injection, correct SIP sim ID path, and asyncio-free LiveKit wiring#8

Merged
callumreid merged 7 commits into
mainfrom
fix/sync-check-sim-id
Mar 31, 2026
Merged

fix: pipecat bot.py detection, OTel dep injection, correct SIP sim ID path, and asyncio-free LiveKit wiring#8
callumreid merged 7 commits into
mainfrom
fix/sync-check-sim-id

Conversation

@callumreid

@callumreid callumreid commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Pipecat entry-point detection: findEntryPoint is now framework-aware; pipecat projects prefer bot.py over agent.py (matching Pipecat Cloud conventions)
  • OTel dep injection (addOtelDeps): Appends opentelemetry-api, opentelemetry-sdk, and opentelemetry-exporter-otlp-proto-http to requirements.txt (preferred for Docker deployments) or pyproject.toml; fixes closing-bracket regex to avoid false matches inside package extras like pipecat-ai[daily,openai]; ensures trailing comma on last dep before TOML injection
  • Pipecat SIP sim ID path: Corrected prompt to extract sim ID from body.dialin_settings.sip_headers.X-Coval-Simulation-Id (was body.dialin_settings.custom_context.coval_simulation_id)
  • LiveKit wiring: _check_sim_id is a plain def (not async def) so it works directly as a LiveKit event handler without asyncio.ensure_future

Test plan

  • Run wizard on a Pipecat agent project with both bot.py and agent.py present — wizard picks bot.py
  • Run wizard on a project using pyproject.toml — OTel packages injected into dependencies array with correct TOML syntax
  • Run wizard on a project using requirements.txt — OTel packages appended
  • Run a Pipecat sim — sim ID extracted from SIP headers and spans appear in Coval
  • Run a LiveKit sim — sim ID received via participant attributes and spans appear in Coval
  • npm test — 75 tests pass

@coderabbitai

coderabbitai Bot commented Mar 10, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Converted LiveKit helper _check_sim_id from asynchronous to synchronous and adjusted event bindings in src/prompts.ts. Added PIPECAT_ENTRY_POINT_NAMES and OTEL_PACKAGES constants in src/constants.ts. findEntryPoint now accepts an optional framework parameter and src/detect.ts passes the detected framework into it. Implemented addOtelDeps(dir, projectFile) in src/files.ts and invoked it from src/index.ts, logging any added OTel packages. Added Jest tests for framework-specific entry-point selection and end-to-end detection.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits v1.0.0 format with 'fix' type and accurately summarizes all three main changes: Pipecat bot.py detection, OTel dependency injection, and asyncio-free LiveKit wiring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sync-check-sim-id

Comment @coderabbitai help to get the list of available commands and usage tips.

- findEntryPoint now accepts optional framework param; pipecat uses
  bot.py-first order (Pipecat Cloud convention) to avoid picking a
  co-located agent.py over the real cloud entry point
- addOtelDeps() in files.ts writes missing opentelemetry-* packages
  to requirements.txt or pyproject.toml after wizard writes its files
- index.ts calls addOtelDeps and logs which packages were added
- detect.test.ts: 5 new tests covering pipecat/livekit priority and
  the full-pipeline pipecat case

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/files.ts`:
- Around line 63-76: The function currently returns the full missing array even
when no file was modified; update the logic around projectFile, the
pyproject.toml regex replace, and the Pipfile/setup.py cases so you only return
packages that were actually written: for 'requirements.txt' return the appended
entries (or [] if none appended), for 'pyproject.toml' detect when the regex did
not match (no [project].dependencies block) and return [] (or write the
additions only when updated !== content), and for unsupported manifests listed
in PROJECT_FILES such as 'Pipfile' and 'setup.py' either throw a clear
unsupported-manifest error or return [] instead of returning missing; reference
projectFile, missing, updated, content, and writeFileSync to locate and change
the behavior so src/index.ts no longer reports success when nothing changed.
- Around line 65-73: The pyproject.toml string-replace in the block handling
projectFile === 'pyproject.toml' can produce invalid TOML and mis-handle
bracketed package names; instead parse the TOML, update the project.dependencies
(or tool.poetry.dependencies) array/object structurally by merging in the
missing entries, then serialize back to TOML and write via
writeFileSync(filePath, updated, 'utf-8'); modify the code around
content.replace/updated to use a TOML parser/serializer and operate on the
dependencies node rather than a regex so additions are correctly comma-separated
and bracket-safe, referencing the existing variables projectFile, content,
missing, updated, filePath, and writeFileSync.
- Around line 56-59: The current check derives package names with
pkg.split('>=')[0] and searches the raw file string, which is brittle; instead
parse the package manifest and compare against actual dependency keys:
JSON.parse(content) to get
dependencies/devDependencies/optionalDependencies/peerDependencies, build a
single set of installed package names, then compute missing =
OTEL_PACKAGES.filter(pkg => { const name = typeof pkg === 'string' ?
pkg.split(/[@<>=~^]/)[0] : pkg.name; return !installedDepsSet.has(name); });
update the code that computes missing to use this parsed dependency-key
comparison and reference OTEL_PACKAGES, missing, and content/installedDepsSet
accordingly.

In `@src/index.ts`:
- Around line 119-122: The code currently calls addOtelDeps(targetDir,
detection.projectFile) and mutates the dependency manifest
(detection.projectFile) without showing it in the preview or creating a backup;
modify the flow so that addOtelDeps returns both the list of addedDeps and a
manifestChange object (or a diff string), include that manifestChange in the
preview output shown to the user, and before applying changes create a backup
copy of detection.projectFile (e.g., detection.projectFile + ".bak") so the CLI
can restore on cancel/failure; update the success log in the block using
addedDeps to reflect that the manifest was updated and ensure any write to
detection.projectFile is guarded behind the user-approved preview and backup
step.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ce11ab02-28c5-4f28-a1f8-5a024363cf79

📥 Commits

Reviewing files that changed from the base of the PR and between 48937d9 and 7a114df.

📒 Files selected for processing (5)
  • src/__tests__/detect.test.ts
  • src/constants.ts
  • src/detect.ts
  • src/files.ts
  • src/index.ts

Comment thread src/files.ts
Comment thread src/files.ts Outdated
Comment thread src/files.ts Outdated
Comment thread src/index.ts
…p injection

The previous regex used lazy [\s\S]*?(\]) which matched the first ] found,
breaking when packages have extras like pipecat-ai[daily,openai]>=0.0.60.
Now matches \n] so only the line-leading closing bracket is targeted.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (4)
src/files.ts (4)

47-52: ⚠️ Potential issue | 🟠 Major

JSDoc is misleading—"added" vs "missing" are not the same.

The docstring claims it returns "packages that were added," but the function returns missing regardless of whether anything was written. For Pipfile or setup.py, the function returns the missing list but writes nothing. Callers (like src/index.ts) then log a success message for packages that weren't actually added.

Either fix the return semantics (return only what was actually written) or update the JSDoc to reflect reality.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 47 - 52, The JSDoc for addOtelDeps is misleading
because the function currently returns the computed `missing` packages even when
it doesn't write anything (e.g., Pipfile/setup.py); update addOtelDeps to return
only the packages that were actually written to disk: compute `missing`, perform
writes only for supported file types (requirements.txt and pyproject.toml via
the existing write/patch logic), and set the return value to the subset that was
persisted (empty array if no write occurred), keeping the function name and
caller expectations (e.g., src/index.ts) intact so callers only log added
packages when a write actually happened.

56-59: ⚠️ Potential issue | 🟠 Major

Raw substring matching is unreliable.

content.includes(name) matches anywhere in the file—comments, section headers, partial package names. A comment # TODO: add opentelemetry-api would falsely indicate the package is present.

Parse the dependency structure properly for each file format.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 56 - 59, The current check builds missing from
OTEL_PACKAGES by doing a raw substring match on content (see OTEL_PACKAGES,
missing, content), which can yield false positives from comments or partial
matches; instead detect the file type and parse its dependency section properly:
for package.json parse JSON and inspect dependencies/devDependencies, for
requirements.txt strip comments and parse each non-empty line, for
pyproject.toml parse TOML and check [tool.poetry.dependencies] or
[project].dependencies, for go.mod parse require directives, and for Gradle
parse the dependencies block; then test each OTEL_PACKAGES entry against the
parsed dependency names (not a raw substring) and update the missing set
accordingly.

68-74: ⚠️ Potential issue | 🔴 Critical

Missing comma separator produces invalid TOML.

When existing dependencies don't end with a comma, the output concatenates them incorrectly:

# Before
dependencies = [
    "existing-pkg"
]

# After (broken)
dependencies = [
    "existing-pkg"    "opentelemetry-api>=1.0.0",
]

The additions need a leading newline and comma handling. Either ensure inner ends with a comma/newline, or use a TOML parser.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 68 - 74, The current replacement in
content.replace (the regex block producing const updated) concatenates additions
without ensuring a comma/newline after the existing inner entries; update the
replacement logic so additions start with a leading newline and, if inner
contains any non-whitespace characters that do not already end with a comma,
insert a trailing comma (or newline+comma) before appending additions; modify
the code around const additions (and use the open, inner, close params) to
compute a separator like (inner.trim() ? (inner.trimEnd().endsWith(',') ? '\n' :
',\n') : '\n') and then return
`${open}${inner}${separator}${additions}\n${close}` so the result is valid TOML.

75-78: ⚠️ Potential issue | 🟠 Major

Returns missing even when nothing was written.

Three scenarios where this returns packages that weren't actually added:

  1. projectFile is Pipfile or setup.py — no handling, straight to return
  2. pyproject.toml but regex doesn't match (e.g., [tool.poetry.dependencies] instead of [project].dependencies)
  3. pyproject.toml with single-line dependencies = ["pkg"] — no \n] to match

Track what was actually written and return only those packages.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 75 - 78, The function currently returns the
original `missing` list even when no changes were written; change the logic to
track what was actually added by comparing `content` and `updated` (or by
parsing before/after dependency entries) and only include packages that were
actually inserted into the file: compute an `actuallyAdded` set by diffing
dependency entries from `content` vs `updated` (or detecting the exact new
package lines), call `writeFileSync(filePath, updated, 'utf-8')` only if
`updated !== content`, and return `actuallyAdded` instead of `missing`; use the
existing identifiers `content`, `updated`, `missing`, `writeFileSync`, and
`filePath` to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/files.ts`:
- Around line 47-52: The JSDoc for addOtelDeps is misleading because the
function currently returns the computed `missing` packages even when it doesn't
write anything (e.g., Pipfile/setup.py); update addOtelDeps to return only the
packages that were actually written to disk: compute `missing`, perform writes
only for supported file types (requirements.txt and pyproject.toml via the
existing write/patch logic), and set the return value to the subset that was
persisted (empty array if no write occurred), keeping the function name and
caller expectations (e.g., src/index.ts) intact so callers only log added
packages when a write actually happened.
- Around line 56-59: The current check builds missing from OTEL_PACKAGES by
doing a raw substring match on content (see OTEL_PACKAGES, missing, content),
which can yield false positives from comments or partial matches; instead detect
the file type and parse its dependency section properly: for package.json parse
JSON and inspect dependencies/devDependencies, for requirements.txt strip
comments and parse each non-empty line, for pyproject.toml parse TOML and check
[tool.poetry.dependencies] or [project].dependencies, for go.mod parse require
directives, and for Gradle parse the dependencies block; then test each
OTEL_PACKAGES entry against the parsed dependency names (not a raw substring)
and update the missing set accordingly.
- Around line 68-74: The current replacement in content.replace (the regex block
producing const updated) concatenates additions without ensuring a comma/newline
after the existing inner entries; update the replacement logic so additions
start with a leading newline and, if inner contains any non-whitespace
characters that do not already end with a comma, insert a trailing comma (or
newline+comma) before appending additions; modify the code around const
additions (and use the open, inner, close params) to compute a separator like
(inner.trim() ? (inner.trimEnd().endsWith(',') ? '\n' : ',\n') : '\n') and then
return `${open}${inner}${separator}${additions}\n${close}` so the result is
valid TOML.
- Around line 75-78: The function currently returns the original `missing` list
even when no changes were written; change the logic to track what was actually
added by comparing `content` and `updated` (or by parsing before/after
dependency entries) and only include packages that were actually inserted into
the file: compute an `actuallyAdded` set by diffing dependency entries from
`content` vs `updated` (or detecting the exact new package lines), call
`writeFileSync(filePath, updated, 'utf-8')` only if `updated !== content`, and
return `actuallyAdded` instead of `missing`; use the existing identifiers
`content`, `updated`, `missing`, `writeFileSync`, and `filePath` to locate and
modify the code.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f831f7a-9490-4a2f-a63b-736f748d6b0f

📥 Commits

Reviewing files that changed from the base of the PR and between 7a114df and d694b78.

📒 Files selected for processing (1)
  • src/files.ts

@callumreid callumreid changed the title fix: use sync _check_sim_id in LiveKit injection to avoid asyncio import fix: pipecat bot.py detection, OTel dep injection, and asyncio-free LiveKit wiring Mar 31, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
src/files.ts (3)

75-79: ⚠️ Potential issue | 🟠 Major

Return only packages that were actually written.

The function returns missing even when no write occurred (unsupported manifest or pyproject regex no-op), so callers can report false success.

Suggested fix
 export const addOtelDeps = (dir: string, projectFile: string): readonly string[] => {
@@
-  if (missing.length === 0) return []
+  if (missing.length === 0) return []
+  let added: readonly string[] = []
@@
   if (projectFile === 'requirements.txt') {
     writeFileSync(filePath, content.trimEnd() + '\n' + missing.join('\n') + '\n', 'utf-8')
+    added = missing
   } else if (projectFile === 'pyproject.toml') {
@@
-    if (updated !== content) writeFileSync(filePath, updated, 'utf-8')
+    if (updated !== content) {
+      writeFileSync(filePath, updated, 'utf-8')
+      added = missing
+    }
   }
 
-  return missing
+  return added
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 75 - 79, The current implementation returns the
full `missing` list even when no file write occurred (e.g., unsupported manifest
or pyproject regex produced no change); update the function that contains `if
(updated !== content) writeFileSync(filePath, updated, 'utf-8')` so it tracks
which package entries actually caused a write and return only that subset
instead of `missing` — use the existing `updated`/`content` comparison to
collect written package identifiers (or remove entries from `missing` when no
write happened) before the final `return missing`.

68-73: ⚠️ Potential issue | 🔴 Critical

Regex-based pyproject.toml mutation is structurally unsafe.

This string replacement is format-fragile and can still produce invalid TOML or skip valid dependency layouts. Update the dependencies node structurally (parse/mutate/serialize) instead of regex splicing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 68 - 73, The current regex-based replacement
around content.replace that builds `updated` is fragile; instead parse the TOML
into a data structure, update the project.dependencies array programmatically,
then serialize back to TOML. Concretely: replace the use of content.replace(...)
that references `open/inner/close` with logic that parses `content` into a TOML
object (using your project's TOML parser), ensure `doc.project.dependencies`
exists and is an array, push any items from the `missing` array that are not
already present, and then stringify/serialize the modified TOML back to a string
to set `updated`; keep references to the same symbols (`missing`, the `updated`
variable and the place where content was used) so the rest of the flow is
unchanged.

56-59: ⚠️ Potential issue | 🟠 Major

Use parsed dependency keys, not substring checks.

pkg.split('>=')[0] hard-codes one spec format, and content.includes(name) can match comments or unrelated sections. This can incorrectly treat required OTel deps as already present.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 56 - 59, The current missing calculation uses
pkg.split('>=')[0] and content.includes(name), which is brittle and can match
comments or wrong substrings; update the logic that computes missing (the
OTEL_PACKAGES -> missing mapping) to parse the target package list into
dependency keys and check them against the parsed package.json dependency
objects (dependencies/devDependencies/peerDependencies) instead of doing a
substring search, or—if content is not valid JSON—use a strict regex that
matches exact dependency keys (anchored key in a JSON-style dependency section)
rather than content.includes; modify the block that defines missing so it
derives the package name via a proper parser (not pkg.split) and verifies
existence by key lookup in parsedDeps (or anchored regex) to accurately detect
missing OTel packages.
🤖 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/files.ts`:
- Around line 63-66: Replace the magic filename strings in src/files.ts with
named constants: add constants for REQUIREMENTS_TXT and PYPROJECT_TOML in
src/constants.ts and import them into src/files.ts, then replace occurrences of
the literals 'requirements.txt' and 'pyproject.toml' (used in the conditional
that checks projectFile and any other comparisons) with those constants (e.g.,
REQUIREMENTS_TXT and PYPROJECT_TOML) so all file-name checks/reference use the
centralized constants; ensure imports are updated and tests/usage of projectFile
still work with the new constants.

---

Duplicate comments:
In `@src/files.ts`:
- Around line 75-79: The current implementation returns the full `missing` list
even when no file write occurred (e.g., unsupported manifest or pyproject regex
produced no change); update the function that contains `if (updated !== content)
writeFileSync(filePath, updated, 'utf-8')` so it tracks which package entries
actually caused a write and return only that subset instead of `missing` — use
the existing `updated`/`content` comparison to collect written package
identifiers (or remove entries from `missing` when no write happened) before the
final `return missing`.
- Around line 68-73: The current regex-based replacement around content.replace
that builds `updated` is fragile; instead parse the TOML into a data structure,
update the project.dependencies array programmatically, then serialize back to
TOML. Concretely: replace the use of content.replace(...) that references
`open/inner/close` with logic that parses `content` into a TOML object (using
your project's TOML parser), ensure `doc.project.dependencies` exists and is an
array, push any items from the `missing` array that are not already present, and
then stringify/serialize the modified TOML back to a string to set `updated`;
keep references to the same symbols (`missing`, the `updated` variable and the
place where content was used) so the rest of the flow is unchanged.
- Around line 56-59: The current missing calculation uses pkg.split('>=')[0] and
content.includes(name), which is brittle and can match comments or wrong
substrings; update the logic that computes missing (the OTEL_PACKAGES -> missing
mapping) to parse the target package list into dependency keys and check them
against the parsed package.json dependency objects
(dependencies/devDependencies/peerDependencies) instead of doing a substring
search, or—if content is not valid JSON—use a strict regex that matches exact
dependency keys (anchored key in a JSON-style dependency section) rather than
content.includes; modify the block that defines missing so it derives the
package name via a proper parser (not pkg.split) and verifies existence by key
lookup in parsedDeps (or anchored regex) to accurately detect missing OTel
packages.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9f5f360b-0887-4405-8a7e-d32e0751643d

📥 Commits

Reviewing files that changed from the base of the PR and between d694b78 and 0a374e5.

📒 Files selected for processing (1)
  • src/files.ts

Comment thread src/files.ts Outdated

@borgesius borgesius left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yer a wizard harry

…gside pyproject.toml

pip/Docker deployments use requirements.txt regardless of which project file
was detected first. Without this, projects with both files get OTel packages
added only to pyproject.toml, causing ModuleNotFoundError at deploy time.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (4)
src/files.ts (4)

60-62: 🛠️ Refactor suggestion | 🟠 Major

Replace manifest filename literals with constants.

'requirements.txt' and 'pyproject.toml' are hardcoded in library logic. Move these to src/constants.ts and reference constants.

As per coding guidelines, "No magic numbers/strings — use named constants from src/constants.ts".

Also applies to: 72-74

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 60 - 62, Replace the hardcoded manifest strings
with named constants: add constants (e.g. REQUIREMENTS_FILENAME and
PYPROJECT_FILENAME) to src/constants.ts and use them instead of literal
'requirements.txt' and 'pyproject.toml' in this module; update the references
around reqTxt, targetFile, projectFile and filePath (and the similar occurrences
at the later block covering lines 72-74) to import and use REQUIREMENTS_FILENAME
and PYPROJECT_FILENAME so the logic uses the named constants rather than magic
strings.

72-87: ⚠️ Potential issue | 🟠 Major

Return value reports additions even when nothing was written.

When target is unsupported or [project].dependencies is not matched, the function still returns missing, and callers log a false success.

Proposed fix (return only actually written packages)
-  if (targetFile === 'requirements.txt') {
+  if (targetFile === 'requirements.txt') {
     writeFileSync(filePath, content.trimEnd() + '\n' + missing.join('\n') + '\n', 'utf-8')
-  } else if (targetFile === 'pyproject.toml') {
+    return missing
+  }
+  if (targetFile === 'pyproject.toml') {
     const updated = content.replace(
       /(\[project\][\s\S]*?dependencies\s*=\s*\[)([\s\S]*?)(\n\])/,
       (_, open: string, inner: string, close: string) => {
         const additions = missing.map((p) => `    "${p}",`).join('\n')
         return `${open}${inner}\n${additions}${close}`
       },
     )
-    if (updated !== content) writeFileSync(filePath, updated, 'utf-8')
+    if (updated !== content) {
+      writeFileSync(filePath, updated, 'utf-8')
+      return missing
+    }
+    return []
   }
-
-  return missing
+  return []
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 72 - 87, The function currently returns the full
missing array even when no file change occurs; change it to return only the
packages actually written: introduce a local written array (e.g., written:
string[] = []) and in the targetFile === 'requirements.txt' branch set written =
missing after calling writeFileSync; in the 'pyproject.toml' branch only call
writeFileSync and set written = missing when updated !== content (i.e., when a
replacement actually happened); for all other cases leave written empty and
return written instead of missing so callers only see additions that were
persisted (refer to targetFile, missing, content, updated, and writeFileSync).

65-68: ⚠️ Potential issue | 🟠 Major

Dependency presence check is substring-based and can skip required packages.

content.includes(name) is not manifest-aware; comments or similarly named packages can produce false positives.

Proposed fix (manifest-entry-based comparison)
-  const missing = OTEL_PACKAGES.filter((pkg) => {
-    const name = pkg.split('>=')[0]
-    return !content.includes(name)
-  })
+  const normalizeName = (spec: string): string => spec.split(/[<>=!~^]/)[0].trim().toLowerCase()
+  const installed = new Set(
+    content
+      .split(/\r?\n/)
+      .map((line) => line.replace(/#.*/, '').trim())
+      .filter(Boolean)
+      .map((line) => line.replace(/^["']|["']$/g, ''))
+      .map(normalizeName),
+  )
+  const missing = OTEL_PACKAGES.filter((pkg) => !installed.has(normalizeName(pkg)))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 65 - 68, The current check building missing from
OTEL_PACKAGES uses content.includes(name) which is substring-based and can yield
false positives; instead parse the manifest content into actual dependency
entries and compare package names exactly: split content into lines, strip
comments/whitespace, extract dependency names (e.g., by splitting on spaces,
'=', ':', or version delimiters) and check OTEL_PACKAGES' base names (the part
before '>=') against those parsed entries when computing missing; update the
logic around the missing variable and the name extraction so the presence test
uses exact manifest-entry matching rather than content.includes.

74-83: ⚠️ Potential issue | 🔴 Critical

pyproject.toml rewrite can emit invalid TOML.

If the last existing dependency has no trailing comma, added entries are concatenated without a separator and break parsing.

#!/bin/bash
python - <<'PY'
import re, tomllib

content = """[project]
dependencies = [
    "livekit-agents"
]
"""

missing = ["opentelemetry-api>=1.0.0", "opentelemetry-sdk>=1.0.0"]

updated = re.sub(
    r'(\[project\][\s\S]*?dependencies\s*=\s*\[)([\s\S]*?)(\n\])',
    lambda m: f'{m.group(1)}{m.group(2)}\n' + '\n'.join(f'    "{p}",' for p in missing) + m.group(3),
    content,
    flags=re.S,
)

print("=== Rewritten TOML ===")
print(updated)
print("=== Parse result ===")
try:
    tomllib.loads(updated)
    print("valid")
except Exception as e:
    print("invalid:", e)
PY
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/files.ts` around lines 74 - 83, The current pyproject.toml rewrite (in
the branch where targetFile === 'pyproject.toml', using content.replace and the
replacement lambda that builds `additions` and returns
`${open}${inner}\n${additions}${close}`) can emit invalid TOML when the last
existing dependency lacks a trailing comma; update the replacement logic to
detect whether `inner` (the captured dependencies block) is empty or already
ends with a comma and, if not, append a comma (or a comma+newline) to `inner`
before inserting `additions`, so the new entries are always separated; keep the
same symbols (`targetFile`, `content.replace`, the regex capturing groups,
`inner`, `additions`, `updated`) and return
`${open}${fixedInner}\n${additions}${close}` where `fixedInner` has the comma
inserted only when necessary.
🤖 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/files.ts`:
- Around line 52-56: There are duplicate JSDoc blocks describing the same
behavior (adding OTel packages to a dependency file); remove the redundant
second docblock and keep a single canonical comment directly above the
implementation (e.g., above the addOtelPackagesToDependencyFile function or its
export) so the file has one clear JSDoc description.

---

Duplicate comments:
In `@src/files.ts`:
- Around line 60-62: Replace the hardcoded manifest strings with named
constants: add constants (e.g. REQUIREMENTS_FILENAME and PYPROJECT_FILENAME) to
src/constants.ts and use them instead of literal 'requirements.txt' and
'pyproject.toml' in this module; update the references around reqTxt,
targetFile, projectFile and filePath (and the similar occurrences at the later
block covering lines 72-74) to import and use REQUIREMENTS_FILENAME and
PYPROJECT_FILENAME so the logic uses the named constants rather than magic
strings.
- Around line 72-87: The function currently returns the full missing array even
when no file change occurs; change it to return only the packages actually
written: introduce a local written array (e.g., written: string[] = []) and in
the targetFile === 'requirements.txt' branch set written = missing after calling
writeFileSync; in the 'pyproject.toml' branch only call writeFileSync and set
written = missing when updated !== content (i.e., when a replacement actually
happened); for all other cases leave written empty and return written instead of
missing so callers only see additions that were persisted (refer to targetFile,
missing, content, updated, and writeFileSync).
- Around line 65-68: The current check building missing from OTEL_PACKAGES uses
content.includes(name) which is substring-based and can yield false positives;
instead parse the manifest content into actual dependency entries and compare
package names exactly: split content into lines, strip comments/whitespace,
extract dependency names (e.g., by splitting on spaces, '=', ':', or version
delimiters) and check OTEL_PACKAGES' base names (the part before '>=') against
those parsed entries when computing missing; update the logic around the missing
variable and the name extraction so the presence test uses exact manifest-entry
matching rather than content.includes.
- Around line 74-83: The current pyproject.toml rewrite (in the branch where
targetFile === 'pyproject.toml', using content.replace and the replacement
lambda that builds `additions` and returns
`${open}${inner}\n${additions}${close}`) can emit invalid TOML when the last
existing dependency lacks a trailing comma; update the replacement logic to
detect whether `inner` (the captured dependencies block) is empty or already
ends with a comma and, if not, append a comma (or a comma+newline) to `inner`
before inserting `additions`, so the new entries are always separated; keep the
same symbols (`targetFile`, `content.replace`, the regex capturing groups,
`inner`, `additions`, `updated`) and return
`${open}${fixedInner}\n${additions}${close}` where `fixedInner` has the comma
inserted only when necessary.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f17df326-51b5-4f39-ae18-3d1725544f92

📥 Commits

Reviewing files that changed from the base of the PR and between 0a374e5 and 453fdf5.

📒 Files selected for processing (1)
  • src/files.ts

Comment thread src/files.ts
The old prompt used body.dialin_settings.custom_context.coval_simulation_id
which doesn't exist. The Coval SIP header X-Coval-Simulation-Id is forwarded
by the PCC webhook under body.dialin_settings.sip_headers, so the correct
path is body.dialin_settings.sip_headers['X-Coval-Simulation-Id'].

Also moved extraction to bot() top-level (not on_dialin_connected) since
args.body is available at session start.
@callumreid callumreid changed the title fix: pipecat bot.py detection, OTel dep injection, and asyncio-free LiveKit wiring fix: pipecat bot.py detection, OTel dep injection, correct SIP sim ID path, and asyncio-free LiveKit wiring Mar 31, 2026
@callumreid callumreid merged commit b269264 into main Mar 31, 2026
2 checks passed
callumreid added a commit that referenced this pull request Apr 1, 2026
Incorporate PR #8's changes (pipecat bot.py priority, OTel dep injection,
sync LiveKit sim ID handler) while keeping Vapi framework support and
server.py entry point addition.

Removed redundant 'import asyncio' instruction from LiveKit rules since
PR #8 switched to synchronous _check_sim_id (no asyncio needed).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants