feat(intune): parse Company Portal Windows LocalState logs - #460
feat(intune): parse Company Portal Windows LocalState logs#460adamgell wants to merge 55 commits into
Conversation
Adds `cmtraceopen_parser::intune::portal::windows::company_portal::logs`, a dedicated parser plus canonical evidence document for `%LOCALAPPDATA%\Packages\Microsoft.CompanyPortal_8wekyb3d8bbwe\LocalState\Log_<n>.log` and the sibling `Log.<BridgeName>_<n>.log` bridge logs. Evidence basis and its limitation --------------------------------- Microsoft documents the path and the `Log_<n>.log` pattern but not the record grammar. Exactly ONE verbatim record has ever been published, from Company Portal app version 12-0-0: 2024-11-15T16:50:07.2850341Z INFO Event None 0 <guid> 12-0-0 [Configuration Manager Trace Listener] ... Everything here is derived from that single record, so the grammar is version-scoped from the start: - records are read with GrammarVersion::V1; - 12-0-0 is the only validated app version. Any other version still parses with V1 (it is the only grammar that exists) but downgrades the selection to ParserProvenance::Heuristic and the document to Experimental / Low confidence, and names the gap in coverage; - document confidence never reaches High. Raising it requires a second app version captured from a real device. Encoding, newline style, rotation ordering, the full severity vocabulary, and whether payloads genuinely span lines are all unproven from public evidence. Each is handled defensively rather than assumed, and the open items are recorded in the module docs. Detection safety ---------------- `Log_<n>.log` is a generic name that any UWP package can use, so the file name only nominates a candidate. Confirmation requires field 6 to be a hyphenated GUID and field 7 to be a dash-separated version triple. Two negative fixtures prove it: a column-aligned unrelated UWP log with ISO instants and a severity column is refused even when it sits at the exact Company Portal path, and a generic timestamped log stays on the generic parser. Losslessness and privacy ------------------------ The nested legacy ConfigMgr trace text inside the message — including its day-first date — is never stripped or reinterpreted. Records that fail validation keep their original text and are reported through parse_errors and a coverage row rather than dropped. Dedicated severity wins over keyword inference; only an unrecognized token defers to it. The evidence document is redacted by default and reuses the existing ESP free-text rule table rather than growing a second one; the unredacted form is an explicitly named local-only opt-out. The viewer's LogEntry path is never redacted, because it has to show the file the user opened. Also: the CI parser-crate step now runs every test target in the crate (it named a single target, so new targets ran nowhere), adds a parser-crate clippy gate, and `.gitignore`'s `Logs/` rule is un-ignored for the new `logs/` directories, which it was matching case-insensitively on macOS and Windows checkouts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A code review of this branch produced six findings. None reached the
confidence bar to post as blocking review comments, but four were verified
accurate and are documentation/API-surface defects worth correcting rather
than shipping.
Before: several doc comments claimed more than the implementation delivers.
* models.rs described field 5 as a "monotonic sequence value" while
grammar.rs documented the same field as "semantics unproven". One published
record cannot establish monotonicity, and nothing checks it. The claim is
removed; the field is now described as an unsigned integer of unproven
semantics, which is what the evidence supports.
* CompanyPortalTimestampKind::Invalid was documented as "the field had the
right shape but is not a real instant", but nothing ever constructed it:
parse_utc_instant returns None for that input, so the record is framed
Malformed and reaches the document with timestamp: None. The variant was
dead public API on a published crate describing behavior that does not
happen. Removed, and the enum now documents what actually occurs. Dropping
a half-resolved timestamp is the correct behavior, so only the type and its
doc change.
* matches_company_portal_log_record claimed it was "used by parser::detect".
parser::detect calls classify_line directly, because it needs the
classification to count validated app versions rather than a bool. The
function is the house-convention boolean wrapper; the doc now says so.
* The module claimed losslessness in three places while framing.rs strips
trailing whitespace from every line and drops blank lines entirely. Neither
is reversible from raw_text. The claims are narrowed to what holds — a
record the grammar cannot read is still reported rather than dropped — and
framing.rs now names both exceptions explicitly, including which rule has
to change if a multi-line payload containing a blank line is ever observed.
Why this seam: these are contract statements on a crate published to
crates.io, in a module whose entire premise is not claiming more than the
evidence proves. A doc that overclaims is the same defect class the module
exists to avoid.
Verified on this commit:
cargo test --locked -p cmtraceopen-parser
-> lib 403 passed, company_portal_windows_logs 32 passed,
esp_diagnostics 222 passed, 0 failed
cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings
-> clean
cargo fmt --check --all
-> no diff in any file this branch touches
Refs #366
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a version-aware Company Portal Windows log parser with structured records, redaction, detection, encoding support, desktop integration, frontend tail amendments, fixture protection, documentation, and CI validation. ChangesCompany Portal Windows parser
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
enhancement
|
@coderabbitai full review — exact-head review requested for Please assess this as the bounded raw-format Company Portal parser described in the PR: content-confirmed detection, version-scoped provenance, default evidence redaction, synthetic fixtures, and parser/UI registration. It does not claim live Windows collection, a second real-version capture, or semantic root-cause analysis. An independent review corrected native parser-format inventory coverage and untrusted severity redaction before this replacement PR was published. A fresh local CodeRabbit pass surfaced an untriaged public re-export-surface suggestion; please inspect the exact public API rather than treating that automated suggestion as accepted. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== HEAD =='
git rev-parse HEAD
printf '%s\n' '== Diff stat against main =='
git diff --stat origin/main...HEAD
printf '%s\n' '== Changed paths =='
git diff --name-status origin/main...HEAD
printf '%s\n' '== Public Rust exports and Company Portal references =='
rg -n -C 3 --glob '*.rs' \
'pub\s+(use|mod|struct|enum|fn)|Company\s*Portal|company_portal|company-portal' .
printf '%s\n' '== TypeScript registration references =='
rg -n -C 3 --glob '*.{ts,tsx}' \
'Company\s*Portal|company_portal|company-portal' .
printf '%s\n' '== Source outlines for changed Rust files =='
git diff --name-only --diff-filter=AM origin/main...HEAD \
| grep -E '\.rs$' \
| while IFS= read -r file; do
printf '\n-- %s --\n' "$file"
ast-grep outline "$file" --items all || true
doneLength of output: 175184 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Exact head and changed paths =='
git rev-parse HEAD
git diff --name-status origin/main...HEAD | grep -Ei 'company|parser/(detect|mod)\.rs|models/log_entry\.rs|bundle_ops\.rs|parser_supported_formats'
printf '%s\n' '== Public module chain =='
for file in \
crates/cmtraceopen-parser/src/lib.rs \
crates/cmtraceopen-parser/src/intune/mod.rs \
crates/cmtraceopen-parser/src/intune/portal/mod.rs \
crates/cmtraceopen-parser/src/intune/portal/windows/mod.rs \
crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/mod.rs \
crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs
do
if [ -f "$file" ]; then
printf '\n-- %s --\n' "$file"
cat -n "$file"
fi
done
printf '%s\n' '== Changed Company Portal diff =='
git diff --find-renames --find-copies --unified=40 origin/main...HEAD -- \
crates/cmtraceopen-parser/src/intune/portal/windows/company_portal \
crates/cmtraceopen-parser/src/parser/detect.rs \
crates/cmtraceopen-parser/src/parser/mod.rs \
crates/cmtraceopen-parser/src/models/log_entry.rs \
src-tauri/src/commands/bundle_ops.rs \
src-tauri/tests/parser_supported_formats.rs \
crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs
printf '%s\n' '== Parser selection and dispatch =='
sed -n '430,570p' crates/cmtraceopen-parser/src/parser/detect.rs
sed -n '58,215p' crates/cmtraceopen-parser/src/parser/mod.rs
printf '%s\n' '== Detector, document projection, and entry projection =='
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rs
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rs
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rsLength of output: 209848 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Logical-record consumers and tail implementation =='
rg -n -C 8 --glob '*.rs' \
'parse_one_line\s*\(|parse_lines_with_selection\s*\(|RecordFraming::LogicalRecord|record_framing' \
src-tauri/src crates/cmtraceopen-parser/src
printf '%s\n' '== Tail reader source =='
sed -n '100,330p' src-tauri/src/watcher/tail.rs
sed -n '330,525p' src-tauri/src/watcher/tail.rs
printf '%s\n' '== Parser enum match sites outside changed files =='
rg -n -C 5 --glob '*.{rs,ts,tsx}' \
'ParserKind::|ParserImplementation::|companyPortal' \
src-tauri/src src crates/cmtraceopen-parser/src \
| grep -E 'ParserKind::|ParserImplementation::|companyPortal|match parser|match .*implementation' || true
printf '%s\n' '== UI format labels and parser selection serialization =='
sed -n '250,350p' src/stores/log-store.ts
rg -n -C 6 --glob '*.{rs,ts,tsx}' \
'parserSelection|parser_selection|formatDetected|format_detected' \
src src-tauri/src crates/cmtraceopen-parser/srcLength of output: 214166
Defect: The tail reader does not preserve Company Portal logical records across updates. In Add Company Portal framing to the stateful logical-record path, or retain its final parsed header until the next record header, rotation, or debounce flush. Add a tail regression test that appends a header and continuation in separate writes. The public re-export suggestion is not valid. The API is already public at:
The public module chain and the ✅ Action performedFull review finished. |
Exact-head review checkpoint — not merge-readyIndependent and hosted review have identified two verified no-go behaviors on
The recovery branch is intentionally unchanged. A new isolated correction branch begins at this exact head with separate failing tests and issue-scoped commits; it will require a fresh independent review before any push. Local CodeRabbit review reported nine findings. Disposition after code inspection:
Hosted CI is still running. The Windows ESP diagnostic job currently reports failure while its enclosing run is still finalizing; its log is not available yet, so no cause is claimed. |
There was a problem hiding this comment.
Pull request overview
Adds first dedicated raw-format parser for Windows Company Portal LocalState logs (Log_<n>.log and Log.<BridgeName>_<n>.log) to the cmtraceopen-parser Intune/Portal surface, including conservative detection, logical-record framing, LogEntry projection, redacted evidence document output, and end-to-end registration in the app + frontend.
Changes:
- Implement
cmtraceopen_parser::intune::portal::windows::company_portal::logs(grammar, detection, framing, viewer entries, evidence document, redaction, models). - Register the new parser kind/implementation across backend + frontend, with corpus fixtures and contract tests.
- Update references/docs and CI/gitattributes to support byte-sensitive fixtures and run the full parser-crate test/clippy gates.
Reviewed changes
Copilot reviewed 22 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/log.ts | Add companyPortal to frontend parser kind/implementation unions. |
| src/stores/log-store.ts | Add UI labels for companyPortal parser kind/implementation. |
| src/lib/column-config.ts | Define default column set for companyPortal logs. |
| src-tauri/tests/parser_supported_formats.rs | Add Company Portal to supported-parser contract + add fixture-based detection/parse tests. |
| src-tauri/tests/corpus/company_portal/negative/Log_1.log | Add negative corpus sample to ensure detection doesn’t rely on filename/path alone. |
| src-tauri/tests/corpus/company_portal/clean/Log_1.log | Add positive corpus sample used by native app contract tests. |
| src-tauri/src/commands/bundle_ops.rs | Add selection description string for Company Portal parser. |
| references/log-intune-reference.md | Document Company Portal log grammar and version-scoping limitations. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v13-4-2/unknown-app-version/Log_1.log | Fixture exercising unknown-version downgrade behavior. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/truncated-boundaries/Log_1.log | Fixture for truncated first/last record handling. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/severity-levels/Log_1.log | Fixture covering multiple severity tokens (incl. unknown). |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/same-timestamp-distinct-activity/Log_1.log | Fixture ensuring distinct activity IDs aren’t merged when timestamps match. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_2.log | Fixture for rotated member behavior. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_1.log | Fixture for rotated member behavior (current). |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/redaction/Log_1.log | Fixture for redaction of synthetic sensitive values. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-unrelated-uwp/Log_1.log | Negative fixture for unrelated UWP log collision. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-generic-timestamped/Log_1.log | Negative fixture for generic timestamped text logs. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/multiline-continuation/Log_1.log | Fixture for logical-record framing of continuation lines. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/malformed-structural-token/Log_1.log | Fixture for malformed structural token handling. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/invalid-timestamp/Log_1.log | Fixture ensuring invalid timestamps become parse errors, not “best-effort” timestamps. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-nobom/Log_1.log | UTF-8 no-BOM encoding fixture. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-bom/Log_1.log | UTF-8 BOM encoding fixture. |
| crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/code-tokens/Log_1.log | Fixture ensuring known/unknown code tokens are preserved. |
| crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs | Add dedicated Company Portal contract test suite (fixtures, framing, redaction, downgrade, negatives). |
| crates/cmtraceopen-parser/src/parser/mod.rs | Route ParserImplementation::CompanyPortal to new parser entrypoint. |
| crates/cmtraceopen-parser/src/parser/detect.rs | Add Company Portal detection (path hint + strict record-structure confirmation + heuristic downgrade). |
| crates/cmtraceopen-parser/src/models/log_entry.rs | Add CompanyPortal variants to ParserKind and ParserImplementation. |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/redaction.rs | Implement redacted export projection using shared ESP redaction rules. |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rs | Define serde-stable evidence document wire types (records, coverage, schema versioning). |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs | Replace skeleton with full module docs/exports and module wiring. |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/grammar.rs | Implement V1 record grammar parsing and strict structure checks. |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/framing.rs | Implement logical-record framing shared by viewer and evidence document. |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rs | Implement LogEntry projection for viewer (no redaction). |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rs | Build canonical evidence document (redacted-by-default) + coverage reporting. |
| crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rs | Implement file-identity parsing and per-line structural classification for detection. |
| crates/cmtraceopen-parser/src/esp/redaction.rs | Expose redaction entrypoint as pub(crate) for reuse by other evidence modules. |
| crates/cmtraceopen-parser/src/esp/mod.rs | Re-export crate-internal redact_text helper for sibling modules. |
| Cargo.lock | Update lockfile for workspace version bump to 1.5.1. |
| .github/workflows/cmtrace-ci.yml | Run full parser-crate tests + parser-crate clippy in CI (not just ESP suite). |
| .gitattributes | Mark parser fixtures/corpus as byte-sensitive (no EOL normalization; disable whitespace errors). |
| /// `true` when the record could not be read as a well-formed record. | ||
| pub(super) fn is_parse_error(&self) -> bool { | ||
| !matches!(self.kind, FramedRecordKind::Record(_)) | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/cmtrace-ci.yml:
- Around line 245-247: Update the comment near the Windows parser-crate targets
to remove the claim that this is the only place parser tests execute, and state
instead that the job adds Windows-specific validation. Leave the workflow
commands unchanged.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rs`:
- Around line 17-39: Remove the constant is_record field from
CompanyPortalLineClassification and delete its documentation and initializer in
classify_line. Keep classify_line returning None for non-record lines and retain
app_version_is_validated as the sole classification fact.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rs`:
- Around line 146-159: Update the coverage status construction in the document
parser so total_records == 0 is handled before the unreadable == 0 check and
does not report CompanyPortalCoverageStatus::Available. Preserve the existing
Available status only for non-empty content with no unreadable records, while
retaining ParseFailed for records that fail to parse.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs`:
- Around line 48-53: Remove the `pub use grammar::*;` re-export from the logs
module’s public exports, while keeping the underlying `grammar` module private.
Leave the other public re-exports unchanged so only the grammar implementation
items stop being exposed.
In `@crates/cmtraceopen-parser/src/parser/mod.rs`:
- Around line 146-148: Update the Company Portal parsing flow around
ParserImplementation::CompanyPortal and ResolvedParser::company_portal() so
inventory_logical_dialect() includes Company Portal and its logical-record state
is persisted in TailReader. Ensure pending records are flushed at the next
header, debounce boundary, file rotation, and session shutdown, and add a
regression test covering a split write between header and continuation lines.
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs`:
- Around line 700-732: Update
portal_logs_every_fixture_line_survives_into_a_record so each non-empty source
line is matched against at least one individual record.raw_text, rather than the
newline-joined joined string. Preserve the existing trimming and failure
context, but ensure matches cannot come from substrings in unrelated records or
across record boundaries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f9eb6ef9-6271-4645-a44b-bb833e93ec63
⛔ Files ignored due to path filters (19)
Cargo.lockis excluded by!**/*.lock,!Cargo.lockcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/code-tokens/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf16le/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-bom/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-nobom/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/invalid-timestamp/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/malformed-structural-token/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/multiline-continuation/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-generic-timestamped/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-unrelated-uwp/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/redaction/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_2.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/same-timestamp-distinct-activity/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/severity-levels/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/truncated-boundaries/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v13-4-2/unknown-app-version/Log_1.logis excluded by!**/*.logsrc-tauri/tests/corpus/company_portal/clean/Log_1.logis excluded by!**/*.logsrc-tauri/tests/corpus/company_portal/negative/Log_1.logis excluded by!**/*.log
📒 Files selected for processing (22)
.gitattributes.github/workflows/cmtrace-ci.ymlcrates/cmtraceopen-parser/src/esp/mod.rscrates/cmtraceopen-parser/src/esp/redaction.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/framing.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/grammar.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/redaction.rscrates/cmtraceopen-parser/src/models/log_entry.rscrates/cmtraceopen-parser/src/parser/detect.rscrates/cmtraceopen-parser/src/parser/mod.rscrates/cmtraceopen-parser/tests/company_portal_windows_logs.rsreferences/log-intune-reference.mdsrc-tauri/src/commands/bundle_ops.rssrc-tauri/tests/parser_supported_formats.rssrc/lib/column-config.tssrc/stores/log-store.tssrc/types/log.ts
| coverage.push(CompanyPortalCoverage { | ||
| artifact_id: FILE_COVERAGE_ARTIFACT_ID.to_string(), | ||
| family: COVERAGE_FAMILY.to_string(), | ||
| status: if unreadable == 0 { | ||
| CompanyPortalCoverageStatus::Available | ||
| } else { | ||
| CompanyPortalCoverageStatus::ParseFailed | ||
| }, | ||
| detail: Some(format!( | ||
| "{} read {parsed_count} of {total_records} record(s) with grammar V1; \ | ||
| {unreadable} record(s) did not match and are preserved as source text.", | ||
| file.file_name | ||
| )), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
An empty file reports Available coverage for zero records.
For empty or fully truncated content, total_records is 0, so unreadable == 0 and the file row claims Available with the detail "read 0 of 0 record(s)". A rotated file truncated to zero bytes is reachable. The document then asserts full coverage of an artifact from which nothing was read, which is the exact confusion the module doc says coverage exists to prevent. Branch on total_records == 0 first.
🐛 Proposed fix
- status: if unreadable == 0 {
+ status: if total_records == 0 {
+ // Nothing was read, so nothing is covered.
+ CompanyPortalCoverageStatus::Unsupported
+ } else if unreadable == 0 {
CompanyPortalCoverageStatus::Available
} else {
CompanyPortalCoverageStatus::ParseFailed
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| coverage.push(CompanyPortalCoverage { | |
| artifact_id: FILE_COVERAGE_ARTIFACT_ID.to_string(), | |
| family: COVERAGE_FAMILY.to_string(), | |
| status: if unreadable == 0 { | |
| CompanyPortalCoverageStatus::Available | |
| } else { | |
| CompanyPortalCoverageStatus::ParseFailed | |
| }, | |
| detail: Some(format!( | |
| "{} read {parsed_count} of {total_records} record(s) with grammar V1; \ | |
| {unreadable} record(s) did not match and are preserved as source text.", | |
| file.file_name | |
| )), | |
| }); | |
| coverage.push(CompanyPortalCoverage { | |
| artifact_id: FILE_COVERAGE_ARTIFACT_ID.to_string(), | |
| family: COVERAGE_FAMILY.to_string(), | |
| status: if total_records == 0 { | |
| // Nothing was read, so nothing is covered. | |
| CompanyPortalCoverageStatus::Unsupported | |
| } else if unreadable == 0 { | |
| CompanyPortalCoverageStatus::Available | |
| } else { | |
| CompanyPortalCoverageStatus::ParseFailed | |
| }, | |
| detail: Some(format!( | |
| "{} read {parsed_count} of {total_records} record(s) with grammar V1; \ | |
| {unreadable} record(s) did not match and are preserved as source text.", | |
| file.file_name | |
| )), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rs`
around lines 146 - 159, Update the coverage status construction in the document
parser so total_records == 0 is handled before the unreadable == 0 check and
does not report CompanyPortalCoverageStatus::Available. Preserve the existing
Available status only for non-empty content with no unreadable records, while
retaining ParseFailed for records that fail to parse.
|
Pause checkpoint: no correction commit was pushed and no review thread was changed. Remote draft head remains d946b1c. The isolated correction branch is clean locally at 04e1ecb, tree cb356b85. Fresh focused results at that local head: store 39/39, hook 7/7, tail module 29/29, TypeScript noEmit, plus the stale-seed, duplicate-key, blank-line, and empty-logical regressions. A newer exact CodeRabbit run was stopped during wrap-up after identifying three valid remaining validator gaps: observedThroughLine must dominate entry/amendment ranges; amendment start/span bounds must tighten; every optional LogEntry field and message span needs runtime validation. Resume RED-first on those three, then rerun full frontend/app/parser/wasm/strict-Clippy gates, obtain a terminal exact CodeRabbit review and independent GO, and only then publish the reviewed correction without force. |
|
Preservation update: the local Intune correction series is now published on branch codex/intune-366-corrections-r125 at commit 04e1ecb. This PR remains on its existing head and was not advanced. Fresh focused results at the checkpoint: store 39/39, hook 7/7, tail 29/29, named seed/key/blank/empty cases, and TypeScript noEmit all passed. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ections-r125-1785804746
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Correction checkpoint: What moved:
Exact-head local gates:
CI state:
|
Resolve the sole .gitattributes conflict by keeping byte-sensitive parser fixture attributes for Company Portal/SCCM corpora (-text -whitespace) and the Company Portal corpus path, so PR #460 is conflict-free against current main without force-pushing history.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Lane A restack verification (Ivy)Restacked onto current Local gates (post-merge)
Conflict resolutionOnly Refs #366 |
…#460) - Drop always-true `is_record` from line classification (semver surface) - Stop re-exporting private grammar helpers; keep only `looks_like_record_start` - Match parse_state on borrowed kind arms in document builder - Correct ESP Windows job comment (parser tests also run on Linux check) - Strengthen lossless fixture test to exact line multiset membership Verified: company_portal_windows_logs (34) + logs unit tests (41) pass.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs`:
- Around line 48-53: Preserve the public grammar exports in the logs module by
retaining the grammar glob re-export, including parse_record_fields,
leading_component, and CompanyPortalRecordFields. Do not replace or remove pub
use grammar::* in this 0.1.1 crate; defer any API cleanup to a planned breaking
release.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rs`:
- Around line 25-30: Add #[non_exhaustive] to the public
CompanyPortalGrammarVersion and CompanyPortalTimestampKind enums, preserving
their existing derives, serde attributes, variants, and documentation.
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs`:
- Around line 277-287: Update build_coverage so inputs producing zero parsed
records are assigned CompanyPortalCoverageStatus::ParseFailed rather than
Available. Preserve the existing coverage behavior for inputs with parsed
records, and ensure the empty document path used by
portal_logs_empty_input_is_not_available_coverage passes.
In `@src-tauri/src/commands/file_ops.rs`:
- Around line 711-727: Update index_aggregate_entries to index only tail-capable
Company Portal files, using the existing supported-file predicate or symbol.
When duplicate coordinates occur, skip or mark that file’s seed unavailable
instead of returning an AppError, so open_log_folder_aggregate continues
processing other files.
In `@src/lib/tail-payload-validation.test.ts`:
- Around line 248-267: The invalid optional-field table in the “rejects an
invalid optional LogEntry” test uses `{}` for every non-tags field, so it does
not validate each field’s constraints. Replace those placeholders with
field-appropriate invalid values, especially plausible unknown strings for
severity, format, and entryKind, and add explicit rejection cases covering
unknown values against the SEVERITIES, LOG_FORMATS, and ENTRY_KINDS-backed
fields while preserving the existing tags case.
- Around line 236-246: Update the assertion in the large-batch test “validates
large tail batches without spreading them into function arguments” to use
identity comparison with the original value instead of deep structural
comparison. Keep the existing parseTailPayload invocation and large-entry setup
unchanged.
In `@src/lib/tail-payload-validation.ts`:
- Around line 318-324: The payload validation branch around highestObservedLine
currently rejects the entire batch when observedThroughLine is null or lower
than the highest entry/amendment line. Replace that rejection with clamping
observedThroughLine to highestObservedLine and report the coverage
inconsistency, while preserving rejection for genuinely invalid state such as
out-of-range spans.
- Around line 161-166: Update physicalEndLine to count newline characters
without using message.split or allocating an array, then reuse the computed
physical end line from isLogEntry when parseTailPayload processes the same entry
instead of calling physicalEndLine again. Preserve the existing safe-integer
validation and null behavior.
- Around line 14-67: Replace the hand-maintained Set initializers LOG_FORMATS,
PARSER_KINDS, and PARSER_IMPLEMENTATIONS with exhaustive Record-based allowlists
keyed by each corresponding union, then derive each Set from the record keys.
Ensure adding a future variant to any union without updating its record causes
TypeScript compilation to fail while preserving the existing membership checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef24fcf4-a31b-4a1e-a3b4-b26ea76b15af
⛔ Files ignored due to path filters (18)
crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/code-tokens/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf16le/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-bom/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-nobom/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/invalid-timestamp/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/malformed-structural-token/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/multiline-continuation/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-generic-timestamped/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-unrelated-uwp/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/redaction/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_2.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/same-timestamp-distinct-activity/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/severity-levels/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/truncated-boundaries/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v13-4-2/unknown-app-version/Log_1.logis excluded by!**/*.logsrc-tauri/tests/corpus/company_portal/clean/Log_1.logis excluded by!**/*.logsrc-tauri/tests/corpus/company_portal/negative/Log_1.logis excluded by!**/*.log
📒 Files selected for processing (31)
.gitattributes.github/workflows/cmtrace-ci.ymlcrates/cmtraceopen-parser/src/esp/mod.rscrates/cmtraceopen-parser/src/esp/redaction.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/framing.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/grammar.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/redaction.rscrates/cmtraceopen-parser/src/models/log_entry.rscrates/cmtraceopen-parser/src/parser/detect.rscrates/cmtraceopen-parser/src/parser/mod.rscrates/cmtraceopen-parser/tests/company_portal_windows_logs.rsreferences/log-intune-reference.mdsrc-tauri/src/commands/bundle_ops.rssrc-tauri/src/commands/file_ops.rssrc-tauri/src/commands/parsing.rssrc-tauri/src/state/app_state.rssrc-tauri/src/watcher/tail.rssrc-tauri/tests/parser_supported_formats.rssrc/hooks/use-file-watcher.test.tsxsrc/hooks/use-file-watcher.tssrc/lib/column-config.tssrc/lib/tail-payload-validation.test.tssrc/lib/tail-payload-validation.tssrc/stores/log-store.test.tssrc/stores/log-store.tssrc/types/log.ts
| pub use detect::*; | ||
| pub use document::*; | ||
| pub use entries::*; | ||
| pub use grammar::*; | ||
| pub use models::*; | ||
| pub use redaction::*; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Resolve every external reference to the grammar items published by the glob.
set -euo pipefail
logs_dir='crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs'
echo '== public items in grammar.rs =='
ast-grep outline "$logs_dir/grammar.rs" --items all
echo '== references outside the logs module =='
rg -nP --type=rust -C2 \
'\b(parse_record_fields|looks_like_record_start|leading_component|CompanyPortalRecordFields)\b' \
-g "!$logs_dir/**"Repository: adamgell/cmtraceopen
Length of output: 2009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
logs_dir='crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs'
printf '%s\n' '== grammar public declarations =='
rg -n '^(pub(\([^)]*\))?[[:space:]]+(struct|enum|fn|type|const|static)|^pub[[:space:]]+use)' \
"$logs_dir/grammar.rs"
printf '%s\n' '== all grammar item references outside logs =='
rg -nP --type=rust -C1 \
'\b(parse_record_fields|looks_like_record_start|leading_component|CompanyPortalRecordFields)\b' \
-g "!$logs_dir/**" . || true
printf '%s\n' '== module re-exports and crate declarations =='
cat -n "$logs_dir/mod.rs"
rg -n 'cmtraceopen_parser|cmtraceopen-parser|frame_company_portal_logical_records|looks_like_record_start' \
--glob '*.rs' --glob '*.toml' .
printf '%s\n' '== relevant tail context =='
tail_file="$(rg --files | rg '(^|/)tail\.rs$' | head -n1)"
if [ -n "$tail_file" ]; then
sed -n '910,990p' "$tail_file"
else
echo 'tail.rs not found'
fiRepository: adamgell/cmtraceopen
Length of output: 38074
🏁 Script executed:
#!/bin/bash
set -euo pipefail
logs_dir='crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs'
printf '%s\n' '== package version and repository state =='
rg -n '^(version|name)[[:space:]]*=' crates/cmtraceopen-parser/Cargo.toml
git status --short
git diff --numstat -- "$logs_dir/mod.rs" "$logs_dir/grammar.rs"
printf '%s\n' '== all Windows log test references to grammar exports =='
rg -n -C1 \
'\b(parse_record_fields|looks_like_record_start|leading_component|CompanyPortalRecordFields)\b|logs::\*' \
crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs \
"$logs_dir"
printf '%s\n' '== public signatures that expose grammar types =='
rg -n -C2 \
'pub (fn|struct|enum|type|use)|CompanyPortalRecordFields|parse_record_fields|leading_component' \
"$logs_dir"/*.rsRepository: adamgell/cmtraceopen
Length of output: 48787
Preserve the public grammar exports. This 0.1.1 crate exposes parse_record_fields, leading_component, and CompanyPortalRecordFields through the glob. Replacing it removes public API items and causes a semver break. Keep the glob or defer removal to a planned breaking release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs`
around lines 48 - 53, Preserve the public grammar exports in the logs module by
retaining the grammar glob re-export, including parse_record_fields,
leading_component, and CompanyPortalRecordFields. Do not replace or remove pub
use grammar::* in this 0.1.1 crate; defer any API cleanup to a planned breaking
release.
Source: Path instructions
| #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub enum CompanyPortalGrammarVersion { | ||
| /// Field layout observed in app version `12-0-0`. | ||
| V1, | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Mark the version-scoped enums #[non_exhaustive].
CompanyPortalGrammarVersion and CompanyPortalTimestampKind each hold exactly one variant today, and both doc comments state that more variants are expected (V2 for a second observed layout, and the timestamp doc reserves the question of other resolutions). In a published crate, adding a variant later breaks every downstream exhaustive match. Add #[non_exhaustive] now, while the cost is zero.
♻️ Proposed change
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
+#[non_exhaustive]
pub enum CompanyPortalGrammarVersion {
/// Field layout observed in app version `12-0-0`.
V1,
} #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
+#[non_exhaustive]
pub enum CompanyPortalTimestampKind {
/// Resolved to an absolute UTC instant.
Utc,
}As per path instructions: "Treat every public item as a semver commitment: flag breaking changes to public types, signatures, or enum variants".
Also applies to: 89-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rs`
around lines 25 - 30, Add #[non_exhaustive] to the public
CompanyPortalGrammarVersion and CompanyPortalTimestampKind enums, preserving
their existing derives, serde attributes, variants, and documentation.
Source: Path instructions
| fn index_aggregate_entries( | ||
| entries: &[LogEntry], | ||
| ) -> Result<std::collections::HashMap<(&str, u32), &LogEntry>, crate::error::AppError> { | ||
| let mut lookup = std::collections::HashMap::new(); | ||
| for entry in entries { | ||
| if lookup | ||
| .insert((entry.file_path.as_str(), entry.line_number), entry) | ||
| .is_some() | ||
| { | ||
| return Err(crate::error::AppError::Internal(format!( | ||
| "duplicate aggregate entry for {} at physical line {}", | ||
| entry.file_path, entry.line_number | ||
| ))); | ||
| } | ||
| } | ||
| Ok(lookup) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for parsers that emit several entries for one physical line number.
set -uo pipefail
# Find parse_lines implementations and inspect how line_number is assigned.
fd -e rs . crates/cmtraceopen-parser/src --exec rg -nP -C4 'line_number\s*[:=]' {} \
| rg -n -C4 'line_number' \
| head -200
# Look for sites that push more than one entry inside a single line iteration.
ast-grep run --lang rust --pattern $'for $_ in $_ {
$$$
$ENTRIES.push($_);
$$$
$ENTRIES.push($_);
$$$
}' crates/cmtraceopen-parser/srcRepository: adamgell/cmtraceopen
Length of output: 8349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- file_ops.rs relevant symbols ---'
rg -n -C12 'open_log_folder_aggregate|index_aggregate_entries|supports_parser|tail|Company Portal|aggregate' src-tauri/src/commands/file_ops.rs
printf '%s\n' '--- parser files ---'
fd -e rs . crates/cmtraceopen-parser/src | sort
printf '%s\n' '--- parser entry construction and aggregation ---'
rg -n -C8 'line_number_for_offset|LogEntry \{|entries\.push|parse_lines|Company Portal|company.?portal' crates/cmtraceopen-parser/src | head -500
printf '%s\n' '--- LogEntry and AppError definitions ---'
rg -n -C12 'struct LogEntry|enum AppError|type AppError' src-tauri/src crates/cmtraceopen-parser/srcRepository: adamgell/cmtraceopen
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- parser selection and tail-seed logic ---'
rg -n -C15 'enum ParserSelection|struct ParserSelection|supports_parser|from_entry|CompanyPortal|company_portal|WindowsCompanyPortal' \
crates/cmtraceopen-parser/src src-tauri/src/watcher src-tauri/src/commands/file_ops.rs
printf '%s\n' '--- parser entry points ---'
rg -n 'pub fn parse_lines|fn parse_lines|parse_lines\(' crates/cmtraceopen-parser/src/parser crates/cmtraceopen-parser/src/intune/portal/windows/company_portal
printf '%s\n' '--- all direct line-number assignments in parser source ---'
rg -n -P 'line_number\s*:\s*|\.line_number\s*=' crates/cmtraceopen-parser/src/parser crates/cmtraceopen-parser/src/intune/portal/windows/company_portal
printf '%s\n' '--- parser dispatch ---'
rg -n -C20 'parse_file|match.*parser|ParserKind|parser_selection' crates/cmtraceopen-parser/src src-tauri/src/parser.rs src-tauri/src/commands/file_ops.rsRepository: adamgell/cmtraceopen
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tail seed implementation ---'
rg -n -C20 'struct InitialLogicalRecord|impl InitialLogicalRecord|supports_parser|from_entry|from_parse_result' \
src-tauri/src/watcher/tail.rs src-tauri/src/watcher
printf '%s\n' '--- parser dispatch ---'
sed -n '1,175p' crates/cmtraceopen-parser/src/parser/mod.rs
printf '%s\n' '--- Company Portal framing ---'
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/framing.rs
printf '%s\n' '--- Company Portal document and entries ---'
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rs
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rs
printf '%s\n' '--- parser selection model ---'
rg -n -C8 'pub enum ParserImplementation|pub struct ResolvedParser|CompanyPortal|CompanyPortalWindows|RecordFraming' \
crates/cmtraceopen-parser/src/models/log_entry.rs crates/cmtraceopen-parser/src/parser/mod.rsRepository: adamgell/cmtraceopen
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Company Portal framing ---'
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/framing.rs
printf '%s\n' '--- Company Portal document ---'
cat -n crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rs
printf '%s\n' '--- Company Portal entry model and construction ---'
sed -n '1,180p' crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rs
sed -n '180,280p' crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rs
printf '%s\n' '--- Company Portal parse function ---'
rg -n -C25 'pub fn parse_lines|fn parse_lines' \
crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logsRepository: adamgell/cmtraceopen
Length of output: 35770
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("crates/cmtraceopen-parser/src")
framing = (root / "intune/portal/windows/company_portal/logs/framing.rs").read_text()
assert "pub fn frame_records" in framing
assert "(index + 1) as u32" in framing
assert framing.count("records.push(FramedRecord") == 3
assert "line_number: (index + 1) as u32" in framing
print("Company Portal framing: every record push uses the current physical line index")
for path in sorted(root.rglob("*.rs")):
text = path.read_text()
if "parse_lines" not in text:
continue
pushes = len(re.findall(r"\bentries\.push\s*\(", text))
assignments = len(re.findall(r"\bline_number\s*:", text))
if pushes:
print(f"{path}: entries.push={pushes}, line_number fields={assignments}")
PYRepository: adamgell/cmtraceopen
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("crates/cmtraceopen-parser/src")
framing = (root / "intune/portal/windows/company_portal/logs/framing.rs").read_text()
checks = {
"frame_records exists": "frame_records<'a>" in framing,
"physical line assignment exists": "line_number: (index + 1) as u32" in framing,
"three mutually exclusive push branches present":
len(re.findall(r"\brecords\.push\s*\(\s*FramedRecord\s*\{", framing)) == 3,
}
for name, passed in checks.items():
print(f"{name}: {'PASS' if passed else 'FAIL'}")
assert all(checks.values())
print("Company Portal framing assigns each emitted record the current physical line number.")
for path in sorted(root.rglob("*.rs")):
text = path.read_text()
if "parse_lines" not in text:
continue
pushes = len(re.findall(r"\bentries\.push\s*\(", text))
if pushes:
print(f"{path}: entries.push={pushes}")
PYRepository: adamgell/cmtraceopen
Length of output: 1624
Scope the aggregate index to tail-capable files. The lookup is queried only for the final entry of Company Portal files, but it currently allocates a bucket for every aggregate entry. Restrict the index to supported files and let a duplicate coordinate disable that file’s seed instead of aborting open_log_folder_aggregate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/commands/file_ops.rs` around lines 711 - 727, Update
index_aggregate_entries to index only tail-capable Company Portal files, using
the existing supported-file predicate or symbol. When duplicate coordinates
occur, skip or mark that file’s seed unavailable instead of returning an
AppError, so open_log_folder_aggregate continues processing other files.
| it("validates large tail batches without spreading them into function arguments", () => { | ||
| const entries = Array.from({ length: 200_000 }, (_, index) => | ||
| entry({ id: index, lineNumber: index + 1 }), | ||
| ); | ||
| const value = payload({ | ||
| entries, | ||
| observedThroughLine: entries.length, | ||
| }); | ||
|
|
||
| expect(parseTailPayload(value)).toEqual(value); | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Use toBe for the 200,000-entry batch.
parseTailPayload returns the same object reference it was given at Line 326 of src/lib/tail-payload-validation.ts. toEqual therefore performs a deep structural comparison of 200,000 objects to prove something identity already proves. toBe(value) asserts the same contract in constant time and keeps the suite fast.
♻️ Identity assertion
- expect(parseTailPayload(value)).toEqual(value);
+ expect(parseTailPayload(value)).toBe(value);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("validates large tail batches without spreading them into function arguments", () => { | |
| const entries = Array.from({ length: 200_000 }, (_, index) => | |
| entry({ id: index, lineNumber: index + 1 }), | |
| ); | |
| const value = payload({ | |
| entries, | |
| observedThroughLine: entries.length, | |
| }); | |
| expect(parseTailPayload(value)).toEqual(value); | |
| }); | |
| it("validates large tail batches without spreading them into function arguments", () => { | |
| const entries = Array.from({ length: 200_000 }, (_, index) => | |
| entry({ id: index, lineNumber: index + 1 }), | |
| ); | |
| const value = payload({ | |
| entries, | |
| observedThroughLine: entries.length, | |
| }); | |
| expect(parseTailPayload(value)).toBe(value); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tail-payload-validation.test.ts` around lines 236 - 246, Update the
assertion in the large-batch test “validates large tail batches without
spreading them into function arguments” to use identity comparison with the
original value instead of deep structural comparison. Keep the existing
parseTailPayload invocation and large-entry setup unchanged.
| it.each( | ||
| Object.keys(validOptionalFields).map((field) => [ | ||
| field, | ||
| field === "tags" ? ["valid", 1] : {}, | ||
| ]), | ||
| )("rejects an invalid optional LogEntry %s", (field, invalidValue) => { | ||
| expect( | ||
| parseTailPayload( | ||
| payload({ | ||
| entries: [ | ||
| { | ||
| ...entry(), | ||
| [field]: invalidValue, | ||
| }, | ||
| ], | ||
| observedThroughLine: 1, | ||
| }), | ||
| ), | ||
| ).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Every non-tags case uses {}, so this table proves only that an object is refused.
The invalid value at Line 251 is {} for all fields except tags. That triggers rejection for the same trivial reason in every case. It does not exercise the constraint each field actually declares.
The gap that matters is the string enums. severity, format, and entryKind gate every entry through SEVERITIES, LOG_FORMATS, and ENTRY_KINDS, and no test supplies a plausible wrong string such as "Bogus" or "Verbose". Those sets are hand-maintained mirrors of Rust enums, so a wrong-string case is the realistic failure and it is untested.
Add per-field invalid values that match each field's declared type, and add explicit rejection cases for an unknown severity, format, and entryKind.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tail-payload-validation.test.ts` around lines 248 - 267, The invalid
optional-field table in the “rejects an invalid optional LogEntry” test uses
`{}` for every non-tags field, so it does not validate each field’s constraints.
Replace those placeholders with field-appropriate invalid values, especially
plausible unknown strings for severity, format, and entryKind, and add explicit
rejection cases covering unknown values against the SEVERITIES, LOG_FORMATS, and
ENTRY_KINDS-backed fields while preserving the existing tags case.
| const LOG_FORMATS = new Set<LogFormat>([ | ||
| "Ccm", | ||
| "Simple", | ||
| "Plain", | ||
| "Timestamped", | ||
| "DnsDebug", | ||
| "DnsAudit", | ||
| "CmtLog", | ||
| ]); | ||
| const PARSER_KINDS = new Set<ParserKind>([ | ||
| "ccm", | ||
| "simple", | ||
| "timestamped", | ||
| "plain", | ||
| "iisW3c", | ||
| "panther", | ||
| "cbs", | ||
| "dism", | ||
| "reportingEvents", | ||
| "msi", | ||
| "psadtLegacy", | ||
| "intuneMacOs", | ||
| "intuneDeviceInventory", | ||
| "dhcp", | ||
| "burn", | ||
| "patchMyPcDetection", | ||
| "registry", | ||
| "secureBootLog", | ||
| "dnsDebug", | ||
| "dnsAudit", | ||
| "cmtLog", | ||
| "companyPortal", | ||
| ]); | ||
| const PARSER_IMPLEMENTATIONS = new Set<ParserImplementation>([ | ||
| "ccm", | ||
| "simple", | ||
| "genericTimestamped", | ||
| "iisW3c", | ||
| "reportingEvents", | ||
| "plainText", | ||
| "msi", | ||
| "psadtLegacy", | ||
| "intuneMacOs", | ||
| "intuneDeviceInventory", | ||
| "dhcp", | ||
| "burn", | ||
| "patchMyPcDetection", | ||
| "registry", | ||
| "secureBootLog", | ||
| "dnsDebug", | ||
| "dnsAudit", | ||
| "cmtLog", | ||
| "companyPortal", | ||
| ]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Make these allowlists fail at compile time when a Rust variant is added.
LOG_FORMATS, PARSER_KINDS, and PARSER_IMPLEMENTATIONS are hand-maintained mirrors of Rust enums. The typed Set<ParserKind> annotation catches an entry that is not part of the union, but it cannot catch a missing entry. That is the failure direction that matters here.
If a future variant is added to ParserKind and not added at Line 23, isParserSelection rejects the payload, parseTailPayload returns null, and use-file-watcher.ts drops the batch with only a console.error. Tailing then stops for that parser with no visible cause. This PR already had to hand-add companyPortal in two places.
Build each set from an exhaustive record so npx tsc --noEmit fails when a variant is missing.
♻️ Compile-enforced allowlists
-const PARSER_KINDS = new Set<ParserKind>([
- "ccm",
- "simple",
+const PARSER_KIND_MEMBERS: Record<ParserKind, true> = {
+ ccm: true,
+ simple: true,
// ...every remaining variant, each required by the Record type
- "companyPortal",
-]);
+ companyPortal: true,
+};
+const PARSER_KINDS = new Set<ParserKind>(
+ Object.keys(PARSER_KIND_MEMBERS) as ParserKind[],
+);Apply the same shape to LOG_FORMATS and PARSER_IMPLEMENTATIONS.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tail-payload-validation.ts` around lines 14 - 67, Replace the
hand-maintained Set initializers LOG_FORMATS, PARSER_KINDS, and
PARSER_IMPLEMENTATIONS with exhaustive Record-based allowlists keyed by each
corresponding union, then derive each Set from the record keys. Ensure adding a
future variant to any union without updating its record causes TypeScript
compilation to fail while preserving the existing membership checks.
| function physicalEndLine(entry: LogEntry): number | null { | ||
| const endLine = entry.lineNumber + entry.message.split("\n").length - 1; | ||
| return isSafeInteger(endLine, entry.lineNumber, 4_294_967_295) | ||
| ? endLine | ||
| : null; | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
physicalEndLine splits every message twice and allocates an array each time.
isLogEntry calls physicalEndLine at Line 217, and parseTailPayload calls it again for the same entry at Line 301. Each call runs message.split("\n"), which allocates one array per message. The cost is two full passes over the message bytes of the whole batch, plus the garbage. The test at Line 236 of src/lib/tail-payload-validation.test.ts already exercises 200,000 entries.
Count newlines without allocating, and reuse the value computed during entry validation.
♻️ Allocation-free line count
function physicalEndLine(entry: LogEntry): number | null {
- const endLine = entry.lineNumber + entry.message.split("\n").length - 1;
+ let newlines = 0;
+ for (let index = 0; index < entry.message.length; index += 1) {
+ if (entry.message.charCodeAt(index) === 10) {
+ newlines += 1;
+ }
+ }
+ const endLine = entry.lineNumber + newlines;
return isSafeInteger(endLine, entry.lineNumber, 4_294_967_295)
? endLine
: null;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tail-payload-validation.ts` around lines 161 - 166, Update
physicalEndLine to count newline characters without using message.split or
allocating an array, then reuse the computed physical end line from isLogEntry
when parseTailPayload processes the same entry instead of calling
physicalEndLine again. Preserve the existing safe-integer validation and null
behavior.
| if ( | ||
| highestObservedLine !== null && | ||
| (payload.observedThroughLine === null || | ||
| payload.observedThroughLine < highestObservedLine) | ||
| ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
A low observedThroughLine discards the whole batch, including valid entries.
This check rejects the entire payload when observedThroughLine is below the highest line implied by entries or amendments. The caller then logs one line and drops every entry in the batch.
That couples a backend coverage-accounting mistake to total data loss in the UI. The physical_lines under-count I flagged at src-tauri/src/watcher/tail.rs Lines 955 to 966 produces exactly this shape: parse_logical_records derives observed_through_line from next_line, so an under-count makes the reported coverage fall below an entry's own end line and this branch drops records the backend parsed correctly.
Consider clamping observedThroughLine up to highestObservedLine and reporting the inconsistency, rather than discarding parsed entries. Reserve rejection for fields that would corrupt state, such as out-of-range spans.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tail-payload-validation.ts` around lines 318 - 324, The payload
validation branch around highestObservedLine currently rejects the entire batch
when observedThroughLine is null or lower than the highest entry/amendment line.
Replace that rejection with clamping observedThroughLine to highestObservedLine
and report the coverage inconsistency, while preserving rejection for genuinely
invalid state such as out-of-range spans.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs (1)
51-53:⚠️ Potential issue | 🟠 MajorRestore the previous public grammar exports.
This re-export keeps only
looks_like_record_start. It removes the previously exportedparse_record_fields,leading_component, andCompanyPortalRecordFields. Downstream crates that use these items will fail to compile. Keep the existing exports, or defer the removal to a planned breaking release.As per path instructions: "Treat every public item as a semver commitment."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs` around lines 51 - 53, Restore the public re-exports for parse_record_fields, leading_component, and CompanyPortalRecordFields alongside looks_like_record_start in the grammar exports of the logs module. Preserve downstream access to all previously public items and do not make their removal part of this change.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs`:
- Around line 713-715: After the source-line matching loop in the lossless
matrix test, assert that remaining is empty so parsed output cannot contain
extra or duplicated lines; preserve the existing matching and multiplicity
checks.
---
Duplicate comments:
In
`@crates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rs`:
- Around line 51-53: Restore the public re-exports for parse_record_fields,
leading_component, and CompanyPortalRecordFields alongside
looks_like_record_start in the grammar exports of the logs module. Preserve
downstream access to all previously public items and do not make their removal
part of this change.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d853e0f-8b37-48b9-9e22-fd1f93dae999
📒 Files selected for processing (5)
.github/workflows/cmtrace-ci.ymlcrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rscrates/cmtraceopen-parser/tests/company_portal_windows_logs.rs
| // Lossless-by-construction check across the whole matrix: every non-empty | ||
| // source line must appear as a complete framed line (not merely as a | ||
| // substring of some other line), and multiplicity is preserved. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that no parsed lines remain unmatched.
The loop verifies that every source line exists in the parsed output. It does not verify that the parsed output contains only those lines. Extra or duplicated parsed lines remain in remaining, so the test can pass while multiplicity is incorrect. Assert that remaining is empty after the loop.
Proposed assertion
remaining.remove(index);
}
+ assert!(
+ remaining.is_empty(),
+ "{label}: parser emitted extra or duplicated lines: {remaining:?}"
+ );Also applies to: 731-744
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs` around lines
713 - 715, After the source-line matching loop in the lossless matrix test,
assert that remaining is empty so parsed output cannot contain extra or
duplicated lines; preserve the existing matching and multiplicity checks.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cmtrace-ci.yml (1)
233-247: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe
restore-keysfallback can serve a stalesrc-tauri/target/to this job.Line 243 restores any
${{ runner.os }}-esp-cargo-cache when the lockfile hash changes. On Windows a target directory built by a different toolchain revision produces loader failures at test start, which matches theSTATUS_ENTRYPOINT_NOT_FOUNDthis job reported twice before any test ran. Include the toolchain identity in the key, or cache only the registry and git directories and rebuildtarget/on this job.♻️ Suggested key change
- key: ${{ runner.os }}-esp-cargo-${{ hashFiles('Cargo.lock') }} - restore-keys: ${{ runner.os }}-esp-cargo- + key: ${{ runner.os }}-esp-cargo-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('Cargo.lock') }} + restore-keys: ${{ runner.os }}-esp-cargo-${{ steps.toolchain.outputs.cachekey }}-
dtolnay/rust-toolchainexposescachekey; give the step at Line 231 anid: toolchain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/cmtrace-ci.yml around lines 233 - 247, Update the “Cache Rust dependencies” step and its surrounding toolchain setup to prevent restore-keys from reusing an incompatible src-tauri/target directory: expose the dtolnay/rust-toolchain cachekey with the toolchain step id “toolchain” and incorporate that identity into the cache key and restore-keys, or remove target/ from the cached paths while retaining registry and git caches.
♻️ Duplicate comments (6)
crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs (2)
739-746: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStill open: assert that
remainingis empty after the loop.The multiset now proves every source line survives. It does not prove the parser emitted nothing extra. A record that duplicates a physical line, or that invents one, leaves entries in
remainingand the test still passes. Multiplicity is only half checked.💚 Proposed assertion
remaining.remove(index); } + assert!( + remaining.is_empty(), + "{label}: parser emitted extra or duplicated lines: {remaining:?}" + ); }As per path instructions: "Verify assertions test real behavior rather than restating the implementation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs` around lines 739 - 746, After the loop that matches each non-empty source line against remaining, assert that remaining is empty so extra or duplicated parser output causes the test to fail. Keep the existing multiplicity-aware matching and loss panic unchanged, and add the assertion in the same test flow.Source: Path instructions
277-287: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis test fails on this head, and it panics on the wrong assertion when it does.
The PR objectives state that the empty-input correction was not applied to this head, so
build_coveragestill reportsAvailablefor zero records. Fixbuild_coverageso zero parsed records cannot yield available coverage.Second, separate defect in the test itself: Line 283 indexes
coverage[0]with no prior length check. If the empty path yields an empty coverage vector, the failure is an index-out-of-bounds panic rather than the contract message on Line 285. Assert the vector is non-empty first.💚 Proposed test hardening
assert!(document.records.is_empty()); + assert!( + !document.coverage.is_empty(), + "an empty input must still produce a coverage row" + ); assert_eq!( document.coverage[0].status,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs` around lines 277 - 287, Update build_coverage so inputs producing zero parsed records never receive CompanyPortalCoverageStatus::Available, while preserving the existing non-empty coverage behavior. In portal_logs_empty_input_is_not_available_coverage, first assert document.coverage is non-empty, then access coverage[0] for the status assertion so failures report the intended contract violation instead of panicking.src/lib/tail-payload-validation.test.ts (1)
248-267: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEvery non-tags case passes
{}, so this table proves only that an object is refused.Line 251 supplies
{}for all fields excepttags. That trips rejection for the same trivial reason each time and never exercises the constraint the field declares.The untested gap is the string enums.
severity,format, andentryKindgate every entry throughSEVERITIES,LOG_FORMATS, andENTRY_KINDS. No case supplies a plausible wrong string such as"Verbose"or"Bogus". Those sets are hand-maintained mirrors of Rust enums, which is the drift risk I flagged atsrc/lib/tail-payload-validation.tslines 14 to 67, so the realistic failure is the one with no coverage.Give each field an invalid value that matches its declared type, and add explicit unknown-string cases for
severity,format, andentryKind.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tail-payload-validation.test.ts` around lines 248 - 267, Update the invalid optional-field table in the `rejects an invalid optional LogEntry` test so each field uses a type-compatible value that violates its specific constraint instead of `{}`. Add explicit unknown-string cases for `severity`, `format`, and `entryKind`, while retaining the existing `tags` case, so validation against `SEVERITIES`, `LOG_FORMATS`, and `ENTRY_KINDS` is exercised.src/lib/tail-payload-validation.ts (2)
161-166: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
physicalEndLinesplits every message twice and allocates one array per call.
isLogEntrycalls it at line 217 andparseTailPayloadcalls it again for the same entry at line 301. Each call runsmessage.split("\n"). That is two full passes over the message bytes of the whole batch plus 2N throwaway arrays, on the event thread. The test at line 236 ofsrc/lib/tail-payload-validation.test.tsalready drives 200,000 entries through this.Count newlines in place, and reuse the value across both call sites.
♻️ Allocation-free line count
function physicalEndLine(entry: LogEntry): number | null { - const endLine = entry.lineNumber + entry.message.split("\n").length - 1; + let newlines = 0; + for (let index = 0; index < entry.message.length; index += 1) { + if (entry.message.charCodeAt(index) === 10) { + newlines += 1; + } + } + const endLine = entry.lineNumber + newlines; return isSafeInteger(endLine, entry.lineNumber, 4_294_967_295) ? endLine : null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tail-payload-validation.ts` around lines 161 - 166, Update physicalEndLine and the validation flow to count newline characters without message.split allocations, then compute the physical end line from that count. Ensure isLogEntry and parseTailPayload reuse the same computed end-line value for each entry rather than invoking physicalEndLine twice, while preserving the existing integer-range validation and null behavior.
14-67: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winA missing enum entry silently stops tailing, and the type annotation cannot catch it.
LOG_FORMATS,PARSER_KINDS, andPARSER_IMPLEMENTATIONSmirror Rust enums by hand.Set<ParserKind>rejects an entry outside the union, but it accepts a set that is missing a variant. That is the direction that breaks.Add a variant to
ParserKindwithout adding it at line 23, andisParserSelectionfails,parseTailPayloadreturnsnull, anduse-file-watcher.tsline 122 drops the batch with oneconsole.error. Tailing stops with no visible cause.Derive each set from an exhaustive
Record<Union, true>sonpx tsc --noEmitfails on a missing variant.♻️ Compile-enforced allowlists
-const PARSER_KINDS = new Set<ParserKind>([ - "ccm", - "simple", +const PARSER_KIND_MEMBERS: Record<ParserKind, true> = { + ccm: true, + simple: true, // every remaining variant, each required by the Record type - "companyPortal", -]); + companyPortal: true, +}; +const PARSER_KINDS = new Set<ParserKind>( + Object.keys(PARSER_KIND_MEMBERS) as ParserKind[], +);Apply the same shape to
LOG_FORMATSandPARSER_IMPLEMENTATIONS.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tail-payload-validation.ts` around lines 14 - 67, Replace the manually typed Set allowlists LOG_FORMATS, PARSER_KINDS, and PARSER_IMPLEMENTATIONS with exhaustive Record<Union, true> definitions, then derive each Set from its record keys. Ensure every current variant remains included and future additions to LogFormat, ParserKind, or ParserImplementation cause a TypeScript compile error until added to the corresponding allowlist.Source: Coding guidelines
src-tauri/src/commands/file_ops.rs (1)
711-727: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winA duplicate coordinate in any file aborts the whole folder load.
index_aggregate_entriesindexes every aggregate entry, but line 414 queries it only for Company Portal files. Line 399 propagates the duplicate error, soopen_log_folder_aggregatereturnsAppError::Internaland the user sees nothing. The folder can contain any parser, and a parser that emits two entries for one physical line makes an unrelated file's tail seed break the whole open.Scope the index to tail-capable files, and downgrade a duplicate to "no seed for this file".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/commands/file_ops.rs` around lines 711 - 727, Update index_aggregate_entries and its caller in open_log_folder_aggregate to index only tail-capable files, such as Company Portal entries, before querying the index. Treat duplicate coordinates within that scoped index as an absent seed for the affected file rather than returning AppError::Internal, while preserving valid seed lookups and allowing unrelated parser entries to load normally.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitattributes:
- Around line 7-14: Add the -whitespace attribute to the existing
src-tauri/tests/fixtures/** rule in .gitattributes, preserving its -text
behavior so deliberate CRLF fixture data is excluded from git diff --check
whitespace validation.
In @.github/workflows/cmtrace-ci.yml:
- Around line 249-250: Remove the redundant Windows-specific parser clippy step
while retaining the Windows parser test step for path-handling coverage. Rename
the relevant parser test step to “Parser crate tests” and the remaining parser
clippy step to “Parser crate clippy” in the workflow.
In `@crates/cmtraceopen-parser/src/models/log_entry.rs`:
- Line 70: Add rustdoc comments to the new public `ParserKind::CompanyPortal`
and `ParserImplementation::CompanyPortal` enum variants, clearly documenting
their purpose and behavior while leaving the existing enum definitions
unchanged.
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs`:
- Around line 129-134: Update the continuation-line assertions in the MULTILINE
test to compare each expected physical line with an individual line from
record.raw_text, using whole-line equality rather than contains. Preserve
trimming only as currently intended and ensure the assertion rejects cross-line
matches and substring matches, validating the test’s byte-identity behavior.
In `@references/log-intune-reference.md`:
- Line 212: Update the MDM diagnostic export entry in the reference table to use
the actual filesystem path C:\Users\Public\Documents\MDMDiagnostics\ instead of
the Explorer display-name path, while preserving the existing format and export
instructions.
In `@src/stores/log-store.ts`:
- Around line 834-861: In amendEntry, replace the full buildGuidNameMap(entries)
rebuild with mergeGuidNameMap using the amended entry and existing guidNameMap.
Preserve the current entries update and ensure the incremental merge reflects
the single changed record equivalently.
---
Outside diff comments:
In @.github/workflows/cmtrace-ci.yml:
- Around line 233-247: Update the “Cache Rust dependencies” step and its
surrounding toolchain setup to prevent restore-keys from reusing an incompatible
src-tauri/target directory: expose the dtolnay/rust-toolchain cachekey with the
toolchain step id “toolchain” and incorporate that identity into the cache key
and restore-keys, or remove target/ from the cached paths while retaining
registry and git caches.
---
Duplicate comments:
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs`:
- Around line 739-746: After the loop that matches each non-empty source line
against remaining, assert that remaining is empty so extra or duplicated parser
output causes the test to fail. Keep the existing multiplicity-aware matching
and loss panic unchanged, and add the assertion in the same test flow.
- Around line 277-287: Update build_coverage so inputs producing zero parsed
records never receive CompanyPortalCoverageStatus::Available, while preserving
the existing non-empty coverage behavior. In
portal_logs_empty_input_is_not_available_coverage, first assert
document.coverage is non-empty, then access coverage[0] for the status assertion
so failures report the intended contract violation instead of panicking.
In `@src-tauri/src/commands/file_ops.rs`:
- Around line 711-727: Update index_aggregate_entries and its caller in
open_log_folder_aggregate to index only tail-capable files, such as Company
Portal entries, before querying the index. Treat duplicate coordinates within
that scoped index as an absent seed for the affected file rather than returning
AppError::Internal, while preserving valid seed lookups and allowing unrelated
parser entries to load normally.
In `@src/lib/tail-payload-validation.test.ts`:
- Around line 248-267: Update the invalid optional-field table in the `rejects
an invalid optional LogEntry` test so each field uses a type-compatible value
that violates its specific constraint instead of `{}`. Add explicit
unknown-string cases for `severity`, `format`, and `entryKind`, while retaining
the existing `tags` case, so validation against `SEVERITIES`, `LOG_FORMATS`, and
`ENTRY_KINDS` is exercised.
In `@src/lib/tail-payload-validation.ts`:
- Around line 161-166: Update physicalEndLine and the validation flow to count
newline characters without message.split allocations, then compute the physical
end line from that count. Ensure isLogEntry and parseTailPayload reuse the same
computed end-line value for each entry rather than invoking physicalEndLine
twice, while preserving the existing integer-range validation and null behavior.
- Around line 14-67: Replace the manually typed Set allowlists LOG_FORMATS,
PARSER_KINDS, and PARSER_IMPLEMENTATIONS with exhaustive Record<Union, true>
definitions, then derive each Set from its record keys. Ensure every current
variant remains included and future additions to LogFormat, ParserKind, or
ParserImplementation cause a TypeScript compile error until added to the
corresponding allowlist.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f51a2302-a170-4638-b463-58530bbd3947
⛔ Files ignored due to path filters (18)
crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/code-tokens/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf16le/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-bom/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf8-nobom/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/invalid-timestamp/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/malformed-structural-token/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/multiline-continuation/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-generic-timestamped/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/negative-unrelated-uwp/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/redaction/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/rotation/Log_2.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/same-timestamp-distinct-activity/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/severity-levels/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/truncated-boundaries/Log_1.logis excluded by!**/*.logcrates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v13-4-2/unknown-app-version/Log_1.logis excluded by!**/*.logsrc-tauri/tests/corpus/company_portal/clean/Log_1.logis excluded by!**/*.logsrc-tauri/tests/corpus/company_portal/negative/Log_1.logis excluded by!**/*.log
📒 Files selected for processing (31)
.gitattributes.github/workflows/cmtrace-ci.ymlcrates/cmtraceopen-parser/src/esp/mod.rscrates/cmtraceopen-parser/src/esp/redaction.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/detect.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/document.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/entries.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/framing.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/grammar.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/mod.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/models.rscrates/cmtraceopen-parser/src/intune/portal/windows/company_portal/logs/redaction.rscrates/cmtraceopen-parser/src/models/log_entry.rscrates/cmtraceopen-parser/src/parser/detect.rscrates/cmtraceopen-parser/src/parser/mod.rscrates/cmtraceopen-parser/tests/company_portal_windows_logs.rsreferences/log-intune-reference.mdsrc-tauri/src/commands/bundle_ops.rssrc-tauri/src/commands/file_ops.rssrc-tauri/src/commands/parsing.rssrc-tauri/src/state/app_state.rssrc-tauri/src/watcher/tail.rssrc-tauri/tests/parser_supported_formats.rssrc/hooks/use-file-watcher.test.tsxsrc/hooks/use-file-watcher.tssrc/lib/column-config.tssrc/lib/tail-payload-validation.test.tssrc/lib/tail-payload-validation.tssrc/stores/log-store.test.tssrc/stores/log-store.tssrc/types/log.ts
| src-tauri/tests/fixtures/** -text | ||
|
|
||
| # SCCM parser fixtures are byte-sensitive evidence; never normalize them. | ||
| crates/cmtraceopen-parser/tests/fixtures/** -text | ||
| # Parser-crate fixtures encode BOM, CRLF, and UTF-16LE deliberately; the tests | ||
| # assert on the decoded bytes, so a checkout must not rewrite line endings and | ||
| # `git diff --check` must not read a deliberate CR as trailing whitespace. | ||
| # Covers Company Portal, SCCM, and other pure-parser evidence corpora. | ||
| crates/cmtraceopen-parser/tests/fixtures/** -text -whitespace | ||
| src-tauri/tests/corpus/company_portal/** -text -whitespace |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the corpus path referenced by .gitattributes exists and holds fixtures.
fd -t d -H 'company_portal' src-tauri || echo "no company_portal dir under src-tauri"
fd -t d -H 'corpus' src-tauri || echo "no corpus dir under src-tauri"
fd -t f . src-tauri/tests 2>/dev/null | head -50
# Show which fixture paths the parser tests actually load.
rg -n 'include_(str|bytes)!' crates/cmtraceopen-parser/tests | head -50Repository: adamgell/cmtraceopen
Length of output: 10130
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- .gitattributes ---'
cat -n .gitattributes
printf '%s\n' '--- legacy fixture files and byte markers ---'
git ls-files 'src-tauri/tests/fixtures/**' 'src-tauri/tests/corpus/company_portal/**' |
while IFS= read -r f; do
printf '%s ' "$f"
python3 - "$f" <<'PY'
import sys
p = sys.argv[1]
b = open(p, "rb").read()
print({"bytes": len(b), "cr": b.count(b"\r"), "lf": b.count(b"\n"), "utf16le_bom": b.startswith(b"\xff\xfe"), "utf8_bom": b.startswith(b"\xef\xbb\xbf")})
PY
done
printf '%s\n' '--- effective attributes ---'
git check-attr -a -- \
src-tauri/tests/fixtures/ccm/psadt_install.log \
src-tauri/tests/corpus/company_portal/clean/Log_1.log \
crates/cmtraceopen-parser/tests/fixtures/intune/portal/windows/logs/v12-0-0/encoding-utf16le/Log_1.log
printf '%s\n' '--- whitespace check for legacy fixtures ---'
git diff --check --no-index /dev/null src-tauri/tests/fixtures/ccm/psadt_install.log || true
git diff --check --no-index /dev/null src-tauri/tests/corpus/company_portal/clean/Log_1.log || trueRepository: adamgell/cmtraceopen
Length of output: 24113
Add -whitespace to src-tauri/tests/fixtures/**.
These fixtures contain CRLF data, and git diff --check reports the deliberate CR bytes as trailing whitespace.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitattributes around lines 7 - 14, Add the -whitespace attribute to the
existing src-tauri/tests/fixtures/** rule in .gitattributes, preserving its
-text behavior so deliberate CRLF fixture data is excluded from git diff --check
whitespace validation.
| - name: Clippy parser crate | ||
| run: cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Does the parser crate contain any Windows-gated code that clippy would only lint on Windows?
rg -n 'cfg\(target_os *= *"windows"\)|cfg\(windows\)|target_family *= *"windows"' crates/cmtraceopen-parser || echo "no windows-gated code in cmtraceopen-parser"
rg -n -A20 '^\[(target|dependencies|features)' crates/cmtraceopen-parser/Cargo.tomlRepository: adamgell/cmtraceopen
Length of output: 480
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workflow context ---'
sed -n '60,100p;225,260p' .github/workflows/cmtrace-ci.yml
printf '%s\n' '--- parser crate source files and cfg attributes ---'
git ls-files crates/cmtraceopen-parser
rg -n 'cfg|target_os|target_family|windows|winapi|windows-sys' crates/cmtraceopen-parser || true
printf '%s\n' '--- workspace and package metadata ---'
sed -n '1,80p' crates/cmtraceopen-parser/Cargo.toml
rg -n -A12 -B3 'cmtraceopen-parser|workspace|target\.' Cargo.toml crates .github/workflows/cmtrace-ci.ymlRepository: adamgell/cmtraceopen
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
workflow = Path(".github/workflows/cmtrace-ci.yml").read_text()
manifest = Path("crates/cmtraceopen-parser/Cargo.toml").read_text()
sources = [
p for p in Path("crates/cmtraceopen-parser").rglob("*")
if p.is_file() and p.suffix in {".rs", ".toml"}
]
windows_cfg = []
for path in sources:
text = path.read_text(errors="replace")
if re.search(r'#\s*\[\s*cfg\s*\([^]]*(?:windows|target_os|target_family)[^]]*\)', text):
windows_cfg.append(str(path))
target_sections = re.findall(
r'(?ms)^\[target[^\]]*\].*?(?=^\[|\Z)', manifest
)
commands = re.findall(
r'(?ms)^\s*- name: (Parser crate tests|Parser crate clippy|Test parser crate contracts|Clippy parser crate)\s*$.*?^\s+run: (.+)$',
workflow,
)
print("windows_cfg_files:", windows_cfg or "none")
print("target_manifest_sections:", len(target_sections))
print("parser_commands:")
for name, command in commands:
print(f" {name}: {command}")
print("linux_command_count:", workflow.count(
"cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings"
))
PYRepository: adamgell/cmtraceopen
Length of output: 10627
Remove the redundant Windows parser clippy step. The parser crate has no Windows-gated source or target-specific manifest sections. Keep the Windows test step for Windows path handling. Rename the steps to Parser crate tests and Parser crate clippy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cmtrace-ci.yml around lines 249 - 250, Remove the
redundant Windows-specific parser clippy step while retaining the Windows parser
test step for path-handling coverage. Rename the relevant parser test step to
“Parser crate tests” and the remaining parser clippy step to “Parser crate
clippy” in the workflow.
| DnsDebug, | ||
| DnsAudit, | ||
| CmtLog, | ||
| CompanyPortal, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the new public enum variants.
Add rustdoc comments for ParserKind::CompanyPortal and ParserImplementation::CompanyPortal. These variants are public API commitments.
As per path instructions: "Treat every public item as a semver commitment: flag breaking changes to public types, signatures, or enum variants, and check that new public items are documented."
Also applies to: 95-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/cmtraceopen-parser/src/models/log_entry.rs` at line 70, Add rustdoc
comments to the new public `ParserKind::CompanyPortal` and
`ParserImplementation::CompanyPortal` enum variants, clearly documenting their
purpose and behavior while leaving the existing enum definitions unchanged.
Source: Path instructions
| for line in MULTILINE.lines().skip(2).take(3) { | ||
| assert!( | ||
| record.raw_text.contains(line.trim_end()), | ||
| "continuation line must survive verbatim: {line}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use whole-line equality here, same as the matrix test.
raw_text.contains(...) accepts a match that spans the join between two physical lines inside the same record, and it accepts a short continuation that is a substring of a longer one. The matrix test at Lines 731-745 was already corrected to per-line equality. Apply the same rule here so the byte-identity claim in the test name holds.
💚 Proposed assertion
for line in MULTILINE.lines().skip(2).take(3) {
+ let expected = line.trim_end();
assert!(
- record.raw_text.contains(line.trim_end()),
+ record.raw_text.lines().any(|kept| kept == expected),
"continuation line must survive verbatim: {line}"
);
}As per path instructions: "Verify assertions test real behavior rather than restating the implementation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/cmtraceopen-parser/tests/company_portal_windows_logs.rs` around lines
129 - 134, Update the continuation-line assertions in the MULTILINE test to
compare each expected physical line with an individual line from
record.raw_text, using whole-line equality rather than contains. Preserve
trimming only as currently intended and ensure the assertion rejects cross-line
matches and substring matches, validating the test’s byte-identity behavior.
Source: Path instructions
| | CP app logs | `C:\Users\<user>\AppData\Local\Packages\Microsoft.CompanyPortal_8wekyb3d8bbwe\LocalState\Log_<n>.log` | Plain text | Per-user CP events, errors, enrollment state | | ||
| | CP app logs | `C:\Users\<user>\AppData\Local\Packages\Microsoft.CompanyPortal_8wekyb3d8bbwe\LocalState\Log_<n>.log` | Column-aligned: ISO-8601 UTC, severity, category, scenario, sequence, activity GUID, app version, message | Per-user CP events, errors, enrollment state | | ||
| | CP bridge logs | `...\LocalState\Log.<BridgeName>_<n>.log` (`BridgeLauncher`, `ConfigurationManagerBridge`, `IntuneManagementExtensionBridge`) | Same grammar as the app log | ConfigMgr `root\ccm\ClientSDK` queries and IME service calls made on behalf of CP | | ||
| | MDM diagnostic export | `C:\Users\Public\Public Documents\MDMDiagnostics\` | .cab + .html | Exported via Settings > Accounts > Access work or school > Export management log files | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the MDM diagnostics path.
C:\Users\Public\Public Documents\ is the Explorer display name, not a filesystem path. The real export target is C:\Users\Public\Documents\MDMDiagnostics\. A reader who pastes the current string into a shell gets a missing path.
🐛 Proposed fix
-| MDM diagnostic export | `C:\Users\Public\Public Documents\MDMDiagnostics\` | .cab + .html | Exported via Settings > Accounts > Access work or school > Export management log files |
+| MDM diagnostic export | `C:\Users\Public\Documents\MDMDiagnostics\` | .cab + .html | Exported via Settings > Accounts > Access work or school > Export management log files |As per path instructions: "Documentation. Verify commands, paths, and flags actually exist in the repo."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | MDM diagnostic export | `C:\Users\Public\Public Documents\MDMDiagnostics\` | .cab + .html | Exported via Settings > Accounts > Access work or school > Export management log files | | |
| | MDM diagnostic export | `C:\Users\Public\Documents\MDMDiagnostics\` | .cab + .html | Exported via Settings > Accounts > Access work or school > Export management log files | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@references/log-intune-reference.md` at line 212, Update the MDM diagnostic
export entry in the reference table to use the actual filesystem path
C:\Users\Public\Documents\MDMDiagnostics\ instead of the Explorer display-name
path, while preserving the existing format and export instructions.
Source: Path instructions
| amendEntry: (amendment) => { | ||
| let amendmentApplied = false; | ||
| set((state) => { | ||
| const entryIndex = state.entries.findIndex( | ||
| (entry) => | ||
| entry.id === amendment.entryId && | ||
| canApplyTailAmendment(entry, amendment), | ||
| ); | ||
| if (entryIndex < 0) { | ||
| return state; | ||
| } | ||
|
|
||
| amendmentApplied = true; | ||
| const entries = [...state.entries]; | ||
| entries[entryIndex] = applyTailAmendment(entries[entryIndex], amendment); | ||
| return { | ||
| entries, | ||
| totalLines: Math.max( | ||
| state.totalLines, | ||
| amendment.continuationEndLine, | ||
| ), | ||
| guidNameMap: buildGuidNameMap(entries), | ||
| }; | ||
| }); | ||
| if (amendmentApplied) { | ||
| recomputeAndSetMatches(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Replace the full buildGuidNameMap rebuild with the incremental merge.
Line 855 rebuilds the GUID map from every entry for a single amended record. buildGuidNameMap walks all messages and parses JSON for each "Get policies" hit. Amendments arrive per tail batch on Company Portal continuation lines, so this runs repeatedly against a list that can hold hundreds of thousands of entries.
appendEntries (Line 830) and amendAggregateEntry (Line 963) already use the incremental mergeGuidNameMap. Use it here too. The result is equivalent because only one entry's message changed.
♻️ Proposed change
amendmentApplied = true;
const entries = [...state.entries];
- entries[entryIndex] = applyTailAmendment(entries[entryIndex], amendment);
+ const amendedEntry = applyTailAmendment(entries[entryIndex], amendment);
+ entries[entryIndex] = amendedEntry;
return {
entries,
totalLines: Math.max(
state.totalLines,
amendment.continuationEndLine,
),
- guidNameMap: buildGuidNameMap(entries),
+ guidNameMap: mergeGuidNameMap(state.guidNameMap, [amendedEntry]),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| amendEntry: (amendment) => { | |
| let amendmentApplied = false; | |
| set((state) => { | |
| const entryIndex = state.entries.findIndex( | |
| (entry) => | |
| entry.id === amendment.entryId && | |
| canApplyTailAmendment(entry, amendment), | |
| ); | |
| if (entryIndex < 0) { | |
| return state; | |
| } | |
| amendmentApplied = true; | |
| const entries = [...state.entries]; | |
| entries[entryIndex] = applyTailAmendment(entries[entryIndex], amendment); | |
| return { | |
| entries, | |
| totalLines: Math.max( | |
| state.totalLines, | |
| amendment.continuationEndLine, | |
| ), | |
| guidNameMap: buildGuidNameMap(entries), | |
| }; | |
| }); | |
| if (amendmentApplied) { | |
| recomputeAndSetMatches(); | |
| } | |
| }, | |
| amendEntry: (amendment) => { | |
| let amendmentApplied = false; | |
| set((state) => { | |
| const entryIndex = state.entries.findIndex( | |
| (entry) => | |
| entry.id === amendment.entryId && | |
| canApplyTailAmendment(entry, amendment), | |
| ); | |
| if (entryIndex < 0) { | |
| return state; | |
| } | |
| amendmentApplied = true; | |
| const entries = [...state.entries]; | |
| const amendedEntry = applyTailAmendment(entries[entryIndex], amendment); | |
| entries[entryIndex] = amendedEntry; | |
| return { | |
| entries, | |
| totalLines: Math.max( | |
| state.totalLines, | |
| amendment.continuationEndLine, | |
| ), | |
| guidNameMap: mergeGuidNameMap(state.guidNameMap, [amendedEntry]), | |
| }; | |
| }); | |
| if (amendmentApplied) { | |
| recomputeAndSetMatches(); | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/log-store.ts` around lines 834 - 861, In amendEntry, replace the
full buildGuidNameMap(entries) rebuild with mergeGuidNameMap using the amended
entry and existing guidNameMap. Preserve the current entries update and ensure
the incremental merge reflects the single changed record equivalently.
Scope
Implements the first raw-format parser for Windows Company Portal
LocalStateLog_<n>.logfiles for #366 (part of #356). Restacked on currentmainvianon-destructive merge (no force-push).
It adds content-confirmed detection, logical-record framing, typed
LogEntry-compatible projection, version-scoped raw evidence documents,synthetic/sanitized fixtures, default redaction, and the required parser/UI
registration. The pure parser crate performs no filesystem collection or live
Windows access.
Evidence and safety boundaries
Log_<n>.logname only nominates acandidate; record structure must confirm it.
versions are experimental/low-confidence coverage, not validated facts.
logs, generic timestamped logs, and same-time distinct activities are all
covered by synthetic fixtures.
Available) coverage.with bounded amendments and physical-line provenance.
real-version capture, or a semantic root-cause engine.
Restack note (lane A)
Merged
origin/main(8064b5aa) intocodex/intune-366-review-fixes-r119to clear
CONFLICTINGstate. Sole content conflict was.gitattributes;resolved by retaining byte-sensitive fixture rules for parser corpora
(
-text -whitespace) including Company Portal and SCCM paths.Local verification after restack
Refs #366
Summary by CodeRabbit