fix(frame): add size and duplicate-name guards to Frame::add_column

[adityashirsatrao007]

Summary

Fixes #925

Problem

Frame::add_column lacked row-count validation and duplicate-name checking, causing memory corruption and inconsistent frame state.

Changes

Both C++ guards (size validation + duplicate-name check) are already present in main. This PR adds the missing regression test:

tests/test_frame.py

• Added test_add_column_rejects_duplicate_name — ensures adding a column with an existing name raises ValueError.

Existing tests already cover:

• ✅ valid same-length columns accepted
• ✅ mismatched-length columns rejected with ValueError
• ✅ first column in empty frame accepted at any size
• ✅ zero-row explicit frame rejects nonempty first column

Diff scope

1 file changed — tests/test_frame.py (+19 lines)

[Copilot]

Pull request overview

This PR hardens the C++ Frame::add_column API to prevent internal inconsistency and potential memory corruption when adding columns via the pybind-exposed core types (fixing #925).

Changes:

• Rejects adding a column whose row count doesn’t match the existing frame (when non-empty).
• Rejects adding a column whose name already exists in the frame (std::invalid_argument → Python ValueError).
• Adds a Python test that locks in the duplicate-name rejection behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
cpp/src/frame.cpp	Adds a duplicate-name guard (and retains row-count validation) in Frame::add_column to preserve frame invariants.
tests/test_frame.py	Adds coverage for rejecting duplicate column names through the pybind-exposed Frame.add_column.

---

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

[im-anishraj]

Thanks for tightening this down to the duplicate-name guard while preserving the existing size guard behavior. The diff is focused, the regression test covers the duplicate-name path, and the full repository CI/wheel matrix is green.

I also attempted a targeted local validation in a clean temp worktree, but this machine cannot currently build the extension: MSVC/NMake is not available on PATH, and the available MinGW is too old for the project C++17 headers (<variant>/<optional>). So I am approving the code review based on the clean diff plus green CI, but I am not merging it from this environment.

[adityashirsatrao007]

Thanks for tightening this down to the duplicate-name guard while preserving the existing size guard behavior. The diff is focused, the regression test covers the duplicate-name path, and the full repository CI/wheel matrix is green.

I also attempted a targeted local validation in a clean temp worktree, but this machine cannot currently build the extension: MSVC/NMake is not available on PATH, and the available MinGW is too old for the project C++17 headers (<variant>/<optional>). So I am approving the code review based on the clean diff plus green CI, but I am not merging it from this environment.

Thanks for the detailed review and approval. Understood regarding the local environment limitations. I’ll keep an eye on the PR in case any follow-up changes are needed.

[im-anishraj]

Thanks for the focused guard. I rechecked this against the current main after today’s merges, and the branch no longer merges cleanly.

Blocker before merge:

• Please sync/rebase with the latest main and resolve the conflicts in cpp/src/frame.cpp and tests/test_frame.py. The conflict blocks the required local merge-result validation, including the MSVC native build/tests and Python test gate.

Once the PR is clean again, I will rerun the native + Python validation before making a merge decision.

[im-anishraj]

Thanks for rebasing this. The branch is mergeable now, and I approved the pending workflow runs for the new head. I’ll wait for CI/wheel checks to report green before running the local validation pass and reconsidering the review.

[im-anishraj]

Thanks for rebasing. I validated the current merge result locally on Windows with Python 3.12/MSVC: editable install with dev/parquet extras, targeted ests/test_frame.py -k add_column, full pytest, Black, Ruff, and mypy all pass. CI is green too, so this is ready.