Skip to content

feat(method): refuse non-lexical modality as unique content - #150

Merged
seonghobae merged 8 commits into
mainfrom
agent/modality-source
Aug 24, 2026
Merged

feat(method): refuse non-lexical modality as unique content#150
seonghobae merged 8 commits into
mainfrom
agent/modality-source

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Non-lexical modality stays explicit method structure (ADR 0004/0012). It is not unique latent content and is not erased by a stopword list. Recovery is the computed share of recovered kinds that match known truth versus collapsing every token to unique content.

Complementary to #147 section, #148 style, #149 copied-text, #145 stopword, and #75 method_effects. This crate owns modality-versus-unique-content identity.

Local gates:

  • `cargo test -p modality_source --all-targets` GREEN after RED (package did not exist)
  • clippy `-D warnings` PASS
  • workspace contract PASS
  • docstring contract PASS
  • lines 14/14; nightly-2026-08-01 branches 6/6

Does not allocate migration `0008`. Does not recreate those in-flight method-source crates.

Keep this PR draft. Preferred merge remains #47 only when exact-head required Checks pass and a qualifying independent APPROVE exists. Restored #144 to draft. Do not empty-commit #47.

Non-lexical modality stays explicit method structure (ADR 0004 and
0012). It is not unique latent content and is not erased by a
stopword list. Recovery is the computed share of modality kinds that
match known truth versus collapsing every token to unique content.
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 43 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e7aec2c7-c120-4c6e-9249-f266cb595d39

📥 Commits

Reviewing files that changed from the base of the PR and between 94f8bbc and cef63ce.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • ARCHITECTURE.md
  • CHANGELOG.md
  • Cargo.toml
  • README.md
  • crates/modality_source/Cargo.toml
  • crates/modality_source/src/error.rs
  • crates/modality_source/src/kind.rs
  • crates/modality_source/src/lib.rs
  • crates/modality_source/tests/crate_contract.rs
  • crates/modality_source/tests/modality_source_contract.rs
  • docs/TRACEABILITY.md
  • docs/adr/0004-shared-multilingual-latent-space.md
  • docs/adr/0012-temporal-relational-shared-latent-topic-measurement.md
  • docs/adr/README.md
  • docs/research/modality-source-identity.md
  • docs/research/standards-and-literature.md
  • docs/validation/temporal-event-foundation.md
  • scripts/check_workspace_contract.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@seonghobae
seonghobae marked this pull request as ready for review August 20, 2026 17:56
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current head 29da41b merges protected main and fixes the previous exact-head CI root cause: repository rustfmt ordering in the modality-source exports/tests. Focused proof passed: cargo fmt --all -- --check, cargo test -p modality_source --offline (6 tests), workspace contracts, documentation validation, Rust docstring contract, and git diff --check. The exact-head Checks are rerunning; merge remains gated by one qualifying independent approval.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head validation update (2e9cf6c): fixed the quality contract to derive the Rust crate count from scripts/check_workspace_contract.py instead of hard-coding 10. Local evidence: 89 quality tests passed; coverage 100% (991/991 statements, 442/442 branches); workspace, docstring, documentation, and diff checks passed. Please review and rerun Checks against this exact head; merge remains subject to the repository's two independent approvals and protected rules.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review fix (00deb56): removed the duplicate active-PR Purpose-bound provider payloads ledger row; one implemented-main row now remains. Local validation passed: 89 quality tests, 100% statement/branch coverage (991/991, 442/442), workspace/docstring/documentation contracts, and diff check. Please re-review this exact head.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Review the exact current PR head 00deb56 against base main with the protected independent agents. Re-check current source, tests, coverage, documentation, and security; do not modify or merge. @opencode-agent @cwl-noema-review

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review refresh for 00deb56:

  • Non-lexical modality is not treated as unique content, preserving the distinction between report style and measurable lexical evidence.
  • git diff --check, documentation validation, workspace contract, docstring contract, and cargo fmt --all -- --check passed locally.
  • Current hosted checks are pass=14, pending=1, fail=0; merge remains withheld until the pending exact-head required check completes and qualifying approvals exist.
  • No new actionable source defect was found at this exact head; no approval is being self-issued.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Rebased current head f9110e0 onto origin/main. The changelog conflict retains both feature and current-main entries; inherited documentation trailing whitespace was removed. Local merge-tree, git diff --cached --check, and cargo fmt --all -- --check pass. Exact-head hosted checks and required independent approvals remain required before protected merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent @cwl-noema-review

Review-only request for exact current head f9110e0. Re-review modality identity and current validation ledger, the Rust identity contract, APA 7 research scope, documentation maturity, and protected-merge requirements. Do not merge or enable auto-merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review request: exact head f9110e0eec68d823778a433c4ccab3a1beca46f5 has one purpose-bound provider-payload row and the distinct modality-source row; the duplicate-row finding is resolved on this exact head. Terminal checks are green and coverage-evidence remains queued. Please review this exact head. No merge bypass is requested.

@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 00:55
@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head queue disposition

Exact head f9110e0eec68d823778a433c4ccab3a1beca46f5 is green across required Checks with no failed or queued gate. The bounded non-lexical-modality identity crate and documentation remain mechanically mergeable, but REVIEW_REQUIRED has no qualifying independent approval. WAIT_AND_REMEDIATE; no bypass.

# Conflicts:
#	ARCHITECTURE.md
#	CHANGELOG.md
#	Cargo.lock
#	Cargo.toml
#	README.md
#	docs/TRACEABILITY.md
#	docs/adr/0004-shared-multilingual-latent-space.md
#	docs/adr/0012-temporal-relational-shared-latent-topic-measurement.md
#	docs/adr/README.md
#	docs/research/standards-and-literature.md
#	docs/validation/temporal-event-foundation.md
#	scripts/check_workspace_contract.py
#	tests/quality/test_check_docstrings.py
@seonghobae
seonghobae merged commit 1d13186 into main Aug 24, 2026
19 of 22 checks passed

@devin-ai-integration devin-ai-integration 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.

Devin Review found 4 new potential issues.

Open in Devin Review

Comment thread CHANGELOG.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Historical changelog line still says 'ten' crates

CHANGELOG.md still reads "Rust 1.97.1 virtual Cargo workspace with ten explicit modular foundation crates." while README.md was updated to "eleven". This CHANGELOG line is a historical Task-1 Added entry describing the original ten-crate foundation, so leaving it unchanged is arguably faithful to what was originally added rather than a defect. No test or contract script asserts a crate count against this text, so it does not break CI. Flagging only for editorial consistency consideration.

(Refers to this code)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: New crate satisfies workspace contract checks

The new modality_source crate meets every requirement enforced by check_workspace_contract.py: Cargo.toml sets publish = false, [lints] workspace = true, and inherits all required package fields; lib.rs has crate-level //! rustdoc, #![forbid(unsafe_code)], and #![deny(missing_docs)] with no placeholder API; and crate_contract.rs provides the required package-identity contract test. EXPECTED_CRATES and both members/default-members lists were updated consistently, and README's crate count was updated from ten to eleven.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +78 to +84
let mut matches = 0_u32;
for (truth_kind, decided_kind) in truth.iter().zip(decided) {
if truth_kind == decided_kind {
matches += 1;
}
}
Ok(f64::from(matches) / truth.len() as f64)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Branch coverage relies on integration tests, not unit tests alone

The 100% branch-coverage contract (AGENTS.md #8) requires both branches of if truth_kind == decided_kind in kind.rs to be exercised. The in-module unit test only ever passes matching pairs, so the mismatch (false) branch is only covered by the integration test recovered_kinds_match_known_truth_better_than_a_unique_content_collapse in modality_source_contract.rs, which supplies collapsed values. Coverage therefore depends on cargo llvm-cov including integration test targets; if the coverage run scopes to lib-only, this branch would appear uncovered. Not a bug given the standard workspace coverage config, but worth noting the dependency.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +71 to +85
pub fn identity_recovery_rate(
truth: &[ModalityKind],
decided: &[ModalityKind],
) -> Result<f64, ModalitySourceError> {
if truth.is_empty() || truth.len() != decided.len() {
return Err(ModalitySourceError::InvalidModalityPayload);
}
let mut matches = 0_u32;
for (truth_kind, decided_kind) in truth.iter().zip(decided) {
if truth_kind == decided_kind {
matches += 1;
}
}
Ok(f64::from(matches) / truth.len() as f64)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: identity_recovery_rate fail-closed and arithmetic verified

identity_recovery_rate at kind.rs correctly fails closed for empty or length-mismatched slices before any division, and the match count divided by truth.len() cannot divide by zero due to the earlier empty check. The #![allow(clippy::cast_precision_loss)] in lib.rs is what permits the as f64 cast on the length. Logic is sound.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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