Skip to content

sync: mirror from thyn-ai/codna@4057deb9e983f4c0fc9f4254b867a737b4288eaf - #36

Closed
0xamlab wants to merge 1 commit into
mainfrom
sync/4057deb9e983f4c0fc9f4254b867a737b4288eaf
Closed

0xamlab wants to merge 1 commit into
mainfrom
sync/4057deb9e983f4c0fc9f4254b867a737b4288eaf

Conversation

@0xamlab

@0xamlab 0xamlab commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Automated mirror of cli/ (flattened), the root allowlist (server.json, glama.json, LICENSE, .gitleaks.toml) and the public-root/ community tree from thyn-ai/codna at 4057deb9e983f4c0fc9f4254b867a737b4288eaf. Merges itself via auto-merge once the mirror's required checks (codna review, Analyze (python)) pass and the codna-ai review approves; if a later sync lands first, this PR is closed as superseded -- every sync PR is a full snapshot of the exported surface.

@github-actions

Copy link
Copy Markdown

Thanks for opening your first pull request in the Codna CLI! Please make sure the checks are green and that the change follows CONTRIBUTING.md. A maintainer will review it shortly.

@codna-ai codna-ai 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.

codna review — 3 finding(s): 3 inline, 0 in summary.

Reply @codna fix on a finding to have Codna open a verified, test-green, risk-gated fix PR.

⏸ Not approved: 3 medium/high finding(s) in this review.

Comment thread codna/testrun.py
Comment on lines +379 to +380
result = Sandbox(network="deny", timeout_seconds=budget, env=env, network_backend=backend).run(
argv, cwd=run_dir, accepted_exit_codes=(0, 1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 HIGH · correctness — CODNA_JUNIT env var points to wrong path for suites 1..N in multi-suite runs

The diff removes run_env = {**env, "CODNA_JUNIT": run_junit} and replaces it with env=env. The outer env["CODNA_JUNIT"] is set once to the base junit path (e.g. report.xml). For multi-suite auto-detected pytest runs (len(suites) > 1), each suite writes its JUnit report to a suite-specific path (report-0.xml, report-1.xml, …), passed via --junitxml={suite_junit}. However CODNA_JUNIT in the environment still points at the base report.xml for every suite. Any test code or pytest plugin that uses $CODNA_JUNIT (as documented and as the test helper at line 374 demonstrates) will write to the wrong path for suites 1..N, either overwriting suite 0's report or writing to a file that codna never reads. The original per-suite run_env was the correct fix for this.

- result = Sandbox(network="deny", timeout_seconds=budget, env=env, network_backend=backend).run(
+ run_env = {**env, "CODNA_JUNIT": run_junit}
+ result = Sandbox(network="deny", timeout_seconds=budget, env=run_env, network_backend=backend).run(

Reply @codna fix and Codna opens a verified, test-green, risk-gated fix PR.

Comment thread codna/testrun.py
Comment on lines +355 to +360
runs.append((
suite,
[sys.executable, "-m", "pytest", "-q", f"--junitxml={suite_junit}"],
suite_junit,
per_suite,
))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM · correctness — Removed --ignore flags allow pytest to double-collect nested suite roots

The diff removes the argv lines that passed --ignore={candidate} for each nested suite root that is a direct child of the current suite. Previously, when running a container suite, pytest was told to skip sub-directories that are run independently as their own suite. Without these --ignore flags, pytest running from a parent suite directory will recurse into and collect tests from those nested roots a second time (once from the parent, once from the child suite's own run). This causes duplicate test IDs in the JUnit output and can produce misleading pass/fail results.

Reply @codna fix and Codna opens a verified, test-green, risk-gated fix PR.

Comment on lines +870 to +873
for lid, previous_kid, keyval in zip(ids, previous_keyids, keyvals):
key = keyval.item() if hasattr(keyval, "item") else keyval
if index.key_i2v.get(previous_kid) != key:
index.lexical_dirty.add(lid)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM · correctness — update() reads previous_keyids unconditionally in else-branch even when texts is

When texts is None, previous_keyids is computed (line 843) and then used in the else-branch zip at line 870. This is correct. However, when keys is provided (not None) AND texts is None, previous_keyids still refers to key-IDs fetched BEFORE the native upsert. After ty_index_insert, the native layer may have updated its internal state for those IDs. If the caller is changing partition keys (keys is not None), iterating previous_keyids in the dirty-tracking loop is intentional, but the comment (line 831-833) says texts=None means text is UNCHANGED — yet any row whose key changed is marked dirty. Rows whose key did NOT change but whose column data changed are not marked dirty. The lexical dirty set is therefore incomplete: a row whose text-relevant column changed but partition key didn't gets no dirty mark, so stale BM25 data persists.

Reply @codna fix and Codna opens a verified, test-green, risk-gated fix PR.

@0xamlab

0xamlab commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #37 (a later thyn-ai/codna commit). Every sync PR is a full snapshot of the exported surface, so this one carries nothing #37 does not. Closing.

@0xamlab 0xamlab closed this Oct 31, 2026
auto-merge was automatically disabled October 31, 2026 21:08

Pull request was closed

@0xamlab
0xamlab deleted the sync/4057deb9e983f4c0fc9f4254b867a737b4288eaf branch October 31, 2026 21:08
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.

1 participant