test(devicetree): cover summary counts, node paging, and property selection in devicetree views by saiyam-jain-dev · Pull Request #7305 · morluto/rea

saiyam-jain-dev · 2026-10-26T22:12:49Z

Summary

Adds comprehensive unit tests in src/domain/analysisView/devicetreeView.test.ts covering Devicetree analysis view projections, including summary count/reference calculations, node paging, property selection by node path, collection validation rules, and error handling for unsupported view kinds and missing node paths.

Problem and expected behavior

While projectDevicetreeView implements summary and paging views for Flattened Devicetree (.dtb) inspection evidence, it previously lacked a dedicated unit test suite covering its projection logic, collection validation rules (e.g. requiring path for properties, rejecting path for reservations/references, and handling nonexistent nodes), and calculated summary counters.

Change and scope

Created src/domain/analysisView/devicetreeView.test.ts verifying:
- Summary view calculation of reservation_count, reference_count, and unresolved_reference_count.
- Node collection paging with omission of nested properties, calculation of property_count, and assignment of source_index.
- Node selection by exact path preserving original node index.
- Property paging for a selected node path.
- Refusal of property queries without a path or with an absent node path.
- Reservation and reference collection paging with source_index.
- Refusal of path on collections that do not support path selection.
- Rejection of unsupported view kinds with AnalysisInputError.

Contribution scope fit: test-only coverage improvement within existing boundaries (not applicable).

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/analysisView/devicetreeView.test.ts.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: platform-independent.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): none.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: src/domain/analysisView/devicetreeView.test.ts
Base reproduction or other evidence: previously unexercised projection code.
User-visible CLI/MCP output: none.
Remaining proof gaps: none.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npx vitest run src/domain/analysisView/devicetreeView.test.ts — passed (9 passed in 35ms)
npm run check:fast (turbo run typecheck lint) — passed cleanly (2/2 tasks successful in 50s)

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none.

Author readiness

Human owner: @saiyam-jain-dev

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

chatgpt-codex-connector · 2026-10-26T22:13:00Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-26T22:16:08.497789Z We couldn’t complete this review. Comment `@codex review` to try again.	`a8e0415`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

saiyam-jain-dev · 2026-10-26T22:14:38Z

@codex review

chatgpt-codex-connector · 2026-10-26T22:16:08Z

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

An unknown error occurred

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

greptile-apps · 2026-10-26T22:16:45Z

Confidence Score: 4/5

[Low impact] The PR's effects stay within Devicetree view test coverage.

The tests match the projection behavior, but the required error-preservation change should be made before merging.

Findings

Tests hide returned error details ▶

Fix with agent prompt

### Issue 1
src/domain/analysisView/devicetreeView.test.ts:165-166
The successful-view tests replace returned errors with generic messages, making failures harder to diagnose. `AGENTS.md` requires test changes to follow `docs/testing.md`, which says to throw the typed error before asserting a successful `Result`.

Use `if (!result.ok) throw result.error`, then check `kind` separately. Apply this to the other successful-view tests too. This repository requirement must be satisfied before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds dedicated tests for Devicetree views.

Devicetree view outputs now have dedicated tests.

Source review found no new security issue. Tests were not rerun. PR supersession could not be checked because GitHub access required authentication.

_{Reviews (3) · Last reviewed commit: "test(devicetree): cover subsequent page ..." · Reviewed by Greptile}

…ection in devicetree views

greptile-apps · 2026-10-29T18:10:19Z

+  if (!result.ok || result.value.kind !== "devicetree")
+    throw new Error("Devicetree nodes page failed");


Tests hide returned error details

The successful-view tests replace returned errors with generic messages, making failures harder to diagnose. AGENTS.md requires test changes to follow docs/testing.md, which says to throw the typed error before asserting a successful Result.

Use if (!result.ok) throw result.error, then check kind separately. Apply this to the other successful-view tests too. This repository requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: src/domain/analysisView/devicetreeView.test.ts Line: 165-166 Comment: **Tests hide returned error details** The successful-view tests replace returned errors with generic messages, making failures harder to diagnose. `AGENTS.md` requires test changes to follow `docs/testing.md`, which says to throw the typed error before asserting a successful `Result`. Use `if (!result.ok) throw result.error`, then check `kind` separately. Apply this to the other successful-view tests too. This repository requirement must be satisfied before merging. **Context Used:** AGENTS.md ([source](https://github.com/morluto/rea/blob/main/AGENTS.md)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

CassieFox23 · 2026-11-08T15:58:48Z

TL;DR: I ran the new devicetreeView.test.ts on Node 22.23.4, 24.22.1 and 26.12.1, and 10 tests passed on each. The PR body says "9 passed", one lower than this head has. The test for unsupported view kinds never checks the AnalysisInputError class that the PR body names.

Check	Node 22.23.4	Node 24.22.1	Node 26.12.1
`src/domain/analysisView/devicetreeView.test.ts` at `0b7f6f5`	pass: 1 file, 10 tests	pass: 1 file, 10 tests	pass: 1 file, 10 tests

The PR body says the tests cover "Rejection of unsupported view kinds with AnalysisInputError".
In the diff, the test rejects unsupported view kinds with an input error (lines 397-415) uses toMatchObject on operation inspect_analysis_view and one issue { path: ["view"], reason: "invalid_value" }.
That test never checks the error class, and the name AnalysisInputError does not appear in the diff.
Four rejection tests in the file assert the same issue shape: path ["view"] and reason invalid_value. So they cannot tell the four rejections apart.
One idea: assert the error class and something specific to each case, such as its message.
CI on 0b7f6f5: all 17 check runs that ran passed, including CI required, Static checks, Source tests, Portable tests and the test shards. PR readiness also passed.
GitHub says the PR is mergeable, and its merge state is clean.
The head is 482 commits behind main b81da90.

Not run

A real devicetree blob.
A merge with current main, b81da90.

Test setup

Box: Fedora 44 VM (4 vCPUs, 8 GB) on my Nobara 44 PC, Linux 7.2.11
Node: 22.23.4, 24.22.1, 26.12.1
Head / base: 0b7f6f5 / c0c3902 (merge base with main)
Install: npm ci --offline and npm run test:prepare on Node 24.22.1, exit 0

Commands

npm ci --offline
npm run test:prepare
xvfb-run -a npm run test:focused -- src/domain/analysisView/devicetreeView.test.ts

🤖 Written and tested with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊

greptile-apps Bot reviewed Oct 26, 2026

View reviewed changes

Comment thread src/domain/analysisView/devicetreeView.test.ts Resolved

Comment thread src/domain/analysisView/devicetreeView.test.ts Resolved

CassieFox23 mentioned this pull request Oct 28, 2026

[Tracker] Open PRs that can't merge yet: conflicts and red CI #5654

Open

saiyam-jain-dev added 2 commits October 29, 2026 23:34

test(devicetree): cover summary counts, node paging, and property sel…

971d4d6

…ection in devicetree views

test(devicetree): cover subsequent page and assert structured errors

0b7f6f5

saiyam-jain-dev force-pushed the test/devicetree-view-coverage branch from f900571 to 0b7f6f5 Compare October 29, 2026 18:07

greptile-apps Bot reviewed Oct 29, 2026

View reviewed changes

xu91102 mentioned this pull request Nov 5, 2026

[Bug] Devicetree views silently select the first duplicate node path in imported Evidence #8969

Closed

Navigation Menu

Repository navigation

test(devicetree): cover summary counts, node paging, and property selection in devicetree views - #7305

test(devicetree): cover summary counts, node paging, and property selection in devicetree views#7305
saiyam-jain-dev wants to merge 2 commits into
morluto:mainfrom
saiyam-jain-dev:test/devicetree-view-coverage

saiyam-jain-dev commented Oct 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

saiyam-jain-dev commented Oct 26, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Uh oh!

greptile-apps Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Oct 29, 2026

Uh oh!

CassieFox23 commented Nov 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if (!result.ok \|\| result.value.kind !== "devicetree")
		throw new Error("Devicetree nodes page failed");

Conversation

saiyam-jain-dev commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem and expected behavior

Change and scope

Contract and boundary impact

Evidence and regression coverage

Validation performed

Compatibility, safety, and release

Author readiness

Before merge

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

saiyam-jain-dev commented Oct 26, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Uh oh!

greptile-apps Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

CassieFox23 commented Nov 8, 2026

Not run

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

saiyam-jain-dev commented Oct 26, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 26, 2026 •

edited

Loading

greptile-apps Bot commented Oct 26, 2026 •

edited

Loading