Skip to content

Fix pinheader right side direction#911

Merged
imrishabh18 merged 3 commits intomainfrom
codex/fix-pinheader-direction-for-right-facing-pin
Jun 12, 2025
Merged

Fix pinheader right side direction#911
imrishabh18 merged 3 commits intomainfrom
codex/fix-pinheader-direction-for-right-facing-pin

Conversation

@imrishabh18
Copy link
Member

@imrishabh18 imrishabh18 commented Jun 12, 2025

Summary

  • fix detection of explicit pin mapping when only one side is defined
  • add regression test for pinheader right side direction

Testing

  • bun x tsc --noEmit (fails: Reached heap limit Allocation failed - JavaScript heap out of memory)
  • bun test tests/repros/repro20-pinheader-rightSide-direction.test.tsx

https://chatgpt.com/codex/tasks/task_b_684af12d168483279bb71d1c229081ed

/close #842

@vercel
Copy link

vercel bot commented Jun 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tscircuit-core-benchmarks ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2025 3:53pm

Comment on lines +72 to +78
const a = arrangement as ExplicitPinMappingArrangement
return (
a.leftSide !== undefined ||
a.rightSide !== undefined ||
a.topSide !== undefined ||
a.bottomSide !== undefined
)
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable a is too short and non-descriptive for a cast representation of arrangement. Consider using a more meaningful name such as explicitArrangement to improve code readability and make the intent clearer. This would be more consistent with the naming conventions used elsewhere in the codebase.

Suggested change
const a = arrangement as ExplicitPinMappingArrangement
return (
a.leftSide !== undefined ||
a.rightSide !== undefined ||
a.topSide !== undefined ||
a.bottomSide !== undefined
)
const explicitArrangement = arrangement as ExplicitPinMappingArrangement
return (
explicitArrangement.leftSide !== undefined ||
explicitArrangement.rightSide !== undefined ||
explicitArrangement.topSide !== undefined ||
explicitArrangement.bottomSide !== undefined
)

Spotted by Diamond (based on custom rules)

Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +1 to +51
import { test, expect } from "bun:test"
import { getTestFixture } from "../fixtures/get-test-fixture"

export default test("pinheader right side direction", async () => {
const { circuit } = getTestFixture()

circuit.add(
<board width="10mm" height="10mm">
<pinheader
name="J2"
pinCount={3}
facingDirection="right"
schPinArrangement={{
rightSide: {
direction: "top-to-bottom",
pins: ["pin1", "pin2", "pin3"],
},
}}
/>
<pinheader
name="J3"
pinCount={3}
schX={5}
facingDirection="right"
schPinArrangement={{
rightSide: {
direction: "bottom-to-top",
pins: ["pin1", "pin2", "pin3"],
},
}}
/>
</board>,
)

await circuit.renderUntilSettled()

const ports = circuit.db.schematic_port.list()
const comp1 = ports
.filter((p) => p.schematic_component_id === "schematic_component_0")
.sort((a, b) => b.center.y - a.center.y)
.map((p) => p.pin_number)
const comp2 = ports
.filter((p) => p.schematic_component_id === "schematic_component_1")
.sort((a, b) => b.center.y - a.center.y)
.map((p) => p.pin_number)

expect(comp1).toEqual([1, 2, 3])
expect(comp2).toEqual([3, 2, 1])

expect(circuit).toMatchSchematicSnapshot(import.meta.path)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

The filename repro20-pinheader-rightSide-direction.test.tsx uses camelCase in the middle of the name, which is inconsistent with the project's kebab-case naming convention. For better consistency with the codebase, consider renaming it to repro20-pinheader-right-side-direction.test.tsx.

Spotted by Diamond (based on custom rules)

Is this helpful? React 👍 or 👎 to let us know.

expect(comp1).toEqual([1, 2, 3])
expect(comp2).toEqual([3, 2, 1])

expect(circuit).toMatchSchematicSnapshot(import.meta.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

This snapshot is cut off i think unfortunately, maybe stack vertically?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this, it's an AI generated test file. I have the test in other file which verify the fix

@imrishabh18 imrishabh18 merged commit 9451694 into main Jun 12, 2025
10 checks passed
@imrishabh18 imrishabh18 deleted the codex/fix-pinheader-direction-for-right-facing-pin branch June 12, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

order of pin number not being changed for the PinHeaders facing right side

2 participants