Skip to content

fix: prevent false-positive horizontal orientation issues on vertical capacitors#26

Open
Bilal-Lodhi wants to merge 5 commits into
tscircuit:mainfrom
Bilal-Lodhi:fix-issue-21
Open

fix: prevent false-positive horizontal orientation issues on vertical capacitors#26
Bilal-Lodhi wants to merge 5 commits into
tscircuit:mainfrom
Bilal-Lodhi:fix-issue-21

Conversation

@Bilal-Lodhi

Copy link
Copy Markdown

Fixes #21. The solver was incorrectly throwing a CapacitorSymbolHorizontal error for vertical capacitors because it only checked for specific horizontal symbol string names.

I added early-return checks if the symbol matches known vertical names (capacitor_top, capacitor_bottom) or if the layout geometry clearly shows a vertical orientation (height > width).

All orientation tests pass successfully locally.

Copilot AI review requested due to automatic review settings May 20, 2026 04:45

Copilot AI 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.

Pull request overview

This PR updates the schematic placement analyzer’s capacitor-orientation solver to avoid raising a CapacitorSymbolHorizontal issue for capacitors that are clearly drawn vertically (e.g., rotated 90°), addressing false positives reported in #21.

Changes:

  • Add explicit vertical capacitor symbol name allowlist (capacitor_top, capacitor_bottom) to bypass the horizontal-orientation issue.
  • Add a geometry-based fallback: skip the horizontal-orientation issue when the schematic box is taller than it is wide (height > width).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MustafaMulla29 MustafaMulla29 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests?

@Bilal-Lodhi

Bilal-Lodhi commented May 20, 2026

Copy link
Copy Markdown
Author

Tests?

I put together a standalone verification script to test the solver behavior directly. Here is the terminal output showing the horizontal issue triggering and the vertical one bypassing cleanly:

E:\CODE\paid-bounty#21\circuit-json-schematic-placement-analysis>bun run test-scripts/verify-fix.tsx
=== Horizontal Capacitor ===
=== Vertical Capacitor (schRotation=90) ===
Horizontal capacitor → CapacitorSymbolHorizontal: ✓ (expected)
Vertical capacitor → CapacitorSymbolHorizontal: ✓ (none, expected)

ALL CHECKS PASSED ✓

E:\CODE\paid-bounty#21\circuit-json-schematic-placement-analysis>

@MustafaMulla29

Copy link
Copy Markdown
Contributor

You have to add tests here in this pr.

@Bilal-Lodhi

Copy link
Copy Markdown
Author

done

@MustafaMulla29 MustafaMulla29 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That test is nowhere near the codebase patterns

@Bilal-Lodhi

Copy link
Copy Markdown
Author

That test is nowhere near the codebase patterns

Apologies I completely misunderstood it. I have deleted the custom test

I ran the standard test runner via bun test and all 19 tests (including both the horizontal and vertical capacitor test cases) are passing cleanly:

19 pass
0 fail
1 snapshots, 93 expect() calls
Ran 19 tests across 19 files. [6.34s]

@MustafaMulla29 MustafaMulla29 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not the fix, please understand what is the issue and the fix does not belong here. This repo is just analysis repo.

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.

vertical capacitors

3 participants